[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