[PATCH] posix: Handle negative fds in spawn's addfchdir

[PATCH] posix: Handle negative fds in spawn's addfchdir

[Lucas Chollet]

Hello all,
This is my first contribution here, so I hope I got everything right.
I tested this patch on my Linux x86_64 machine with `make check` and
got no regressions compared to master.

8<

This patch makes `posix_spawn_file_actions_addfchdir` return EBADF on
negative file descriptors. This is mandated by POSIX and similar to what
is already done in addopen/addclose.

Signed-off-by: Lucas Chollet <lucas.chollet@free.fr>
---
 posix/spawn_faction_addfchdir.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/posix/spawn_faction_addfchdir.c b/posix/spawn_faction_addfchdir.c
index a3df6df489..b51139a61f 100644
--- a/posix/spawn_faction_addfchdir.c
+++ b/posix/spawn_faction_addfchdir.c
@@ -28,6 +28,9 @@ posix_spawn_file_actions_addfchdir_np (posix_spawn_file_actions_t *actions,
 {
   struct __spawn_action *rec;
 
+  if (!__spawn_valid_fd (fd))
+    return EBADF;
+
   /* Allocate more memory if needed.  */
   if (actions->__used == actions->__allocated
       && __posix_spawn_file_actions_realloc (actions) != 0)
-- 
2.51.0

Re: [PATCH] posix: Handle negative fds in spawn's addfchdir

[Collin Funk]

Lucas Chollet <lucas.chollet@free.fr> writes:

> Hello all,
> This is my first contribution here, so I hope I got everything right.
> I tested this patch on my Linux x86_64 machine with `make check` and
> got no regressions compared to master.
>
> 8<
>
> This patch makes `posix_spawn_file_actions_addfchdir` return EBADF on
> negative file descriptors. This is mandated by POSIX and similar to what
> is already done in addopen/addclose.
>
> Signed-off-by: Lucas Chollet <lucas.chollet@free.fr>
> ---
>  posix/spawn_faction_addfchdir.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/posix/spawn_faction_addfchdir.c b/posix/spawn_faction_addfchdir.c
> index a3df6df489..b51139a61f 100644
> --- a/posix/spawn_faction_addfchdir.c
> +++ b/posix/spawn_faction_addfchdir.c
> @@ -28,6 +28,9 @@ posix_spawn_file_actions_addfchdir_np (posix_spawn_file_actions_t *actions,
>  {
>    struct __spawn_action *rec;
>  
> +  if (!__spawn_valid_fd (fd))
> +    return EBADF;
> +
>    /* Allocate more memory if needed.  */
>    if (actions->__used == actions->__allocated
>        && __posix_spawn_file_actions_realloc (actions) != 0)

Thank you for the patch!

However, I want to note here that __spawn_valid_fd() checks if the file
descriptor is greater than getdtablesize(). Using it here, along with
some (all?) other places it is used, is incorrect. POSIX states [1]:

    [EBADF]
        The value specified by fildes is negative.

    It shall not be considered an error for the path or fildes argument
    passed to these functions to specify a pathname or file descriptor
    for which the specified operation could not be performed at the time
    of the call. Any such error shall be detected when the associated
    file actions object is later used during a posix_spawn() or
    posix_spawnp() operation.

I noticed glibc's misbehavior when working on Gnulib last year, but
evidently forgot to fix it...

Collin

[1] https://pubs.opengroup.org/onlinepubs/9799919799/functions/posix_spawn_file_actions_addchdir.html
[2] https://lists.gnu.org/archive/html/bug-gnulib/2025-10/msg00118.html

Re: [PATCH] posix: Handle negative fds in spawn's addfchdir

[Adhemerval Zanella Netto]

On 25/03/26 15:27, Collin Funk wrote:
> Lucas Chollet <lucas.chollet@free.fr> writes:
> 
>> Hello all,
>> This is my first contribution here, so I hope I got everything right.
>> I tested this patch on my Linux x86_64 machine with `make check` and
>> got no regressions compared to master.
>>
>> 8<
>>
>> This patch makes `posix_spawn_file_actions_addfchdir` return EBADF on
>> negative file descriptors. This is mandated by POSIX and similar to what
>> is already done in addopen/addclose.
>>
>> Signed-off-by: Lucas Chollet <lucas.chollet@free.fr>
>> ---
>>  posix/spawn_faction_addfchdir.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/posix/spawn_faction_addfchdir.c b/posix/spawn_faction_addfchdir.c
>> index a3df6df489..b51139a61f 100644
>> --- a/posix/spawn_faction_addfchdir.c
>> +++ b/posix/spawn_faction_addfchdir.c
>> @@ -28,6 +28,9 @@ posix_spawn_file_actions_addfchdir_np (posix_spawn_file_actions_t *actions,
>>  {
>>    struct __spawn_action *rec;
>>  
>> +  if (!__spawn_valid_fd (fd))
>> +    return EBADF;
>> +
>>    /* Allocate more memory if needed.  */
>>    if (actions->__used == actions->__allocated
>>        && __posix_spawn_file_actions_realloc (actions) != 0)
> 
> Thank you for the patch!
> 
> However, I want to note here that __spawn_valid_fd() checks if the file
> descriptor is greater than getdtablesize(). Using it here, along with
> some (all?) other places it is used, is incorrect. POSIX states [1]:
> 
>     [EBADF]
>         The value specified by fildes is negative.
> 
>     It shall not be considered an error for the path or fildes argument
>     passed to these functions to specify a pathname or file descriptor
>     for which the specified operation could not be performed at the time
>     of the call. Any such error shall be detected when the associated
>     file actions object is later used during a posix_spawn() or
>     posix_spawnp() operation.
> 
> I noticed glibc's misbehavior when working on Gnulib last year, but
> evidently forgot to fix it...

Indeed it seems that POSIX 2018 [1] changed the error handling for possible
invalid file descriptors.   It seems that __spawn_valid_fd checks are not
required anymore.

[1] https://austingroupbugs.net/view.php?id=418 

> 
> Collin
> 
> [1] https://pubs.opengroup.org/onlinepubs/9799919799/functions/posix_spawn_file_actions_addchdir.html
> [2] https://lists.gnu.org/archive/html/bug-gnulib/2025-10/msg00118.html

Re: [PATCH] posix: Handle negative fds in spawn's addfchdir

[Adhemerval Zanella Netto]

On 25/03/26 16:15, Adhemerval Zanella Netto wrote:
> 
> 
> On 25/03/26 15:27, Collin Funk wrote:
>> Lucas Chollet <lucas.chollet@free.fr> writes:
>>
>>> Hello all,
>>> This is my first contribution here, so I hope I got everything right.
>>> I tested this patch on my Linux x86_64 machine with `make check` and
>>> got no regressions compared to master.
>>>
>>> 8<
>>>
>>> This patch makes `posix_spawn_file_actions_addfchdir` return EBADF on
>>> negative file descriptors. This is mandated by POSIX and similar to what
>>> is already done in addopen/addclose.
>>>
>>> Signed-off-by: Lucas Chollet <lucas.chollet@free.fr>
>>> ---
>>>  posix/spawn_faction_addfchdir.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/posix/spawn_faction_addfchdir.c b/posix/spawn_faction_addfchdir.c
>>> index a3df6df489..b51139a61f 100644
>>> --- a/posix/spawn_faction_addfchdir.c
>>> +++ b/posix/spawn_faction_addfchdir.c
>>> @@ -28,6 +28,9 @@ posix_spawn_file_actions_addfchdir_np (posix_spawn_file_actions_t *actions,
>>>  {
>>>    struct __spawn_action *rec;
>>>  
>>> +  if (!__spawn_valid_fd (fd))
>>> +    return EBADF;
>>> +
>>>    /* Allocate more memory if needed.  */
>>>    if (actions->__used == actions->__allocated
>>>        && __posix_spawn_file_actions_realloc (actions) != 0)
>>
>> Thank you for the patch!
>>
>> However, I want to note here that __spawn_valid_fd() checks if the file
>> descriptor is greater than getdtablesize(). Using it here, along with
>> some (all?) other places it is used, is incorrect. POSIX states [1]:
>>
>>     [EBADF]
>>         The value specified by fildes is negative.
>>
>>     It shall not be considered an error for the path or fildes argument
>>     passed to these functions to specify a pathname or file descriptor
>>     for which the specified operation could not be performed at the time
>>     of the call. Any such error shall be detected when the associated
>>     file actions object is later used during a posix_spawn() or
>>     posix_spawnp() operation.
>>
>> I noticed glibc's misbehavior when working on Gnulib last year, but
>> evidently forgot to fix it...
> 
> Indeed it seems that POSIX 2018 [1] changed the error handling for possible
> invalid file descriptors.   It seems that __spawn_valid_fd checks are not
> required anymore.
> 
> [1] https://austingroupbugs.net/view.php?id=418 

However, the POSIX requirements seems conflicting.  It states that

  [EBADF]
  The value specified by fildes is negative or greater than or equal to {OPEN_MAX}.

But at the same time it states that values potentially greater than
OPEN_MAX are not necessary an error:

  "It shall not be considered an error for the fildes argument passed to these
   functions to specify a file descriptor for which the specified operation could 
   not be performed"

So I think the 'EBADF' is a 'shall fail' only for negative values,
which seems the interpretation of musl and *BSD. I think it makes more sense.

Re: [PATCH] posix: Handle negative fds in spawn's addfchdir

[Collin Funk]

Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> writes:

> However, the POSIX requirements seems conflicting.  It states that
>
>   [EBADF]
>   The value specified by fildes is negative or greater than or equal to {OPEN_MAX}.
>
> But at the same time it states that values potentially greater than
> OPEN_MAX are not necessary an error:
>
>   "It shall not be considered an error for the fildes argument passed to these
>    functions to specify a file descriptor for which the specified operation could 
>    not be performed"
>
> So I think the 'EBADF' is a 'shall fail' only for negative values,
> which seems the interpretation of musl and *BSD. I think it makes more sense.

Likewise. I assume they just forgot to remove the text from EBADF. I can
open a Austin Group bug report and send a patch after confirmation from
them.

Collin

Re: [PATCH] posix: Handle negative fds in spawn's addfchdir

[Andreas Schwab]

On Mär 25 2026, Adhemerval Zanella Netto wrote:

> However, the POSIX requirements seems conflicting.  It states that
>
>   [EBADF]
>   The value specified by fildes is negative or greater than or equal to {OPEN_MAX}.

https://pubs.opengroup.org/onlinepubs/9799919799/functions/posix_spawn_file_actions_addfchdir.html
does not say anything about OPEN_MAX.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [PATCH] posix: Handle negative fds in spawn's addfchdir

[Collin Funk]

Andreas Schwab <schwab@linux-m68k.org> writes:

> On Mär 25 2026, Adhemerval Zanella Netto wrote:
>
>> However, the POSIX requirements seems conflicting.  It states that
>>
>>   [EBADF]
>>   The value specified by fildes is negative or greater than or equal to {OPEN_MAX}.
>
> https://pubs.opengroup.org/onlinepubs/9799919799/functions/posix_spawn_file_actions_addfchdir.html
> does not say anything about OPEN_MAX.

I think Adhemerval was referring to posix_spawn_file_actions_addclose
which does have the OPEN_MAX text [1].

Collin

[1] https://pubs.opengroup.org/onlinepubs/9799919799/functions/posix_spawn_file_actions_addclose.html

[PATCH v2] posix: Handle negative fds in spawn's addfchdir

[Lucas Chollet]

This patch makes `posix_spawn_file_actions_addfchdir` return EBADF on
negative file descriptors. This is mandated by POSIX.

Signed-off-by: Lucas Chollet <lucas.chollet@free.fr>
---
v2:
 - I used fd < 0 in place of !__spawn_valid_fd (fd)

 posix/spawn_faction_addfchdir.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/posix/spawn_faction_addfchdir.c b/posix/spawn_faction_addfchdir.c
index a3df6df489..e851af1722 100644
--- a/posix/spawn_faction_addfchdir.c
+++ b/posix/spawn_faction_addfchdir.c
@@ -28,6 +28,9 @@ posix_spawn_file_actions_addfchdir_np (posix_spawn_file_actions_t *actions,
 {
   struct __spawn_action *rec;
 
+  if (fd < 0)
+    return EBADF;
+
   /* Allocate more memory if needed.  */
   if (actions->__used == actions->__allocated
       && __posix_spawn_file_actions_realloc (actions) != 0)
-- 
2.51.0

Re: [PATCH] posix: Handle negative fds in spawn's addfchdir

[Lucas]

Thanks for the quick review!

I initially used (fd < 0) but when I realized other factions functions used

__spawn_valid_fd I told myself that it was the surely the way to go.

Anyway, I sent a v2 that uses (fd < 0) instead.

Lucas

On 3/25/26 19:27, Collin Funk wrote:
> Lucas Chollet <lucas.chollet@free.fr> writes:
>
>> Hello all,
>> This is my first contribution here, so I hope I got everything right.
>> I tested this patch on my Linux x86_64 machine with `make check` and
>> got no regressions compared to master.
>>
>> 8<
>>
>> This patch makes `posix_spawn_file_actions_addfchdir` return EBADF on
>> negative file descriptors. This is mandated by POSIX and similar to what
>> is already done in addopen/addclose.
>>
>> Signed-off-by: Lucas Chollet <lucas.chollet@free.fr>
>> ---
>>   posix/spawn_faction_addfchdir.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/posix/spawn_faction_addfchdir.c b/posix/spawn_faction_addfchdir.c
>> index a3df6df489..b51139a61f 100644
>> --- a/posix/spawn_faction_addfchdir.c
>> +++ b/posix/spawn_faction_addfchdir.c
>> @@ -28,6 +28,9 @@ posix_spawn_file_actions_addfchdir_np (posix_spawn_file_actions_t *actions,
>>   {
>>     struct __spawn_action *rec;
>>   
>> +  if (!__spawn_valid_fd (fd))
>> +    return EBADF;
>> +
>>     /* Allocate more memory if needed.  */
>>     if (actions->__used == actions->__allocated
>>         && __posix_spawn_file_actions_realloc (actions) != 0)
> Thank you for the patch!
>
> However, I want to note here that __spawn_valid_fd() checks if the file
> descriptor is greater than getdtablesize(). Using it here, along with
> some (all?) other places it is used, is incorrect. POSIX states [1]:
>
>      [EBADF]
>          The value specified by fildes is negative.
>
>      It shall not be considered an error for the path or fildes argument
>      passed to these functions to specify a pathname or file descriptor
>      for which the specified operation could not be performed at the time
>      of the call. Any such error shall be detected when the associated
>      file actions object is later used during a posix_spawn() or
>      posix_spawnp() operation.
>
> I noticed glibc's misbehavior when working on Gnulib last year, but
> evidently forgot to fix it...
>
> Collin
>
> [1] https://pubs.opengroup.org/onlinepubs/9799919799/functions/posix_spawn_file_actions_addchdir.html
> [2] https://lists.gnu.org/archive/html/bug-gnulib/2025-10/msg00118.html

Re: [PATCH] posix: Handle negative fds in spawn's addfchdir

[Lucas]

If you want to open a Austin Group bug report, for completeness,

posix_spawn_file_actions_adddup2 also has the OPEN_MAX text[1].

[1] 
https://pubs.opengroup.org/onlinepubs/9799919799/functions/posix_spawn_file_actions_adddup2.html

On 3/25/26 23:11, Collin Funk wrote:
> Andreas Schwab <schwab@linux-m68k.org> writes:
>
>> On Mär 25 2026, Adhemerval Zanella Netto wrote:
>>
>>> However, the POSIX requirements seems conflicting.  It states that
>>>
>>>    [EBADF]
>>>    The value specified by fildes is negative or greater than or equal to {OPEN_MAX}.
>> https://pubs.opengroup.org/onlinepubs/9799919799/functions/posix_spawn_file_actions_addfchdir.html
>> does not say anything about OPEN_MAX.
> I think Adhemerval was referring to posix_spawn_file_actions_addclose
> which does have the OPEN_MAX text [1].
>
> Collin
>
> [1] https://pubs.opengroup.org/onlinepubs/9799919799/functions/posix_spawn_file_actions_addclose.html

Re: [PATCH v2] posix: Handle negative fds in spawn's addfchdir

[Adhemerval Zanella Netto]

On 26/03/26 05:23, Lucas Chollet wrote:
> This patch makes `posix_spawn_file_actions_addfchdir` return EBADF on
> negative file descriptors. This is mandated by POSIX.
> 
> Signed-off-by: Lucas Chollet <lucas.chollet@free.fr>

I think we should remove __spawn_valid_fd and make the fd description
handling consistent on all posix_spawn routines (posix_spawn_file_actions_addclose,
posix_spawn_file_actions_addclosefrom, posix_spawn_file_actions_adddup2,
posix_spawn_file_actions_addopen, posix_spawn_file_actions_addtcsetpgrp_np,
and posix_spawn_file_actions_addfchdir_np).

This will also need to adjust posix/tst-posix_spawn-fd that expects
that expets sysconf (_SC_OPEN_MAX) to fail.


> ---
> v2:
>  - I used fd < 0 in place of !__spawn_valid_fd (fd)
> 
>  posix/spawn_faction_addfchdir.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/posix/spawn_faction_addfchdir.c b/posix/spawn_faction_addfchdir.c
> index a3df6df489..e851af1722 100644
> --- a/posix/spawn_faction_addfchdir.c
> +++ b/posix/spawn_faction_addfchdir.c
> @@ -28,6 +28,9 @@ posix_spawn_file_actions_addfchdir_np (posix_spawn_file_actions_t *actions,
>  {
>    struct __spawn_action *rec;
>  
> +  if (fd < 0)
> +    return EBADF;
> +
>    /* Allocate more memory if needed.  */
>    if (actions->__used == actions->__allocated
>        && __posix_spawn_file_actions_realloc (actions) != 0)

[PATCH v3] posix: spawn: Make handling of invalid fds consistent

[Lucas Chollet]

in `posix_spawn_file_actions_*` functions.

As mandated by POSIX, these functions should return `EBADF` on negative
file descriptors but let everything else fail during the call to
`posix_spawn`.

Signed-off-by: Lucas Chollet <lucas.chollet@free.fr>
---
v3: Make all spawn_faction routines consistent with regards to invalid
fd handling.

 posix/Makefile                        |  1 -
 posix/spawn_faction_addclose.c        |  2 +-
 posix/spawn_faction_addclosefrom.c    |  2 +-
 posix/spawn_faction_adddup2.c         |  2 +-
 posix/spawn_faction_addfchdir.c       |  3 +++
 posix/spawn_faction_addopen.c         |  2 +-
 posix/spawn_faction_addtcsetpgrp_np.c |  2 +-
 posix/spawn_int.h                     |  4 ----
 posix/spawn_valid_fd.c                | 30 ---------------------------
 posix/tst-posix_spawn-fd.c            |  2 +-
 10 files changed, 9 insertions(+), 41 deletions(-)
 delete mode 100644 posix/spawn_valid_fd.c

diff --git a/posix/Makefile b/posix/Makefile
index ec28b9e1da..bf5d5a8e23 100644
--- a/posix/Makefile
+++ b/posix/Makefile
@@ -155,7 +155,6 @@ routines := \
   spawn_faction_addtcsetpgrp_np \
   spawn_faction_destroy \
   spawn_faction_init \
-  spawn_valid_fd \
   spawnattr_destroy \
   spawnattr_getdefault \
   spawnattr_getflags \
diff --git a/posix/spawn_faction_addclose.c b/posix/spawn_faction_addclose.c
index 3073c05761..5a0dc6c1cf 100644
--- a/posix/spawn_faction_addclose.c
+++ b/posix/spawn_faction_addclose.c
@@ -29,7 +29,7 @@ __posix_spawn_file_actions_addclose (posix_spawn_file_actions_t *file_actions,
 {
   struct __spawn_action *rec;
 
-  if (!__spawn_valid_fd (fd))
+  if (fd < 0)
     return EBADF;
 
   /* Allocate more memory if needed.  */
diff --git a/posix/spawn_faction_addclosefrom.c b/posix/spawn_faction_addclosefrom.c
index 5c56d035c6..f55d69b2ea 100644
--- a/posix/spawn_faction_addclosefrom.c
+++ b/posix/spawn_faction_addclosefrom.c
@@ -28,7 +28,7 @@ __posix_spawn_file_actions_addclosefrom (posix_spawn_file_actions_t
 #if __SPAWN_SUPPORT_CLOSEFROM
   struct __spawn_action *rec;
 
-  if (!__spawn_valid_fd (from))
+  if (from < 0)
     return EBADF;
 
   /* Allocate more memory if needed.  */
diff --git a/posix/spawn_faction_adddup2.c b/posix/spawn_faction_adddup2.c
index f9564ef4c2..9190eb2a75 100644
--- a/posix/spawn_faction_adddup2.c
+++ b/posix/spawn_faction_adddup2.c
@@ -29,7 +29,7 @@ __posix_spawn_file_actions_adddup2 (posix_spawn_file_actions_t *file_actions,
 {
   struct __spawn_action *rec;
 
-  if (!__spawn_valid_fd (fd) || !__spawn_valid_fd (newfd))
+  if (fd < 0 || newfd < 0)
     return EBADF;
 
   /* Allocate more memory if needed.  */
diff --git a/posix/spawn_faction_addfchdir.c b/posix/spawn_faction_addfchdir.c
index a3df6df489..e851af1722 100644
--- a/posix/spawn_faction_addfchdir.c
+++ b/posix/spawn_faction_addfchdir.c
@@ -28,6 +28,9 @@ posix_spawn_file_actions_addfchdir_np (posix_spawn_file_actions_t *actions,
 {
   struct __spawn_action *rec;
 
+  if (fd < 0)
+    return EBADF;
+
   /* Allocate more memory if needed.  */
   if (actions->__used == actions->__allocated
       && __posix_spawn_file_actions_realloc (actions) != 0)
diff --git a/posix/spawn_faction_addopen.c b/posix/spawn_faction_addopen.c
index bb9fc41b99..bd610e79b0 100644
--- a/posix/spawn_faction_addopen.c
+++ b/posix/spawn_faction_addopen.c
@@ -31,7 +31,7 @@ __posix_spawn_file_actions_addopen (posix_spawn_file_actions_t *file_actions,
 {
   struct __spawn_action *rec;
 
-  if (!__spawn_valid_fd (fd))
+  if (fd < 0)
     return EBADF;
 
   char *path_copy = __strdup (path);
diff --git a/posix/spawn_faction_addtcsetpgrp_np.c b/posix/spawn_faction_addtcsetpgrp_np.c
index 291b115212..6f2f0d315e 100644
--- a/posix/spawn_faction_addtcsetpgrp_np.c
+++ b/posix/spawn_faction_addtcsetpgrp_np.c
@@ -27,7 +27,7 @@ __posix_spawn_file_actions_addtcsetpgrp_np (posix_spawn_file_actions_t
 {
   struct __spawn_action *rec;
 
-  if (!__spawn_valid_fd (tcfd))
+  if (tcfd < 0)
     return EBADF;
 
   /* Allocate more memory if needed.  */
diff --git a/posix/spawn_int.h b/posix/spawn_int.h
index 0291526edb..297a203c65 100644
--- a/posix/spawn_int.h
+++ b/posix/spawn_int.h
@@ -87,8 +87,4 @@ extern int __spawni (int *pid, const char *path,
 		     const posix_spawnattr_t *attrp, char *const argv[],
 		     char *const envp[], int xflags) attribute_hidden;
 
-/* Return true if FD falls into the range valid for file descriptors.
-   The check in this form is mandated by POSIX.  */
-bool __spawn_valid_fd (int fd) attribute_hidden;
-
 #endif /* _SPAWN_INT_H */
diff --git a/posix/spawn_valid_fd.c b/posix/spawn_valid_fd.c
deleted file mode 100644
index 1d4cd99256..0000000000
--- a/posix/spawn_valid_fd.c
+++ /dev/null
@@ -1,30 +0,0 @@
-/* File descriptor validity check for posix_spawn file actions.
-   Copyright (C) 2000-2026 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-
-   The GNU C Library is free software; you can redistribute it and/or
-   modify it under the terms of the GNU Lesser General Public
-   License as published by the Free Software Foundation; either
-   version 2.1 of the License, or (at your option) any later version.
-
-   The GNU C Library is distributed in the hope that it will be useful,
-   but WITHOUT ANY WARRANTY; without even the implied warranty of
-   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
-   Lesser General Public License for more details.
-
-   You should have received a copy of the GNU Lesser General Public
-   License along with the GNU C Library; if not, see
-   <https://www.gnu.org/licenses/>.  */
-
-#include "spawn_int.h"
-
-#include <unistd.h>
-
-bool
-__spawn_valid_fd (int fd)
-{
-  long maxfd = __sysconf (_SC_OPEN_MAX);
-  return __glibc_likely (fd >= 0)
-    && (__glibc_unlikely (maxfd < 0) /* No limit set.  */
-	|| __glibc_likely (fd < maxfd));
-}
diff --git a/posix/tst-posix_spawn-fd.c b/posix/tst-posix_spawn-fd.c
index e75d829562..72a40ff9a3 100644
--- a/posix/tst-posix_spawn-fd.c
+++ b/posix/tst-posix_spawn-fd.c
@@ -72,7 +72,7 @@ all_tests (const char *name, int (*func) (int))
   one_test (name, func, -1, false);
   one_test (name, func, -2, false);
   if (maxfd >= 0)
-    one_test (name, func, maxfd, false);
+    one_test (name, func, maxfd, true);
 }
 
 static int
-- 
2.51.0

Re: [PATCH v2] posix: Handle negative fds in spawn's addfchdir

[Collin Funk]

Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> writes:

> On 26/03/26 05:23, Lucas Chollet wrote:
>> This patch makes `posix_spawn_file_actions_addfchdir` return EBADF on
>> negative file descriptors. This is mandated by POSIX.
>> 
>> Signed-off-by: Lucas Chollet <lucas.chollet@free.fr>
>
> I think we should remove __spawn_valid_fd and make the fd description
> handling consistent on all posix_spawn routines (posix_spawn_file_actions_addclose,
> posix_spawn_file_actions_addclosefrom, posix_spawn_file_actions_adddup2,
> posix_spawn_file_actions_addopen, posix_spawn_file_actions_addtcsetpgrp_np,
> and posix_spawn_file_actions_addfchdir_np).
>
> This will also need to adjust posix/tst-posix_spawn-fd that expects
> that expets sysconf (_SC_OPEN_MAX) to fail.

I agree that removing __spawn_valid_fd is probably the correct decision.
However, I think we should wait for POSIX to clear up the contradictory
text. I opened a bug there [1].

Collin

[1] https://austingroupbugs.net/view.php?id=1977

Re: [PATCH v2] posix: Handle negative fds in spawn's addfchdir

[Adhemerval Zanella Netto]

On 26/03/26 17:35, Collin Funk wrote:
> Adhemerval Zanella Netto <adhemerval.zanella@linaro.org> writes:
> 
>> On 26/03/26 05:23, Lucas Chollet wrote:
>>> This patch makes `posix_spawn_file_actions_addfchdir` return EBADF on
>>> negative file descriptors. This is mandated by POSIX.
>>>
>>> Signed-off-by: Lucas Chollet <lucas.chollet@free.fr>
>>
>> I think we should remove __spawn_valid_fd and make the fd description
>> handling consistent on all posix_spawn routines (posix_spawn_file_actions_addclose,
>> posix_spawn_file_actions_addclosefrom, posix_spawn_file_actions_adddup2,
>> posix_spawn_file_actions_addopen, posix_spawn_file_actions_addtcsetpgrp_np,
>> and posix_spawn_file_actions_addfchdir_np).
>>
>> This will also need to adjust posix/tst-posix_spawn-fd that expects
>> that expets sysconf (_SC_OPEN_MAX) to fail.
> 
> I agree that removing __spawn_valid_fd is probably the correct decision.
> However, I think we should wait for POSIX to clear up the contradictory
> text. I opened a bug there [1].
> 
> Collin
> 
> [1] https://austingroupbugs.net/view.php?id=1977
> 

Sounds reasonable, although musl and *BSD already follows your suggestion.

Re: [PATCH v3] posix: spawn: Make handling of invalid fds consistent

[Adhemerval Zanella Netto]

On 26/03/26 11:13, Lucas Chollet wrote:
> in `posix_spawn_file_actions_*` functions.
> 
> As mandated by POSIX, these functions should return `EBADF` on negative
> file descriptors but let everything else fail during the call to
> `posix_spawn`.
> 
> Signed-off-by: Lucas Chollet <lucas.chollet@free.fr>

Hi Lucas, thanks again for the patch. It looks good, but I think it would
be better to wait for Austin remark on Collin's defect [1].  I think we
can wait until the release.

[1] https://austingroupbugs.net/view.php?id=1977

> ---
> v3: Make all spawn_faction routines consistent with regards to invalid
> fd handling.
> 
>  posix/Makefile                        |  1 -
>  posix/spawn_faction_addclose.c        |  2 +-
>  posix/spawn_faction_addclosefrom.c    |  2 +-
>  posix/spawn_faction_adddup2.c         |  2 +-
>  posix/spawn_faction_addfchdir.c       |  3 +++
>  posix/spawn_faction_addopen.c         |  2 +-
>  posix/spawn_faction_addtcsetpgrp_np.c |  2 +-
>  posix/spawn_int.h                     |  4 ----
>  posix/spawn_valid_fd.c                | 30 ---------------------------
>  posix/tst-posix_spawn-fd.c            |  2 +-
>  10 files changed, 9 insertions(+), 41 deletions(-)
>  delete mode 100644 posix/spawn_valid_fd.c
> 
> diff --git a/posix/Makefile b/posix/Makefile
> index ec28b9e1da..bf5d5a8e23 100644
> --- a/posix/Makefile
> +++ b/posix/Makefile
> @@ -155,7 +155,6 @@ routines := \
>    spawn_faction_addtcsetpgrp_np \
>    spawn_faction_destroy \
>    spawn_faction_init \
> -  spawn_valid_fd \
>    spawnattr_destroy \
>    spawnattr_getdefault \
>    spawnattr_getflags \
> diff --git a/posix/spawn_faction_addclose.c b/posix/spawn_faction_addclose.c
> index 3073c05761..5a0dc6c1cf 100644
> --- a/posix/spawn_faction_addclose.c
> +++ b/posix/spawn_faction_addclose.c
> @@ -29,7 +29,7 @@ __posix_spawn_file_actions_addclose (posix_spawn_file_actions_t *file_actions,
>  {
>    struct __spawn_action *rec;
>  
> -  if (!__spawn_valid_fd (fd))
> +  if (fd < 0)
>      return EBADF;
>  
>    /* Allocate more memory if needed.  */
> diff --git a/posix/spawn_faction_addclosefrom.c b/posix/spawn_faction_addclosefrom.c
> index 5c56d035c6..f55d69b2ea 100644
> --- a/posix/spawn_faction_addclosefrom.c
> +++ b/posix/spawn_faction_addclosefrom.c
> @@ -28,7 +28,7 @@ __posix_spawn_file_actions_addclosefrom (posix_spawn_file_actions_t
>  #if __SPAWN_SUPPORT_CLOSEFROM
>    struct __spawn_action *rec;
>  
> -  if (!__spawn_valid_fd (from))
> +  if (from < 0)
>      return EBADF;
>  
>    /* Allocate more memory if needed.  */
> diff --git a/posix/spawn_faction_adddup2.c b/posix/spawn_faction_adddup2.c
> index f9564ef4c2..9190eb2a75 100644
> --- a/posix/spawn_faction_adddup2.c
> +++ b/posix/spawn_faction_adddup2.c
> @@ -29,7 +29,7 @@ __posix_spawn_file_actions_adddup2 (posix_spawn_file_actions_t *file_actions,
>  {
>    struct __spawn_action *rec;
>  
> -  if (!__spawn_valid_fd (fd) || !__spawn_valid_fd (newfd))
> +  if (fd < 0 || newfd < 0)
>      return EBADF;
>  
>    /* Allocate more memory if needed.  */
> diff --git a/posix/spawn_faction_addfchdir.c b/posix/spawn_faction_addfchdir.c
> index a3df6df489..e851af1722 100644
> --- a/posix/spawn_faction_addfchdir.c
> +++ b/posix/spawn_faction_addfchdir.c
> @@ -28,6 +28,9 @@ posix_spawn_file_actions_addfchdir_np (posix_spawn_file_actions_t *actions,
>  {
>    struct __spawn_action *rec;
>  
> +  if (fd < 0)
> +    return EBADF;
> +
>    /* Allocate more memory if needed.  */
>    if (actions->__used == actions->__allocated
>        && __posix_spawn_file_actions_realloc (actions) != 0)
> diff --git a/posix/spawn_faction_addopen.c b/posix/spawn_faction_addopen.c
> index bb9fc41b99..bd610e79b0 100644
> --- a/posix/spawn_faction_addopen.c
> +++ b/posix/spawn_faction_addopen.c
> @@ -31,7 +31,7 @@ __posix_spawn_file_actions_addopen (posix_spawn_file_actions_t *file_actions,
>  {
>    struct __spawn_action *rec;
>  
> -  if (!__spawn_valid_fd (fd))
> +  if (fd < 0)
>      return EBADF;
>  
>    char *path_copy = __strdup (path);
> diff --git a/posix/spawn_faction_addtcsetpgrp_np.c b/posix/spawn_faction_addtcsetpgrp_np.c
> index 291b115212..6f2f0d315e 100644
> --- a/posix/spawn_faction_addtcsetpgrp_np.c
> +++ b/posix/spawn_faction_addtcsetpgrp_np.c
> @@ -27,7 +27,7 @@ __posix_spawn_file_actions_addtcsetpgrp_np (posix_spawn_file_actions_t
>  {
>    struct __spawn_action *rec;
>  
> -  if (!__spawn_valid_fd (tcfd))
> +  if (tcfd < 0)
>      return EBADF;
>  
>    /* Allocate more memory if needed.  */
> diff --git a/posix/spawn_int.h b/posix/spawn_int.h
> index 0291526edb..297a203c65 100644
> --- a/posix/spawn_int.h
> +++ b/posix/spawn_int.h
> @@ -87,8 +87,4 @@ extern int __spawni (int *pid, const char *path,
>  		     const posix_spawnattr_t *attrp, char *const argv[],
>  		     char *const envp[], int xflags) attribute_hidden;
>  
> -/* Return true if FD falls into the range valid for file descriptors.
> -   The check in this form is mandated by POSIX.  */
> -bool __spawn_valid_fd (int fd) attribute_hidden;
> -
>  #endif /* _SPAWN_INT_H */
> diff --git a/posix/spawn_valid_fd.c b/posix/spawn_valid_fd.c
> deleted file mode 100644
> index 1d4cd99256..0000000000
> --- a/posix/spawn_valid_fd.c
> +++ /dev/null
> @@ -1,30 +0,0 @@
> -/* File descriptor validity check for posix_spawn file actions.
> -   Copyright (C) 2000-2026 Free Software Foundation, Inc.
> -   This file is part of the GNU C Library.
> -
> -   The GNU C Library is free software; you can redistribute it and/or
> -   modify it under the terms of the GNU Lesser General Public
> -   License as published by the Free Software Foundation; either
> -   version 2.1 of the License, or (at your option) any later version.
> -
> -   The GNU C Library is distributed in the hope that it will be useful,
> -   but WITHOUT ANY WARRANTY; without even the implied warranty of
> -   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -   Lesser General Public License for more details.
> -
> -   You should have received a copy of the GNU Lesser General Public
> -   License along with the GNU C Library; if not, see
> -   <https://www.gnu.org/licenses/>.  */
> -
> -#include "spawn_int.h"
> -
> -#include <unistd.h>
> -
> -bool
> -__spawn_valid_fd (int fd)
> -{
> -  long maxfd = __sysconf (_SC_OPEN_MAX);
> -  return __glibc_likely (fd >= 0)
> -    && (__glibc_unlikely (maxfd < 0) /* No limit set.  */
> -	|| __glibc_likely (fd < maxfd));
> -}
> diff --git a/posix/tst-posix_spawn-fd.c b/posix/tst-posix_spawn-fd.c
> index e75d829562..72a40ff9a3 100644
> --- a/posix/tst-posix_spawn-fd.c
> +++ b/posix/tst-posix_spawn-fd.c
> @@ -72,7 +72,7 @@ all_tests (const char *name, int (*func) (int))
>    one_test (name, func, -1, false);
>    one_test (name, func, -2, false);
>    if (maxfd >= 0)
> -    one_test (name, func, maxfd, false);
> +    one_test (name, func, maxfd, true);
>  }
>  
>  static int