[Libguestfs] [libnbd PATCH v4 0/2] lib/utils: introduce async-signal-safe execvpe()

[Laszlo Ersek]

On 3/22/23 15:45, Laszlo Ersek wrote:
> On 3/21/23 18:28, Eric Blake wrote:
> 
>> it is indeed a bug in busybox now that POSIX is moving towards
>> standardizing realpath, so I've filed it:
>> https://bugs.busybox.net/show_bug.cgi?id=15466
> 
> I've found another busybox bug.
> 
> The "/bin/sh" utility is provided by busybox as well (via the usual symlinking).
> 
> Per POSIX, if
> 
>   execvp(file, { argv[0], argv[1], ..., NULL })
> 
> were to fail with -1/ENOEXEC, then execvp() must retry "as if" with
> 
>   execv(<shell path>, { argv[0], file, argv[1], ..., NULL })
> 
> In other words, if direct execution of "file" failed because "file" "has the appropriate access permission but has an unrecognized format", then execvp() is required to try executing "file" as a shell script. For that, <shell path> is left unspecified by POSIX, but the arguments of the shell are specified:
> 
> - Argv[0] remains the same. That is, what we wanted "file" to know itself as, is what we now want *the shell executable* to know itself as.
> 
> - argv[1] becomes "file" -- this is the script that the shell is supposed to run.
> 
> - argv[2] and onwards become positional parameters $1, $2, ... for the shell script.
> 
> And the argv[0] specification is what's violated by busybox, because if argv[0] is anything other than "sh", then the busybox binary doesn't recognize itself as the shell!
> 
> The simplest way to demonstrate the bug is this:
> 
> bash-5.2$ ( exec -a foobar /bin/sh <<< "echo hello" )
> foobar: applet not found
> 
> 
> And then, another way to demonstrate the same busybox issue... lets us, in fact, discover a musl bug in turn!!!
> 
> Consider the following C program (called "test-execvp.c"; the binary is called "test-execvp"):
> 
> -------------
> #include <stdio.h>
> #include <unistd.h>
> 
> int main(void)
> {
>   char *args[] = { "foobar", NULL };
> 
>   execvp("hello.sh", args);
>   perror("execvp");
>   return 1;
> }
> -------------
> 
> The file "hello.sh" resides in the current directory (same directory where "test-execvp" resides). Furthermore it has execute permission, and the following contents:
> 
> -------
> echo hello
> -------
> 
> Now consider the following command (from bash):
> 
> $ PATH=.:$PATH test-execvp
> 
> What is supposed to happen is this:
> 
> (1) bash shall find test-execvp in the current directory per PATH,
> (2) execvp() shall find "hello.sh" in the current directory per PATH,
> (3) execvp() shall hit an internal failure -1/ENOEXEC,
> (4) execvp() shall then invoke the shell (under an unspecified pathname),
> (5) the shell shall get "foobar" for its argv[0], and "hello.sh" for its argv[1]
> (6) we shall see "hello" on the standard output.
> 
> That's exactly what happens on Linux/glibc. (Note: this result has absolutely nothing to do with my execvpe() implementation, or libnbd in the first place.)
> 
> Now, according to my above description of the busybox bug, we're tempted to believe that step (6) fails on Alpine Linux (using musl + busybox). We expect the busybox binary to be launched, via the /bin/sh symlink, in step (4), and we expect it to fail after step (5), due to it not recognizing "foobar" as an "applet name".
> 
> It turns out however that step (4) does not happen. musl does not handle ENOEXEC:
> 
>> bash-5.2$ PATH=.:$PATH test-execvp
>> execvp: Exec format error
> 
> execvp() is not permitted by POSIX to fail with ENOEXEC. (Not considering the virtually impossible situation when executing the system shell binary *itself* fails with ENOEXEC.)
> 
> I've checked the musl source too, at commit 7d756e1c04de ("dns: prefer monotonic clock for timeouts", 2023-02-12). The execvp() implementation:
> 
>   https://git.musl-libc.org/cgit/musl/tree/src/process/execvp.c
> 
> does not handle ENOEXEC; what's more, the entire tree only sets errno=ENOEXEC in "ldso/dynlink.c".

I considered reporting a musl bug, but:

- per <https://wiki.musl-libc.org/reporting-bugs.html>, musl issues
should be reported on the mailing list,

- on the mailing list, I've found two reports of this symptom, one from
2018, another from 2020:

https://www.openwall.com/lists/musl/2018/03/09/2
https://www.openwall.com/lists/musl/2020/02/12/9

So it turns out that my execvp[e]() implementation is not only
async-signal-safe, but mitigates a known bug in musl, too.

(BTW what Rich Felker seems to be missing, under the second link above,
is that execvp() is *NOT* required to be async-signal-safe (approx.
"safe to use from a vforked child"), so malloc() is fair game within
execvp(), even in case malloc() is also async-signal-UNSAFE in musl.)

Laszlo