mailing list of musl libc
 help / color / mirror / code / Atom feed
From: Rich Felker <dalias@libc.org>
To: musl@lists.openwall.com
Subject: Re: [musl] [PATCH] mq_notify: fix close/recv race on failure path
Date: Tue, 13 Dec 2022 21:26:18 -0500	[thread overview]
Message-ID: <20221214022618.GB15716@brightrain.aerifal.cx> (raw)
In-Reply-To: <20221109104613.48062-1-izbyshev@ispras.ru>

On Wed, Nov 09, 2022 at 01:46:13PM +0300, Alexey Izbyshev wrote:
> In case of failure mq_notify closes the socket immediately after
> sending a cancellation request to the worker thread that is going to
> call or have already called recv on that socket. Even if we don't
> consider the kernel behavior when the only descriptor to an object that
> is being used in a system call is closed, if the socket descriptor is
> closed before the kernel looks at it, another thread could open a
> descriptor with the same value in the meantime, resulting in recv
> acting on a wrong object.
> 
> Fix the race by moving pthread_cancel call before the barrier wait to
> guarantee that the cancellation flag is set before the worker thread
> enters recv.
> ---
> Other ways to fix this:
> 
> * Remove the racing close call from mq_notify and surround recv
>   with pthread_cleanup_push/pop.
> 
> * Make the worker thread joinable initially, join it before closing
>   the socket on the failure path, and detach it on the happy path.
>   This would also require disabling cancellation around join/detach
>   to ensure that mq_notify itself is not cancelled in an inappropriate
>   state.

I'd put this aside for a while because of the pthread barrier
involvement I kinda didn't want to deal with. The fix you have sounds
like it works, but I think I'd rather pursue one of the other
approaches, probably the joinable thread one.

At present, the implementation of barriers seems to be buggy (I need
to dig back up the post about that), and they're also a really
expensive synchronization tool that goes both directions where we
really only need one direction (notifying the caller we're done
consuming the args). I'd rather switch to a semaphore, which is the
lightest and most idiomatic (at least per present-day musl idioms) way
to do this.

Using a joinable thread also lets us ensure we don't leave around
threads that are waiting to be scheduled just to exit on failure
return. Depending on scheduling attributes, this probably could be
bad.

(Also, arguably we should perhaps start the thread with unchanged
scheduling attributes and only change to the caller-provided ones
after successfully making the SYS_mq_notify syscall -- bleh. But
that's a separate topic.)

Rich

  reply	other threads:[~2022-12-14  2:26 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-09 10:46 Alexey Izbyshev
2022-12-14  2:26 ` Rich Felker [this message]
2022-12-14  6:49   ` Alexey Izbyshev
2023-02-10 16:29     ` Rich Felker
2023-02-11 14:45       ` Alexey Izbyshev
2023-02-11 14:52         ` Rich Felker
2023-02-11 15:13           ` Alexey Izbyshev
2023-02-11 15:06         ` Rich Felker
2023-02-11 17:13           ` Markus Wichmann
2023-02-11 17:46             ` Rich Felker
2023-02-11 17:50             ` Alexey Izbyshev
2023-02-11 17:59               ` Rich Felker
2023-02-11 18:08                 ` Alexey Izbyshev
2023-02-11 18:35                   ` Rich Felker
2023-02-11 19:28                     ` Alexey Izbyshev
2023-02-11 19:49                       ` Rich Felker
2023-02-11 20:14                         ` Alexey Izbyshev
2023-02-12  0:32                           ` Rich Felker
2023-02-12 18:23                             ` Alexey Izbyshev
2023-02-12 19:35                               ` Alexey Izbyshev
2023-02-12 20:04                                 ` Rich Felker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221214022618.GB15716@brightrain.aerifal.cx \
    --to=dalias@libc.org \
    --cc=musl@lists.openwall.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/musl/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).