[PATCH] posix: Fix return value of system if shell can not be executed [BZ #27053]

[Florian Weimer]

* Adhemerval Zanella via Libc-alpha:

> POSIX states that system returned code for failure to execute the shell
> shall be as if the shell had terminated using _exit(127).  This
> behaviour was removed with 5fb7fc96350575.
>
> This issue should not be possible for pclose because if the shell can
> not be executed, popen would not return a valid FILE object.  Other
> process execution failures (such as killed by a signal) would be
> returned through waitpid from pclose call.

Note that execve is not atomic and a late resource allocation failure
will be reported as a termination by a signal (because the previous
process image is gone at that point).

> The new tst-system has an underlying issue once new containers tests
> are added that might issue the minimal container shell, since it
> changes the shell execution mode.  It is not an issue now, since it is
> the only container tests for stdlib subforder, and I would like to

Typo: “subforder”

> avoid setting non parallel execution.  Another possibility if
> concurrent container tests issues the minimum shell would be to
> add a advisory lock.

I think you should replace “issue[s]” with different words in a few
places.  Right now, the wording is quite confusing.

If the test is destructive to the testroot, I think there is already a
way to mark it as such.

> ---
>  stdlib/tst-system.c    | 17 +++++++++++++++++
>  support/Makefile       |  1 +
>  support/xchmod.c       | 30 ++++++++++++++++++++++++++++++
>  support/xunistd.h      |  1 +
>  sysdeps/posix/system.c |  4 ++++
>  5 files changed, 53 insertions(+)
>  create mode 100644 support/xchmod.c

support/ changes should be committed separately.

> diff --git a/support/xunistd.h b/support/xunistd.h
> index b299db77ba..29b9a028b0 100644
> --- a/support/xunistd.h
> +++ b/support/xunistd.h
> @@ -45,6 +45,7 @@ long xsysconf (int name);
>  long long xlseek (int fd, long long offset, int whence);
>  void xftruncate (int fd, long long length);
>  void xsymlink (const char *target, const char *linkpath);
> +void xchmod (const char *pathname, mode_t mode);
>  
>  /* Equivalent of "mkdir -p".  */
>  void xmkdirp (const char *, mode_t);
> diff --git a/sysdeps/posix/system.c b/sysdeps/posix/system.c
> index a03f478fc7..b86fc9b756 100644
> --- a/sysdeps/posix/system.c
> +++ b/sysdeps/posix/system.c
> @@ -175,6 +175,10 @@ do_system (const char *line)
>        __libc_cleanup_region_end (0);
>  #endif
>      }
> +  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);
>  
>    DO_LOCK ();
>    if (SUB_REF () == 0)

This still sets errno, but I think this is okay.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill