On 210115 1351, Christian Schoenebeck wrote: > On Freitag, 15. Januar 2021 13:23:08 CET Greg Kurz wrote: > > On Thu, 14 Jan 2021 17:17:48 -0500 > > > > Alexander Bulekov <alx...@bu.edu> wrote: > > > Signed-off-by: Alexander Bulekov <alx...@bu.edu> > > > --- > > > > No changelog at all ? > > Yeah, that's indeed quite short. :) > > > > tests/qtest/fuzz/generic_fuzz_configs.h | 12 ++++++++++++ > > > 1 file changed, 12 insertions(+) > > > > > > diff --git a/tests/qtest/fuzz/generic_fuzz_configs.h > > > b/tests/qtest/fuzz/generic_fuzz_configs.h index 7fed035345..ffdb590c58 > > > 100644 > > > --- a/tests/qtest/fuzz/generic_fuzz_configs.h > > > +++ b/tests/qtest/fuzz/generic_fuzz_configs.h > > > @@ -59,6 +59,18 @@ const generic_fuzz_config predefined_configs[] = { > > > > > > .name = "virtio-mouse", > > > .args = "-machine q35 -nodefaults -device virtio-mouse", > > > .objects = "virtio*", > > > > > > + },{ > > > + .name = "virtio-9p", > > > + .args = "-machine q35 -nodefaults " > > > + "-device virtio-9p,fsdev=hshare,mount_tag=hshare " > > > + "-fsdev local,id=hshare,path=/tmp/,security_model=none", > > > > Sharing a general purpose directory like "/tmp" is definitely not a > > recommended practice. This is typically the kind of thing that I'd > > like to see documented in the changelog to help me understand ;-) > > For the (non fuzzing) 9p 'local' tests Peter wanted either an auto generated > (e.g. mkdtemp()) or at least a hard coded test path that allows a person to > easily identify where the data came from. So I guess that applies to fuzzing > as well, i.e. something like "/tmp/qemu-fuzz/9pfs/" at least. > > Also note that the existing (non fuzzing) 9p 'local' tests auto generate > individual test pathes with mkdtemp() already. This was necessary there, > because tests are often run by "make -jN ..." in which case tests were > accessing concurrently the same single test directory before. Probably less > of > a problem here, but you might consider using the same approach that > virtio-9p-test.c already does. > > Also note that 'security_model=none' is maybe Ok as a starting point for > fuzzing, but a realistic 9p config is rather 'security_model=xattr', because > 'security_model=none' requires the qemu process to be run as root to avoid > permission denied errors with any minor operation. I would expect these > fuzzing tests to mostly error out with permission denied errors early instead > of entering relevant execution pathes. >
Ah. That's good to know. I just copied the security_model from the bug report for the recent CVE (https://bugs.launchpad.net/qemu/+bug/1911666) I'll switch to mapped-xattr, in v2 -Alex > > What operations does the fuzz test perform on the device ? > > > > > + .objects = "virtio*", > > > + },{ > > > + .name = "virtio-9p-synth", > > > + .args = "-machine q35 -nodefaults " > > > + "-device virtio-9p,fsdev=hshare,mount_tag=hshare " > > > + "-fsdev synth,id=hshare", > > > + .objects = "virtio*", > > > > Not sure this is super useful since the only known use case for > > the synth fsdev driver is running the virtio-9p qtest, but > > it looks fine anyway. > > Yeah, that's ok. Maybe it raises the chance to enter some execution pathes > here and there. So I would keep the 'synth' driver config. > > > > > > },{ > > > > > > .name = "e1000", > > > .args = "-M q35 -nodefaults " > > Nice to see fuzzing coming for 9p BTW! > > Best regards, > Christian Schoenebeck > >