Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Fri, 6 May 2022 15:29:05 -0700
From: Maciej Żenczykowski <maze@...gle.com>
To: Florian Weimer <fweimer@...hat.com>
Cc: enh <enh@...gle.com>, libc-coord@...ts.openwall.com
Subject: Re: posix_spawn() support for close_range(CLOSE_RANGE_CLOEXEC)

On Mon, Jan 24, 2022 at 2:08 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).
>
> 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.

FYI in https://android-review.googlesource.com/c/platform/bionic/+/1955269
and https://android-review.googlesource.com/c/platform/bionic/+/2072627
bionic implemented POSIX_SPAWN_CLOEXEC_DEFAULT which basically does
close_range(0, ~0U, CLOSE_RANGE_CLOEXEC) just before user registered
fd operations.

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.