On Tue, Feb 21, 2023 at 02:06:33PM +0100, Laszlo Ersek wrote: > 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).
There's no reason for a fifo to ever have the x bit, but a directory regularly has the search bit (which overlays the x bit) set. I've never accidentally attempted to execute a fifo, but I have, more than once, asked to execute something I thought was a file only to have it turn out to be a directory name. See also commit b17fa77b in nbdkit, where it actually hurt us (our shim script was sticking /path/to/nbdkit first in PATH, but when we had /path/to/nbdkit/bash/ as a subdirectory, we actively triggered attempts to execute the subdirectory instead of the intended /bin/bash). > > 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? I'd be happy with extending at least one of the above two tests, if that's easier than adding something new. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
