Libguestfs - Feb 2023 - [libnbd PATCH v3 10/29] lib/utils: add unit tests for async-signal-safe execvpe()

On Wed, Feb 15, 2023 at 05:03:48PM -0600, Eric Blake
wrote:
> > +
> > +# Create subdirectories for triggering non-fatal internal error
conditions of
> > +# execvpe(). (Almost) every subdirectory will contain one entry,
called "f".
> > +#
> > +# Create a directory that's empty.
> > +mkdir empty
> > +
> > +# Create a directory with a named pipe (FIFO) in it.
> > +mkdir fifo
> > +mkfifo fifo/f
> > +
> > +# Create a directory with a non-executable file in it.
> > +mkdir nxregf
> > +touch nxregf/f
> > +
> > +# Create a symlink loop.
> > +ln -s symlink symlink
Another interesting thing you might want to add to the test:

mkdir -p subdir/f

then show that PATH=...:subdir:... does not get tripped up by
subdir/f/ having the execute bit set (aka search bit since it's a
directory) but not being an executable.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

On 2/16/23 00:33, Eric Blake wrote:
> On Wed, Feb 15, 2023 at 05:03:48PM -0600, Eric Blake wrote:
>>> +
>>> +# Create subdirectories for triggering non-fatal internal error
conditions of
>>> +# execvpe(). (Almost) every subdirectory will contain one entry,
called "f".
>>> +#
>>> +# Create a directory that's empty.
>>> +mkdir empty
>>> +
>>> +# Create a directory with a named pipe (FIFO) in it.
>>> +mkdir fifo
>>> +mkfifo fifo/f
>>> +
>>> +# Create a directory with a non-executable file in it.
>>> +mkdir nxregf
>>> +touch nxregf/f
>>> +
>>> +# Create a symlink loop.
>>> +ln -s symlink symlink
> 
> Another interesting thing you might want to add to the test:
> 
> mkdir -p subdir/f
> 
> then show that PATH=...:subdir:... does not get tripped up by
> subdir/f/ having the execute bit set (aka search bit since it's a
> directory) but not being an executable.
> 
Hmm. There could be two spots to test this (because what we're
triggering here is EACCES). One, *between* the following two tests:

run "" fifo/f;    execve_fail fifo/f    EACCES
run "" nxregf/f;  execve_fail nxregf/f  EACCES

because "fifo/f" is effectively the same test as "subdir/f"
-- not a
regular file in the first place.

Two, inside:

run empty:fifo:nxregf:symlink f
execve_fail empty/f,fifo/f,nxregf/f,symlink/f ELOOP

(again inserted between the fifo and the nxregf cases).

Now, while developing the test cases, I did (somewhat unwittingly) end
up testing what you propose, but then I decided that trying to execute a
directory was not much different from trying to execute a fifo -- the x
file mode bit shouldn't matter because the file type wouldn't be right
in the first place (the x bit should only matter on regular files).

So... Do you think "subdir/f" improves code coverage? If so, do you
agree that I should extend one (or both!) of the above two tests -- or
else, should I add something entirely new?

Thanks!
Laszlo