On 200205 1326, Darren Kenny wrote:
> On Wed, Jan 29, 2020 at 05:34:26AM +0000, Bulekov, Alexander wrote:
> > These three targets should simply fuzz reads/writes to a couple ioports,
> > but they mostly serve as examples of different ways to write targets.
> > They demonstrate using qtest and qos for fuzzing, as well as using
> > rebooting and forking to reset state, or not resetting it at all.
> >
> > Signed-off-by: Alexander Bulekov <alx...@bu.edu>
> > Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com>
>
> Reviewed-by: Darren Kenny <darren.ke...@oracle.com>
>
> A couple of nit below w.r.t. commenting on how the fuzzed data is
> being processed.
>
> > ---
> > tests/qtest/fuzz/Makefile.include | 3 +
> > tests/qtest/fuzz/i440fx_fuzz.c | 178 ++++++++++++++++++++++++++++++
> > 2 files changed, 181 insertions(+)
> > create mode 100644 tests/qtest/fuzz/i440fx_fuzz.c
> >
> > diff --git a/tests/qtest/fuzz/Makefile.include
> > b/tests/qtest/fuzz/Makefile.include
> > index e3bdd33ff4..38b8cdd9f1 100644
> > --- a/tests/qtest/fuzz/Makefile.include
> > +++ b/tests/qtest/fuzz/Makefile.include
> > @@ -6,6 +6,9 @@ fuzz-obj-y += tests/qtest/fuzz/fuzz.o # Fuzzer skeleton
> > fuzz-obj-y += tests/qtest/fuzz/fork_fuzz.o
> > fuzz-obj-y += tests/qtest/fuzz/qos_fuzz.o
> >
> > +# Targets
> > +fuzz-obj-y += tests/qtest/fuzz/i440fx_fuzz.o
> > +
> > FUZZ_CFLAGS += -I$(SRC_PATH)/tests -I$(SRC_PATH)/tests/qtest
> >
> > # Linker Script to force coverage-counters into known regions which we can
> > mark
> > diff --git a/tests/qtest/fuzz/i440fx_fuzz.c b/tests/qtest/fuzz/i440fx_fuzz.c
> > new file mode 100644
> > index 0000000000..c7791182b8
> > --- /dev/null
> > +++ b/tests/qtest/fuzz/i440fx_fuzz.c
> > @@ -0,0 +1,178 @@
> > +/*
> > + * I440FX Fuzzing Target
> > + *
> > + * Copyright Red Hat Inc., 2019
> > + *
> > + * Authors:
> > + * Alexander Bulekov <alx...@bu.edu>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or
> > later.
> > + * See the COPYING file in the top-level directory.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +
> > +#include "qemu/main-loop.h"
> > +#include "tests/qtest/libqtest.h"
> > +#include "tests/qtest/libqos/pci.h"
> > +#include "tests/qtest/libqos/pci-pc.h"
> > +#include "fuzz.h"
> > +#include "fuzz/qos_fuzz.h"
> > +#include "fuzz/fork_fuzz.h"
> > +
> > +
> > +#define I440FX_PCI_HOST_BRIDGE_CFG 0xcf8
> > +#define I440FX_PCI_HOST_BRIDGE_DATA 0xcfc
> > +
> > +enum action_id {
> > + WRITEB,
> > + WRITEW,
> > + WRITEL,
> > + READB,
> > + READW,
> > + READL,
> > + ACTION_MAX
> > +};
> > +
>
> While it eventually becomes clear what is happening in these
> functions, it does take several attempts at reading it to understand
> what is going on.
>
> For that reason, it might be worth a couple of comments for future
> maintainers as to what is going on.
Yes - I haven't touched this target in a while. It could definitely
use some comments. Since the port-io input space isn't enormous, maybe
it makes sends to fuzz all of port-io, instead of just the I440FX
addresses.
-Alex
> Thanks,
>
> Darren.
>
> > +static void i440fx_fuzz_qtest(QTestState *s,
> > + const unsigned char *Data, size_t Size) {
> > + typedef struct QTestFuzzAction {
> > + uint32_t value;
> > + uint8_t id;
> > + uint8_t addr;
> > + } QTestFuzzAction;
> > + QTestFuzzAction a;
> > +
> > + while (Size >= sizeof(a)) {
> > + memcpy(&a, Data, sizeof(a));
> > + uint16_t addr = a.addr % 2 ? I440FX_PCI_HOST_BRIDGE_CFG :
> > + I440FX_PCI_HOST_BRIDGE_DATA;
> > + switch (a.id % ACTION_MAX) {
> > + case WRITEB:
> > + qtest_outb(s, addr, (uint8_t)a.value);
> > + break;
> > + case WRITEW:
> > + qtest_outw(s, addr, (uint16_t)a.value);
> > + break;
> > + case WRITEL:
> > + qtest_outl(s, addr, (uint32_t)a.value);
> > + break;
> > + case READB:
> > + qtest_inb(s, addr);
> > + break;
> > + case READW:
> > + qtest_inw(s, addr);
> > + break;
> > + case READL:
> > + qtest_inl(s, addr);
> > + break;
> > + }
> > + Size -= sizeof(a);
> > + Data += sizeof(a);
> > + }
> > + flush_events(s);
> > +}
> > +
> > +static void i440fx_fuzz_qos(QTestState *s,
> > + const unsigned char *Data, size_t Size) {
> > +
> > + typedef struct QOSFuzzAction {
> > + uint32_t value;
> > + int devfn;
> > + uint8_t offset;
> > + uint8_t id;
> > + } QOSFuzzAction;
> > +
> > + static QPCIBus *bus;
> > + if (!bus) {
> > + bus = qpci_new_pc(s, fuzz_qos_alloc);
> > + }
> > +
> > + QOSFuzzAction a;
> > + while (Size >= sizeof(a)) {
> > + memcpy(&a, Data, sizeof(a));
> > + switch (a.id % ACTION_MAX) {
> > + case WRITEB:
> > + bus->config_writeb(bus, a.devfn, a.offset, (uint8_t)a.value);
> > + break;
> > + case WRITEW:
> > + bus->config_writew(bus, a.devfn, a.offset, (uint16_t)a.value);
> > + break;
> > + case WRITEL:
> > + bus->config_writel(bus, a.devfn, a.offset, (uint32_t)a.value);
> > + break;
> > + case READB:
> > + bus->config_readb(bus, a.devfn, a.offset);
> > + break;
> > + case READW:
> > + bus->config_readw(bus, a.devfn, a.offset);
> > + break;
> > + case READL:
> > + bus->config_readl(bus, a.devfn, a.offset);
> > + break;
> > + }
> > + Size -= sizeof(a);
> > + Data += sizeof(a);
> > + }
> > + flush_events(s);
> > +}
> > +
> > +static void i440fx_fuzz_qos_fork(QTestState *s,
> > + const unsigned char *Data, size_t Size) {
> > + if (fork() == 0) {
> > + i440fx_fuzz_qos(s, Data, Size);
> > + _Exit(0);
> > + } else {
> > + wait(NULL);
> > + }
> > +}
> > +
> > +static const char *i440fx_qtest_argv = TARGET_NAME " -machine accel=qtest"
> > + "-m 0 -display none";
> > +static const char *i440fx_argv(FuzzTarget *t)
> > +{
> > + return i440fx_qtest_argv;
> > +}
> > +
> > +static void fork_init(void)
> > +{
> > + counter_shm_init();
> > +}
> > +
> > +static void register_pci_fuzz_targets(void)
> > +{
> > + /* Uses simple qtest commands and reboots to reset state */
> > + fuzz_add_target(&(FuzzTarget){
> > + .name = "i440fx-qtest-reboot-fuzz",
> > + .description = "Fuzz the i440fx using raw qtest commands
> > and"
> > + "rebooting after each run",
> > + .get_init_cmdline = i440fx_argv,
> > + .fuzz = i440fx_fuzz_qtest});
> > +
> > + /* Uses libqos and forks to prevent state leakage */
> > + fuzz_add_qos_target(&(FuzzTarget){
> > + .name = "i440fx-qos-fork-fuzz",
> > + .description = "Fuzz the i440fx using raw qtest commands
> > and"
> > + "rebooting after each run",
> > + .pre_vm_init = &fork_init,
> > + .fuzz = i440fx_fuzz_qos_fork,},
> > + "i440FX-pcihost",
> > + &(QOSGraphTestOptions){}
> > + );
> > +
> > + /*
> > + * Uses libqos. Doesn't do anything to reset state. Note that if we
> > were to
> > + * reboot after each run, we would also have to redo the qos-related
> > + * initialization (qos_init_path)
> > + */
> > + fuzz_add_qos_target(&(FuzzTarget){
> > + .name = "i440fx-qos-noreset-fuzz",
> > + .description = "Fuzz the i440fx using raw qtest commands
> > and"
> > + "rebooting after each run",
> > + .fuzz = i440fx_fuzz_qos,},
> > + "i440FX-pcihost",
> > + &(QOSGraphTestOptions){}
> > + );
> > +}
> > +
> > +fuzz_target_init(register_pci_fuzz_targets);
> > --
> > 2.23.0
> >
> >
