[PATCH] posix: fix system when a child cannot be created [BZ #32450]

[Adhemerval Zanella Netto]

On 11/12/24 17:47, Aurelien Jarno wrote:
> POSIX states that "if a child process cannot be created, or if the
> termination status for the command language interpreter cannot be
> obtained, system() shall return -1 and set errno to indicate the error."
> 
> In the glibc implementation it could happen when posix_spawn fails,
> which happens when the underlying fork, vfork, or clone call fails. They
> could fail with EAGAIN and ENOMEM.
> 
> ENOMEM could be returned by posix_spawn either because of a failure to
> create a child or a failure to execute a shell, so it is difficult to
> catch it. But we can at least improve system by catching the EAGAIN
> case.
> 
> Resolves: BZ #32450
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  stdlib/tst-system.c    | 17 +++++++++++++++++
>  sysdeps/posix/system.c | 10 +++++++---
>  2 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/stdlib/tst-system.c b/stdlib/tst-system.c
> index b5b630a41b..dff21557e9 100644
> --- a/stdlib/tst-system.c
> +++ b/stdlib/tst-system.c
> @@ -20,6 +20,7 @@
>  #include <string.h>
>  #include <signal.h>
>  #include <paths.h>
> +#include <sys/resource.h>
>  
>  #include <support/capture_subprocess.h>
>  #include <support/check.h>
> @@ -194,6 +195,22 @@ do_test (void)
>      xpthread_join (long_sleep_thread);
>    }
>  
> +  {
> +    struct rlimit rlimit_orig, rlimit_new;
> +
> +    getrlimit(RLIMIT_NPROC, &rlimit_orig);

Space after function name, also we need to check if getrlimit succeed
(and return FAIL_EXIT1 otherwise).

> +
> +    /* Force failure for the system call */
> +    rlimit_new.rlim_cur = 0;
> +    rlimit_new.rlim_max = rlimit_orig.rlim_max;
> +    setrlimit(RLIMIT_NPROC, &rlimit_new);

Same as before.

> +
> +    TEST_COMPARE(system (""), -1);
> +
> +    /* Restore NPROC limit */
> +    setrlimit(RLIMIT_NPROC, &rlimit_orig);
> +  }
> +
>    TEST_COMPARE (system (""), 0);
>  
>    return 0;
> diff --git a/sysdeps/posix/system.c b/sysdeps/posix/system.c
> index be32704280..b9a8839974 100644
> --- a/sysdeps/posix/system.c
> +++ b/sysdeps/posix/system.c
> @@ -175,10 +175,14 @@ do_system (const char *line)
>        __libc_cleanup_region_end (0);
>  #endif
>      }
> +  else if (ret == EAGAIN)
> +    /* POSIX states that failure to create a child process should
> +       return -1.  */
> +    status = -1;


It seems to align with POSIX 2024, however I think it should also handle
ENOMEM (it seems that all other possible errors are due the use of namespaces,
which is not the case for system).

>    else
> -   /* POSIX states that failure to execute the shell should return
> -      as if the shell had terminated using _exit(127).  */
> -   status = W_EXITCODE (127, 0);
> +    /* POSIX states that failure to execute the shell should return
> +       as if the shell had terminated using _exit(127).  */
> +    status = W_EXITCODE (127, 0);
>  
>    /* sigaction can not fail with SIGINT/SIGQUIT used with old
>       disposition.  Same applies for sigprocmask.  */