[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAA2zVHq9dmBeWyZocAo4DCGQ=6uca88C6mMYrZn9YcGrZOXEEw@mail.gmail.com>
Date: Mon, 24 Jan 2022 19:28:01 -0500
From: James Y Knight <jyknight@...gle.com>
To: libc-coord@...ts.openwall.com
Cc: Florian Weimer <fweimer@...hat.com>, enh <enh@...gle.com>
Subject: Re: posix_spawn() support for close_range(CLOSE_RANGE_CLOEXEC)
On Mon, Jan 24, 2022 at 5:47 PM Maciej Żenczykowski <maze@...gle.com> wrote:
> On Mon, Jan 24, 2022 at 1:24 PM Florian Weimer <fweimer@...hat.com> wrote:
> >
> > > we have a proposal to add a new posix_spawn() flag to bionic
> > > (Android's libc) that would mark all file descriptors not otherwise
> > > mentioned as close-on-exec.
> > >
> > > (see
> https://android-review.googlesource.com/c/platform/bionic/+/1955269
> > > for the proposed implementation.)
> > >
> > > anyone aware of any other work in this kind of direction?
> >
> > Solaris and glibc have posix_spawn_file_actions_addclosefrom_np (Solaris
> > was first). Given that the execve is unavoidable (except for errors),
> > that seems pretty much equivalent. Perhaps POSIX_SPAWN_CLOEXEC_DEFAULT
> > is a little bit easier to use?
>
> Right, I realized that basically what I want/need is to close
> everything except for a specific set of fds.
> That set is stdin/out/err (closing these before exec is just a really
> bad idea, they should at the minimum be /dev/null) and any file
> descriptors I'm explicitly trying to pass (which requires explicit
> posix_spawn_actions_adddup2 (maybe addopen) calls or it's racy wrt
> other threads, since if they're opened pre-posix_spawn, then they
> need/should be opened with O_CLOEXEC anyway, and we don't have a
> posix_spawn_actions_adduncloexec helper besides dup2 - note that
> dup2(X, X) doesn't work to unset O_CLOEXEC).
I'm unclear what you mean to say here?
Per <https://sourceware.org/bugzilla/show_bug.cgi?id=23640> and <
https://www.austingroupbugs.net/view.php?id=411>
posix_spawn_actions_adddup2 _should_ cause O_CLOEXEC to be cleared,
although "dup2" itself does not.
I don't know if bionic implements that, but if it doesn't, your proposed
flag is going to be broken -- it will result in the broken behavior where
the FD gets closed (if the process _did_ mark it CLOEXEC initially, and the
input fd number happens to be the same in the parent, as the number you
want to pass on to the subprocess).
adddup2 is also better than addopen, because it allows for better
> error handling (dup2 is unlikely to fail, open much more so, logging
> prior to posix_spawn() is easier).
>
> Hence, this approach seems to be by far the easiest to use.
> Just set the flag, and your code works and doesn't leak fds any more
> even if other threads are forgetting to set O_CLOEXEC and/or racing
> with you.
>
Content of type "text/html" skipped
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
