Re: [PATCH 2/2] linux: posix_spawn: return EINVAL on argc < 1

[Joseph Myers <joseph@codesourcery.com>]

On Fri, 28 Jan 2022, Adhemerval Zanella via Libc-alpha wrote:

> Since Linux is discussing changing on execve syscall [1], I think it would be
> better to the same not only for posix_spawn, but rather to all execve
> functions.
> 
> And since all ends up calling execve, even posix_spawn, I think it would be
> better to:
> 
>   1. Make posix/execve.c call __execveat.
>     1.1. It would also allow remove the Hurd implementation 'sysdeps/mach/hurd/execve.c'
>   2. Add the proper check on generic, Linux, and Hurd implementation execveat.
>     2.1. Maybe even add __execveat_internal that just issue the syscall and let the
>          generic wrapper handle the argument parsing.
>   3. Add a regression test.
> 
> I also think returning EINVAL is better than the kernel EFAULT one (it 
> seems that the last message on thread does settle for that).

Apart from the need for a test, a note under "Deprecated and removed 
features, and other changes affecting compatibility" in NEWS, and 
documentation in the manual if there's an appropriate place for it to go 
(there is for execve; posix_spawn isn't documented in the manual at all), 
I'd also think it would be better in terms of application compatibility to 
construct an array { pathname, NULL } and pass that in place of argv when 
argv[0] is NULL (*not* when argv[0] is an empty string, I don't see a 
problem with an empty string there), rather than returning an error.  
That's more similar in spirit to what we do with reopening fds 0, 1, 2 if 
not open at startup (but I'd also tend to think the kernel is a better 
place than libc to deal with this, given that anything the *calling* 
program does in userspace with execve can't avoid security issues in the 
*called* program with NULL argv[0] - in the case of fds 0, 1, 2 glibc is 
addressing the problem state directly in the *called* process).

I don't think the "should" in the POSIX specification of posix_spawn is 
very relevant as a justification for the patch (it only requires things 
for Strictly Conforming POSIX Applications).

-- 
Joseph S. Myers
joseph@codesourcery.com