Date: Thu, 17 Feb 2022 16:29:16 -0800 From: enh <enh@...gle.com> To: James Y Knight <jyknight@...gle.com> Cc: libc-coord@...ts.openwall.com, Florian Weimer <fweimer@...hat.com> Subject: Re: posix_spawn() support for close_range(CLOSE_RANGE_CLOEXEC) On Mon, Jan 24, 2022 at 4:28 PM James Y Knight <jyknight@...gle.com> wrote: > > > 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). > d'oh! yeah, we'd missed this special case for posix_spawn_file_actions_adddup2(). fixed by https://android-review.googlesource.com/c/platform/bionic/+/1989308 in AOSP for Android 13. thanks for pointing this out! > 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.