On Wed, Oct 14, 2020 at 10:29:41AM +0300, Dima Stepanov wrote: > On Tue, Oct 13, 2020 at 11:30:52AM -0400, Alexander Bulekov wrote: > > On 201007 1647, Dima Stepanov wrote: > > > The virtio-blk fuzz target sets up and fuzzes the available virtio-blk > > > queues. The implementation is based on two files: > > > - tests/qtest/fuzz/virtio_scsi_fuzz.c > > > - tests/qtest/virtio_blk_test.c > > > > > > Signed-off-by: Dima Stepanov <[email protected]> > > > --- > > > tests/qtest/fuzz/meson.build | 1 + > > > tests/qtest/fuzz/virtio_blk_fuzz.c | 234 > > > +++++++++++++++++++++++++++++++++++++ > > > 2 files changed, 235 insertions(+) > > > create mode 100644 tests/qtest/fuzz/virtio_blk_fuzz.c > > > > > > diff --git a/tests/qtest/fuzz/meson.build b/tests/qtest/fuzz/meson.build > > > index b31ace7..3b923dc 100644 > > > --- a/tests/qtest/fuzz/meson.build > > > +++ b/tests/qtest/fuzz/meson.build > > > @@ -5,6 +5,7 @@ specific_fuzz_ss.add(files('fuzz.c', 'fork_fuzz.c', > > > 'qos_fuzz.c', > > > specific_fuzz_ss.add(when: 'CONFIG_I440FX', if_true: > > > files('i440fx_fuzz.c')) > > > specific_fuzz_ss.add(when: 'CONFIG_VIRTIO_NET', if_true: > > > files('virtio_net_fuzz.c')) > > > specific_fuzz_ss.add(when: 'CONFIG_VIRTIO_SCSI', if_true: > > > files('virtio_scsi_fuzz.c')) > > > +specific_fuzz_ss.add(files('virtio_blk_fuzz.c')) > > > > Hi Dima, > > For consistency, maybe > > specific_fuzz_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: > > files('virtio_blk_fuzz.c')) > Good point, will update it. > > > > ... > > > + > > > +static char *drive_create(void) > > > +{ > > > + int fd, ret; > > > + char *t_path = g_strdup("/tmp/qtest.XXXXXX"); > > > + > > > + /* Create a temporary raw image */ > > > + fd = mkstemp(t_path); > > > + g_assert_cmpint(fd, >=, 0); > > > + ret = ftruncate(fd, TEST_IMAGE_SIZE); > > > + g_assert_cmpint(ret, ==, 0); > > > + close(fd); > > > + > > > + g_test_queue_destroy(drive_destroy, t_path); > > > + return t_path; > > > +} > > > + > > > > I tested this out and it works with multi-process fuzzing under -jobs=4 > > -workers=4 (this initialization happens after libfuzzer has already > > forked the processes). This seems like an interesting alternative to > > using fake null-co:// files. > > I wonder if some state might leak as these disks are filled with fuzzer > > data. > Yes, i've also chosen between the fake null device and temporary file. > Tried this approach, just to see what will happen ). It seems to me that > slightly different paths can be triggered in this case and it is closer > to real usage. > But indeed, mb some state can leak, this is interesting. > > > > > Nit: these disk files remain after the fuzzer exists. It looks > > like the libfuzzer people suggest simply using atexit() to perform > > cleanup: https://reviews.llvm.org/D45762 > > The is that the only way I have found to terminate the fuzzer is with > > SIGKILL, where atexit is skipped. QEMU installs some signal handlers in > > os-posix.c:os_setup_signal_handling to notify the main_loop that the > > qemu was killed. Since we replace qemu_main_loop by manually running > > main_loop_wait, we don't check main_loop_should_exit(). > Got it! Thanks for sharing this is good to know ). > > No other comments mixed in below. > > Dima. > > > > I sent a patch to disable QEMU's signal handlers for the fuzzer. > > Message-Id: <[email protected]> Sorry, i couldn't find a patch you've pointed out above. Could you share some link to it? Also, am i correct that it is a general change for the QEMU fuzzing, so all the fuzzing targets will automatically reuse it?
> > > > With an atexit() call to clean up the temporary images: > > Reviewed-by: Alexander Bulekov <[email protected]> > > > > > +static void *virtio_blk_test_setup(GString *cmd_line, void *arg) > > > +{ > > > + char *tmp_path = drive_create(); > > > + > > > + g_string_append_printf(cmd_line, > > > + " -drive if=none,id=drive0,file=%s," > > > + "format=raw,auto-read-only=off ", > > > + tmp_path); > > > + > > > + return arg; > > > +} > > > + > > > +static void register_virtio_blk_fuzz_targets(void) > > > +{ > > > + fuzz_add_qos_target(&(FuzzTarget){ > > > + .name = "virtio-blk-fuzz", > > > + .description = "Fuzz the virtio-blk virtual queues, > > > forking " > > > + "for each fuzz run", > > > + .pre_vm_init = &counter_shm_init, > > > + .pre_fuzz = &virtio_blk_pre_fuzz, > > > + .fuzz = virtio_blk_fork_fuzz,}, > > > + "virtio-blk", > > > + &(QOSGraphTestOptions){.before = virtio_blk_test_setup} > > > + ); > > > + > > > + fuzz_add_qos_target(&(FuzzTarget){ > > > + .name = "virtio-blk-flags-fuzz", > > > + .description = "Fuzz the virtio-blk virtual queues, > > > forking " > > > + "for each fuzz run (also fuzzes the virtio flags)", > > > + .pre_vm_init = &counter_shm_init, > > > + .pre_fuzz = &virtio_blk_pre_fuzz, > > > + .fuzz = virtio_blk_with_flag_fuzz,}, > > > + "virtio-blk", > > > + &(QOSGraphTestOptions){.before = virtio_blk_test_setup} > > > + ); > > > +} > > > + > > > +fuzz_target_init(register_virtio_blk_fuzz_targets); > > > -- > > > 2.7.4 > > >
