posix_spawn: preserve FD flags when processing FAE_OPEN
Commit Message
According to the POSIX documentation, "when the new process image is
executed, any file descriptor (from this new set) which has its
FD_CLOEXEC flag set shall be closed (see posix_spawn())." The "new set"
is after processing the file actions, so if addopen had the O_CLOEXEC
flag set the descriptor should be closed after the exec.
The adddup2 docs, by contrast, specify that the flag should be cleared,
even if dup2 wouldn't have done so due to the specified file descriptors
being equal. Add a comment to that effect.
Addresses: https://sourceware.org/pipermail/newlib/2025/021968.html
Fixes: c7c1a1ca1b ("2013-10-01 Petr Hosek <phosek@chromium.org>")
Signed-off-by: Jeremy Drake <cygwin@jdrake.com>
---
newlib/libc/posix/posix_spawn.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
Comments
On Jul 8 15:51, Jeremy Drake wrote:
> According to the POSIX documentation, "when the new process image is
> executed, any file descriptor (from this new set) which has its
> FD_CLOEXEC flag set shall be closed (see posix_spawn())." The "new set"
> is after processing the file actions, so if addopen had the O_CLOEXEC
> flag set the descriptor should be closed after the exec.
>
> The adddup2 docs, by contrast, specify that the flag should be cleared,
> even if dup2 wouldn't have done so due to the specified file descriptors
> being equal. Add a comment to that effect.
>
> Addresses: https://sourceware.org/pipermail/newlib/2025/021968.html
> Fixes: c7c1a1ca1b ("2013-10-01 Petr Hosek <phosek@chromium.org>")
> Signed-off-by: Jeremy Drake <cygwin@jdrake.com>
> ---
> newlib/libc/posix/posix_spawn.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
LGTM. dup3 would be easier, but I'm not sure we can easily use it from
within newlib, so, go ahead.
Thanks,
Corinna
On Wed, 9 Jul 2025, Corinna Vinschen wrote:
> On Jul 8 15:51, Jeremy Drake wrote:
> > According to the POSIX documentation, "when the new process image is
> > executed, any file descriptor (from this new set) which has its
> > FD_CLOEXEC flag set shall be closed (see posix_spawn())." The "new set"
> > is after processing the file actions, so if addopen had the O_CLOEXEC
> > flag set the descriptor should be closed after the exec.
> >
> > The adddup2 docs, by contrast, specify that the flag should be cleared,
> > even if dup2 wouldn't have done so due to the specified file descriptors
> > being equal. Add a comment to that effect.
> >
> > Addresses: https://sourceware.org/pipermail/newlib/2025/021968.html
> > Fixes: c7c1a1ca1b ("2013-10-01 Petr Hosek <phosek@chromium.org>")
> > Signed-off-by: Jeremy Drake <cygwin@jdrake.com>
> > ---
> > newlib/libc/posix/posix_spawn.c | 14 +++++++++++---
> > 1 file changed, 11 insertions(+), 3 deletions(-)
>
> LGTM. dup3 would be easier, but I'm not sure we can easily use it from
> within newlib, so, go ahead.
Pushed, along with the test in Cygwin's testsuite.
@@ -179,23 +179,31 @@ process_file_actions_entry(posix_spawn_file_actions_entry_t *fae)
if (fd < 0)
return (errno);
if (fd != fae->fae_fildes) {
+#ifdef HAVE_FCNTL
+ int fdflags = _fcntl(fd, F_GETFD);
+ if (fdflags == -1)
+ return (errno);
+#endif
if (dup2(fd, fae->fae_fildes) == -1)
return (errno);
if (_close(fd) != 0) {
if (errno == EBADF)
return (EBADF);
}
- }
#ifdef HAVE_FCNTL
- if (_fcntl(fae->fae_fildes, F_SETFD, 0) == -1)
- return (errno);
+ if (_fcntl(fae->fae_fildes, F_SETFD, fdflags) == -1)
+ return (errno);
#endif /* HAVE_FCNTL */
+ }
break;
case FAE_DUP2:
/* Perform a dup2() */
if (dup2(fae->fae_fildes, fae->fae_newfildes) == -1)
return (errno);
#ifdef HAVE_FCNTL
+ /* This is necessary because POSIX says the FD_CLOEXEC flag
+ * must be clear, even though dup2 is specified to not clear
+ * the FD_CLOEXEC flag if the two file descriptors are equal */
if (_fcntl(fae->fae_newfildes, F_SETFD, 0) == -1)
return (errno);
#endif /* HAVE_FCNTL */