diff mbox series

[v2] posix: Fix pidfd_spawn/pidfd_spawnp leak if execve fails (BZ 31695)

Message ID 20240506172816.1661462-1-adhemerval.zanella@linaro.org
State Superseded
Headers show
Series [v2] posix: Fix pidfd_spawn/pidfd_spawnp leak if execve fails (BZ 31695) | expand

Commit Message

Adhemerval Zanella May 6, 2024, 5:27 p.m. UTC
If the pidfd_spawn / pidfd_spawnp helper process succeeds, but evecve
fails for some reason (either with an invalid/non-existent, memory
allocation, etc.) the resulting pidfd is never closed, nor returned
to caller (so it can call close).

Since the process creation failed, it should be up to posix_spawn to
also, close the file descriptor in this case (similar to what it
does to reap the process).

Checked on x86_64-linux-gnu.
--
Changes from v1:
- Use __close_nocancel_nostatus instead of __close.
---
 posix/tst-spawn2.c               | 80 +++++++++++++++++++-------------
 sysdeps/unix/sysv/linux/spawni.c | 20 +++++---
 2 files changed, 61 insertions(+), 39 deletions(-)

Comments

Adhemerval Zanella May 20, 2024, 4:47 p.m. UTC | #1
Ping.

On 06/05/24 14:27, Adhemerval Zanella wrote:
> If the pidfd_spawn / pidfd_spawnp helper process succeeds, but evecve
> fails for some reason (either with an invalid/non-existent, memory
> allocation, etc.) the resulting pidfd is never closed, nor returned
> to caller (so it can call close).
> 
> Since the process creation failed, it should be up to posix_spawn to
> also, close the file descriptor in this case (similar to what it
> does to reap the process).
> 
> Checked on x86_64-linux-gnu.
> --
> Changes from v1:
> - Use __close_nocancel_nostatus instead of __close.
> ---
>  posix/tst-spawn2.c               | 80 +++++++++++++++++++-------------
>  sysdeps/unix/sysv/linux/spawni.c | 20 +++++---
>  2 files changed, 61 insertions(+), 39 deletions(-)
> 
> diff --git a/posix/tst-spawn2.c b/posix/tst-spawn2.c
> index bb507204a2..b2bad3f1f7 100644
> --- a/posix/tst-spawn2.c
> +++ b/posix/tst-spawn2.c
> @@ -26,6 +26,7 @@
>  #include <stdio.h>
>  
>  #include <support/check.h>
> +#include <support/descriptors.h>
>  #include <tst-spawn.h>
>  
>  int
> @@ -38,38 +39,53 @@ do_test (void)
>    char * const args[] = { 0 };
>    PID_T_TYPE pid = -1;
>  
> -  int ret = POSIX_SPAWN (&pid, program, 0, 0, args, environ);
> -  if (ret != ENOENT)
> -    {
> -      errno = ret;
> -      FAIL_EXIT1 ("posix_spawn: %m");
> -    }
> -
> -  /* POSIX states the value returned on pid variable in case of an error
> -     is not specified.  GLIBC will update the value iff the child
> -     execution is successful.  */
> -  if (pid != -1)
> -    FAIL_EXIT1 ("posix_spawn returned pid != -1 (%i)", (int) pid);
> -
> -  /* Check if no child is actually created.  */
> -  TEST_COMPARE (WAITID (P_ALL, 0, NULL, WEXITED), -1);
> -  TEST_COMPARE (errno, ECHILD);
> -
> -  /* Same as before, but with posix_spawnp.  */
> -  char *args2[] = { (char*) program, 0 };
> -
> -  ret = POSIX_SPAWNP (&pid, args2[0], 0, 0, args2, environ);
> -  if (ret != ENOENT)
> -    {
> -      errno = ret;
> -      FAIL_EXIT1 ("posix_spawnp: %m");
> -    }
> -
> -  if (pid != -1)
> -    FAIL_EXIT1 ("posix_spawnp returned pid != -1 (%i)", (int) pid);
> -
> -  TEST_COMPARE (WAITID (P_ALL, 0, NULL, WEXITED), -1);
> -  TEST_COMPARE (errno, ECHILD);
> +  {
> +    struct support_descriptors *descrs = support_descriptors_list ();
> +
> +    int ret = POSIX_SPAWN (&pid, program, 0, 0, args, environ);
> +    if (ret != ENOENT)
> +      {
> +	errno = ret;
> +	FAIL_EXIT1 ("posix_spawn: %m");
> +      }
> +
> +    /* POSIX states the value returned on pid variable in case of an error
> +       is not specified.  GLIBC will update the value iff the child
> +       execution is successful.  */
> +    if (pid != -1)
> +      FAIL_EXIT1 ("posix_spawn returned pid != -1 (%i)", (int) pid);
> +
> +    /* Check if no child is actually created.  */
> +    TEST_COMPARE (WAITID (P_ALL, 0, NULL, WEXITED), -1);
> +    TEST_COMPARE (errno, ECHILD);
> +
> +    /* Also check if there is no leak descriptors.  */
> +    support_descriptors_check (descrs);
> +    support_descriptors_free (descrs);
> +  }
> :+
> +  {
> +    /* Same as before, but with posix_spawnp.  */
> +    char *args2[] = { (char*) program, 0 };
> +
> +    struct support_descriptors *descrs = support_descriptors_list ();
> +
> +    int ret = POSIX_SPAWNP (&pid, args2[0], 0, 0, args2, environ);
> +    if (ret != ENOENT)
> +      {
> +	errno = ret;
> +	FAIL_EXIT1 ("posix_spawnp: %m");
> +      }
> +
> +    if (pid != -1)
> +      FAIL_EXIT1 ("posix_spawnp returned pid != -1 (%i)", (int) pid);
> +
> +    TEST_COMPARE (WAITID (P_ALL, 0, NULL, WEXITED), -1);
> +    TEST_COMPARE (errno, ECHILD);
> +
> +    support_descriptors_check (descrs);
> +    support_descriptors_free (descrs);
> +  }
>  
>    return 0;
>  }
> diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c
> index e8ed2babb9..1556dd17e0 100644
> --- a/sysdeps/unix/sysv/linux/spawni.c
> +++ b/sysdeps/unix/sysv/linux/spawni.c
> @@ -449,13 +449,19 @@ __spawnix (int *pid, const char *file,
>  	 caller to actually collect it.  */
>        ec = args.err;
>        if (ec > 0)
> -	/* There still an unlikely case where the child is cancelled after
> -	   setting args.err, due to a positive error value.  Also there is
> -	   possible pid reuse race (where the kernel allocated the same pid
> -	   to an unrelated process).  Unfortunately due synchronization
> -	   issues where the kernel might not have the process collected
> -	   the waitpid below can not use WNOHANG.  */
> -	__waitpid (new_pid, NULL, 0);
> +	{
> +	  /* There still an unlikely case where the child is cancelled after
> +	     setting args.err, due to a positive error value.  Also there is
> +	     possible pid reuse race (where the kernel allocated the same pid
> +	     to an unrelated process).  Unfortunately due synchronization
> +	     issues where the kernel might not have the process collected
> +	     the waitpid below can not use WNOHANG.  */
> +	  __waitpid (new_pid, NULL, 0);
> +	  /* For pidfd we need to also close the file descriptor for the case
> +	     where execve fails.  */
> +	  if (use_pidfd)
> +	    __close_nocancel_nostatus (args.pidfd);
> +	}
>      }
>    else
>      ec = errno;
Peter Cawley May 20, 2024, 4:54 p.m. UTC | #2
For whatever it is worth, the patch looks good to me for addressing the
reported bug.

That said, as we're in the area, we could potentially go further and
replace the __waitpid with a P_PIDFD waitid when we're in the use_pidfd
case, thereby closing the pid reuse race described in the comment.

On Mon, May 20, 2024 at 5:47 PM Adhemerval Zanella Netto <
adhemerval.zanella@linaro.org> wrote:

> Ping.
>
> On 06/05/24 14:27, Adhemerval Zanella wrote:
> > If the pidfd_spawn / pidfd_spawnp helper process succeeds, but evecve
> > fails for some reason (either with an invalid/non-existent, memory
> > allocation, etc.) the resulting pidfd is never closed, nor returned
> > to caller (so it can call close).
> >
> > Since the process creation failed, it should be up to posix_spawn to
> > also, close the file descriptor in this case (similar to what it
> > does to reap the process).
> >
> > Checked on x86_64-linux-gnu.
> > --
> > Changes from v1:
> > - Use __close_nocancel_nostatus instead of __close.
> > ---
> >  posix/tst-spawn2.c               | 80 +++++++++++++++++++-------------
> >  sysdeps/unix/sysv/linux/spawni.c | 20 +++++---
> >  2 files changed, 61 insertions(+), 39 deletions(-)
> >
> > diff --git a/posix/tst-spawn2.c b/posix/tst-spawn2.c
> > index bb507204a2..b2bad3f1f7 100644
> > --- a/posix/tst-spawn2.c
> > +++ b/posix/tst-spawn2.c
> > @@ -26,6 +26,7 @@
> >  #include <stdio.h>
> >
> >  #include <support/check.h>
> > +#include <support/descriptors.h>
> >  #include <tst-spawn.h>
> >
> >  int
> > @@ -38,38 +39,53 @@ do_test (void)
> >    char * const args[] = { 0 };
> >    PID_T_TYPE pid = -1;
> >
> > -  int ret = POSIX_SPAWN (&pid, program, 0, 0, args, environ);
> > -  if (ret != ENOENT)
> > -    {
> > -      errno = ret;
> > -      FAIL_EXIT1 ("posix_spawn: %m");
> > -    }
> > -
> > -  /* POSIX states the value returned on pid variable in case of an error
> > -     is not specified.  GLIBC will update the value iff the child
> > -     execution is successful.  */
> > -  if (pid != -1)
> > -    FAIL_EXIT1 ("posix_spawn returned pid != -1 (%i)", (int) pid);
> > -
> > -  /* Check if no child is actually created.  */
> > -  TEST_COMPARE (WAITID (P_ALL, 0, NULL, WEXITED), -1);
> > -  TEST_COMPARE (errno, ECHILD);
> > -
> > -  /* Same as before, but with posix_spawnp.  */
> > -  char *args2[] = { (char*) program, 0 };
> > -
> > -  ret = POSIX_SPAWNP (&pid, args2[0], 0, 0, args2, environ);
> > -  if (ret != ENOENT)
> > -    {
> > -      errno = ret;
> > -      FAIL_EXIT1 ("posix_spawnp: %m");
> > -    }
> > -
> > -  if (pid != -1)
> > -    FAIL_EXIT1 ("posix_spawnp returned pid != -1 (%i)", (int) pid);
> > -
> > -  TEST_COMPARE (WAITID (P_ALL, 0, NULL, WEXITED), -1);
> > -  TEST_COMPARE (errno, ECHILD);
> > +  {
> > +    struct support_descriptors *descrs = support_descriptors_list ();
> > +
> > +    int ret = POSIX_SPAWN (&pid, program, 0, 0, args, environ);
> > +    if (ret != ENOENT)
> > +      {
> > +     errno = ret;
> > +     FAIL_EXIT1 ("posix_spawn: %m");
> > +      }
> > +
> > +    /* POSIX states the value returned on pid variable in case of an
> error
> > +       is not specified.  GLIBC will update the value iff the child
> > +       execution is successful.  */
> > +    if (pid != -1)
> > +      FAIL_EXIT1 ("posix_spawn returned pid != -1 (%i)", (int) pid);
> > +
> > +    /* Check if no child is actually created.  */
> > +    TEST_COMPARE (WAITID (P_ALL, 0, NULL, WEXITED), -1);
> > +    TEST_COMPARE (errno, ECHILD);
> > +
> > +    /* Also check if there is no leak descriptors.  */
> > +    support_descriptors_check (descrs);
> > +    support_descriptors_free (descrs);
> > +  }
> > :+
> > +  {
> > +    /* Same as before, but with posix_spawnp.  */
> > +    char *args2[] = { (char*) program, 0 };
> > +
> > +    struct support_descriptors *descrs = support_descriptors_list ();
> > +
> > +    int ret = POSIX_SPAWNP (&pid, args2[0], 0, 0, args2, environ);
> > +    if (ret != ENOENT)
> > +      {
> > +     errno = ret;
> > +     FAIL_EXIT1 ("posix_spawnp: %m");
> > +      }
> > +
> > +    if (pid != -1)
> > +      FAIL_EXIT1 ("posix_spawnp returned pid != -1 (%i)", (int) pid);
> > +
> > +    TEST_COMPARE (WAITID (P_ALL, 0, NULL, WEXITED), -1);
> > +    TEST_COMPARE (errno, ECHILD);
> > +
> > +    support_descriptors_check (descrs);
> > +    support_descriptors_free (descrs);
> > +  }
> >
> >    return 0;
> >  }
> > diff --git a/sysdeps/unix/sysv/linux/spawni.c
> b/sysdeps/unix/sysv/linux/spawni.c
> > index e8ed2babb9..1556dd17e0 100644
> > --- a/sysdeps/unix/sysv/linux/spawni.c
> > +++ b/sysdeps/unix/sysv/linux/spawni.c
> > @@ -449,13 +449,19 @@ __spawnix (int *pid, const char *file,
> >        caller to actually collect it.  */
> >        ec = args.err;
> >        if (ec > 0)
> > -     /* There still an unlikely case where the child is cancelled after
> > -        setting args.err, due to a positive error value.  Also there is
> > -        possible pid reuse race (where the kernel allocated the same pid
> > -        to an unrelated process).  Unfortunately due synchronization
> > -        issues where the kernel might not have the process collected
> > -        the waitpid below can not use WNOHANG.  */
> > -     __waitpid (new_pid, NULL, 0);
> > +     {
> > +       /* There still an unlikely case where the child is cancelled
> after
> > +          setting args.err, due to a positive error value.  Also there
> is
> > +          possible pid reuse race (where the kernel allocated the same
> pid
> > +          to an unrelated process).  Unfortunately due synchronization
> > +          issues where the kernel might not have the process collected
> > +          the waitpid below can not use WNOHANG.  */
> > +       __waitpid (new_pid, NULL, 0);
> > +       /* For pidfd we need to also close the file descriptor for the
> case
> > +          where execve fails.  */
> > +       if (use_pidfd)
> > +         __close_nocancel_nostatus (args.pidfd);
> > +     }
> >      }
> >    else
> >      ec = errno;
>
Adhemerval Zanella May 20, 2024, 5:22 p.m. UTC | #3
Indeed using waitid with P_PIDFD is the correct approach, I will update
the patch.

On 20/05/24 13:54, Peter Cawley wrote:
> For whatever it is worth, the patch looks good to me for addressing the reported bug.
> 
> That said, as we're in the area, we could potentially go further and replace the __waitpid with a P_PIDFD waitid when we're in the use_pidfd case, thereby closing the pid reuse race described in the comment.
> 
> On Mon, May 20, 2024 at 5:47 PM Adhemerval Zanella Netto <adhemerval.zanella@linaro.org <mailto:adhemerval.zanella@linaro.org>> wrote:
> 
>     Ping.
> 
>     On 06/05/24 14:27, Adhemerval Zanella wrote:
>     > If the pidfd_spawn / pidfd_spawnp helper process succeeds, but evecve
>     > fails for some reason (either with an invalid/non-existent, memory
>     > allocation, etc.) the resulting pidfd is never closed, nor returned
>     > to caller (so it can call close).
>     >
>     > Since the process creation failed, it should be up to posix_spawn to
>     > also, close the file descriptor in this case (similar to what it
>     > does to reap the process).
>     >
>     > Checked on x86_64-linux-gnu.
>     > --
>     > Changes from v1:
>     > - Use __close_nocancel_nostatus instead of __close.
>     > ---
>     >  posix/tst-spawn2.c               | 80 +++++++++++++++++++-------------
>     >  sysdeps/unix/sysv/linux/spawni.c | 20 +++++---
>     >  2 files changed, 61 insertions(+), 39 deletions(-)
>     >
>     > diff --git a/posix/tst-spawn2.c b/posix/tst-spawn2.c
>     > index bb507204a2..b2bad3f1f7 100644
>     > --- a/posix/tst-spawn2.c
>     > +++ b/posix/tst-spawn2.c
>     > @@ -26,6 +26,7 @@
>     >  #include <stdio.h>
>     > 
>     >  #include <support/check.h>
>     > +#include <support/descriptors.h>
>     >  #include <tst-spawn.h>
>     > 
>     >  int
>     > @@ -38,38 +39,53 @@ do_test (void)
>     >    char * const args[] = { 0 };
>     >    PID_T_TYPE pid = -1;
>     > 
>     > -  int ret = POSIX_SPAWN (&pid, program, 0, 0, args, environ);
>     > -  if (ret != ENOENT)
>     > -    {
>     > -      errno = ret;
>     > -      FAIL_EXIT1 ("posix_spawn: %m");
>     > -    }
>     > -
>     > -  /* POSIX states the value returned on pid variable in case of an error
>     > -     is not specified.  GLIBC will update the value iff the child
>     > -     execution is successful.  */
>     > -  if (pid != -1)
>     > -    FAIL_EXIT1 ("posix_spawn returned pid != -1 (%i)", (int) pid);
>     > -
>     > -  /* Check if no child is actually created.  */
>     > -  TEST_COMPARE (WAITID (P_ALL, 0, NULL, WEXITED), -1);
>     > -  TEST_COMPARE (errno, ECHILD);
>     > -
>     > -  /* Same as before, but with posix_spawnp.  */
>     > -  char *args2[] = { (char*) program, 0 };
>     > -
>     > -  ret = POSIX_SPAWNP (&pid, args2[0], 0, 0, args2, environ);
>     > -  if (ret != ENOENT)
>     > -    {
>     > -      errno = ret;
>     > -      FAIL_EXIT1 ("posix_spawnp: %m");
>     > -    }
>     > -
>     > -  if (pid != -1)
>     > -    FAIL_EXIT1 ("posix_spawnp returned pid != -1 (%i)", (int) pid);
>     > -
>     > -  TEST_COMPARE (WAITID (P_ALL, 0, NULL, WEXITED), -1);
>     > -  TEST_COMPARE (errno, ECHILD);
>     > +  {
>     > +    struct support_descriptors *descrs = support_descriptors_list ();
>     > +
>     > +    int ret = POSIX_SPAWN (&pid, program, 0, 0, args, environ);
>     > +    if (ret != ENOENT)
>     > +      {
>     > +     errno = ret;
>     > +     FAIL_EXIT1 ("posix_spawn: %m");
>     > +      }
>     > +
>     > +    /* POSIX states the value returned on pid variable in case of an error
>     > +       is not specified.  GLIBC will update the value iff the child
>     > +       execution is successful.  */
>     > +    if (pid != -1)
>     > +      FAIL_EXIT1 ("posix_spawn returned pid != -1 (%i)", (int) pid);
>     > +
>     > +    /* Check if no child is actually created.  */
>     > +    TEST_COMPARE (WAITID (P_ALL, 0, NULL, WEXITED), -1);
>     > +    TEST_COMPARE (errno, ECHILD);
>     > +
>     > +    /* Also check if there is no leak descriptors.  */
>     > +    support_descriptors_check (descrs);
>     > +    support_descriptors_free (descrs);
>     > +  }
>     > :+
>     > +  {
>     > +    /* Same as before, but with posix_spawnp.  */
>     > +    char *args2[] = { (char*) program, 0 };
>     > +
>     > +    struct support_descriptors *descrs = support_descriptors_list ();
>     > +
>     > +    int ret = POSIX_SPAWNP (&pid, args2[0], 0, 0, args2, environ);
>     > +    if (ret != ENOENT)
>     > +      {
>     > +     errno = ret;
>     > +     FAIL_EXIT1 ("posix_spawnp: %m");
>     > +      }
>     > +
>     > +    if (pid != -1)
>     > +      FAIL_EXIT1 ("posix_spawnp returned pid != -1 (%i)", (int) pid);
>     > +
>     > +    TEST_COMPARE (WAITID (P_ALL, 0, NULL, WEXITED), -1);
>     > +    TEST_COMPARE (errno, ECHILD);
>     > +
>     > +    support_descriptors_check (descrs);
>     > +    support_descriptors_free (descrs);
>     > +  }
>     > 
>     >    return 0;
>     >  }
>     > diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c
>     > index e8ed2babb9..1556dd17e0 100644
>     > --- a/sysdeps/unix/sysv/linux/spawni.c
>     > +++ b/sysdeps/unix/sysv/linux/spawni.c
>     > @@ -449,13 +449,19 @@ __spawnix (int *pid, const char *file,
>     >        caller to actually collect it.  */
>     >        ec = args.err;
>     >        if (ec > 0)
>     > -     /* There still an unlikely case where the child is cancelled after
>     > -        setting args.err, due to a positive error value.  Also there is
>     > -        possible pid reuse race (where the kernel allocated the same pid
>     > -        to an unrelated process).  Unfortunately due synchronization
>     > -        issues where the kernel might not have the process collected
>     > -        the waitpid below can not use WNOHANG.  */
>     > -     __waitpid (new_pid, NULL, 0);
>     > +     {
>     > +       /* There still an unlikely case where the child is cancelled after
>     > +          setting args.err, due to a positive error value.  Also there is
>     > +          possible pid reuse race (where the kernel allocated the same pid
>     > +          to an unrelated process).  Unfortunately due synchronization
>     > +          issues where the kernel might not have the process collected
>     > +          the waitpid below can not use WNOHANG.  */
>     > +       __waitpid (new_pid, NULL, 0);
>     > +       /* For pidfd we need to also close the file descriptor for the case
>     > +          where execve fails.  */
>     > +       if (use_pidfd)
>     > +         __close_nocancel_nostatus (args.pidfd);
>     > +     }
>     >      }
>     >    else
>     >      ec = errno;
>
diff mbox series

Patch

diff --git a/posix/tst-spawn2.c b/posix/tst-spawn2.c
index bb507204a2..b2bad3f1f7 100644
--- a/posix/tst-spawn2.c
+++ b/posix/tst-spawn2.c
@@ -26,6 +26,7 @@ 
 #include <stdio.h>
 
 #include <support/check.h>
+#include <support/descriptors.h>
 #include <tst-spawn.h>
 
 int
@@ -38,38 +39,53 @@  do_test (void)
   char * const args[] = { 0 };
   PID_T_TYPE pid = -1;
 
-  int ret = POSIX_SPAWN (&pid, program, 0, 0, args, environ);
-  if (ret != ENOENT)
-    {
-      errno = ret;
-      FAIL_EXIT1 ("posix_spawn: %m");
-    }
-
-  /* POSIX states the value returned on pid variable in case of an error
-     is not specified.  GLIBC will update the value iff the child
-     execution is successful.  */
-  if (pid != -1)
-    FAIL_EXIT1 ("posix_spawn returned pid != -1 (%i)", (int) pid);
-
-  /* Check if no child is actually created.  */
-  TEST_COMPARE (WAITID (P_ALL, 0, NULL, WEXITED), -1);
-  TEST_COMPARE (errno, ECHILD);
-
-  /* Same as before, but with posix_spawnp.  */
-  char *args2[] = { (char*) program, 0 };
-
-  ret = POSIX_SPAWNP (&pid, args2[0], 0, 0, args2, environ);
-  if (ret != ENOENT)
-    {
-      errno = ret;
-      FAIL_EXIT1 ("posix_spawnp: %m");
-    }
-
-  if (pid != -1)
-    FAIL_EXIT1 ("posix_spawnp returned pid != -1 (%i)", (int) pid);
-
-  TEST_COMPARE (WAITID (P_ALL, 0, NULL, WEXITED), -1);
-  TEST_COMPARE (errno, ECHILD);
+  {
+    struct support_descriptors *descrs = support_descriptors_list ();
+
+    int ret = POSIX_SPAWN (&pid, program, 0, 0, args, environ);
+    if (ret != ENOENT)
+      {
+	errno = ret;
+	FAIL_EXIT1 ("posix_spawn: %m");
+      }
+
+    /* POSIX states the value returned on pid variable in case of an error
+       is not specified.  GLIBC will update the value iff the child
+       execution is successful.  */
+    if (pid != -1)
+      FAIL_EXIT1 ("posix_spawn returned pid != -1 (%i)", (int) pid);
+
+    /* Check if no child is actually created.  */
+    TEST_COMPARE (WAITID (P_ALL, 0, NULL, WEXITED), -1);
+    TEST_COMPARE (errno, ECHILD);
+
+    /* Also check if there is no leak descriptors.  */
+    support_descriptors_check (descrs);
+    support_descriptors_free (descrs);
+  }
:+
+  {
+    /* Same as before, but with posix_spawnp.  */
+    char *args2[] = { (char*) program, 0 };
+
+    struct support_descriptors *descrs = support_descriptors_list ();
+
+    int ret = POSIX_SPAWNP (&pid, args2[0], 0, 0, args2, environ);
+    if (ret != ENOENT)
+      {
+	errno = ret;
+	FAIL_EXIT1 ("posix_spawnp: %m");
+      }
+
+    if (pid != -1)
+      FAIL_EXIT1 ("posix_spawnp returned pid != -1 (%i)", (int) pid);
+
+    TEST_COMPARE (WAITID (P_ALL, 0, NULL, WEXITED), -1);
+    TEST_COMPARE (errno, ECHILD);
+
+    support_descriptors_check (descrs);
+    support_descriptors_free (descrs);
+  }
 
   return 0;
 }
diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c
index e8ed2babb9..1556dd17e0 100644
--- a/sysdeps/unix/sysv/linux/spawni.c
+++ b/sysdeps/unix/sysv/linux/spawni.c
@@ -449,13 +449,19 @@  __spawnix (int *pid, const char *file,
 	 caller to actually collect it.  */
       ec = args.err;
       if (ec > 0)
-	/* There still an unlikely case where the child is cancelled after
-	   setting args.err, due to a positive error value.  Also there is
-	   possible pid reuse race (where the kernel allocated the same pid
-	   to an unrelated process).  Unfortunately due synchronization
-	   issues where the kernel might not have the process collected
-	   the waitpid below can not use WNOHANG.  */
-	__waitpid (new_pid, NULL, 0);
+	{
+	  /* There still an unlikely case where the child is cancelled after
+	     setting args.err, due to a positive error value.  Also there is
+	     possible pid reuse race (where the kernel allocated the same pid
+	     to an unrelated process).  Unfortunately due synchronization
+	     issues where the kernel might not have the process collected
+	     the waitpid below can not use WNOHANG.  */
+	  __waitpid (new_pid, NULL, 0);
+	  /* For pidfd we need to also close the file descriptor for the case
+	     where execve fails.  */
+	  if (use_pidfd)
+	    __close_nocancel_nostatus (args.pidfd);
+	}
     }
   else
     ec = errno;