From 9ab4fb21f5b336138757912f68bae1bf450c23b5 Mon Sep 17 00:00:00 2001 From: Chen Qun Date: Fri, 23 Oct 2020 14:12:12 +0800 Subject: tests/migration: fix memleak in wait_command/wait_command_fd Properly free each command resp to avoid memory leak. ASAN shows memory leak stack: Indirect leak of 2352520 byte(s) in 571 object(s) allocated from: #0 0x7f6ca3308d4e in __interceptor_calloc (/lib64/libasan.so.5+0x112d4e) #1 0x7f6ca3127a50 in g_malloc0 (/lib64/libglib-2.0.so.0+0x55a50) #2 0x557bf3c71d2b in qdict_new ../qobject/qdict.c:29 #3 0x557bf3c9caba in parse_object ../qobject/json-parser.c:318 #4 0x557bf3c9ce75 in json_parser_parse ../qobject/json-parser.c:580 #5 0x557bf3c8c8cf in json_message_process_token ../qobject/json-streamer.c:92 #6 0x557bf3c9ea59 in json_lexer_feed_char ../qobject/json-lexer.c:313 #7 0x557bf3c9eeb5 in json_lexer_feed ../qobject/json-lexer.c:350 #8 0x557bf3c4793a in qmp_fd_receive ../tests/qtest/libqtest.c:608 #9 0x557bf3c47b58 in qtest_qmp_receive ../tests/qtest/libqtest.c:618 #10 0x557bf3c44245 in wait_command ../tests/qtest/migration-helpers.c:59 #11 0x557bf3c445cb in migrate_query_status ../tests/qtest/migration-helpers.c:108 #12 0x557bf3c44642 in check_migration_status ../tests/qtest/migration-helpers.c:124 #13 0x557bf3c447e7 in wait_for_migration_status ../tests/qtest/migration-helpers.c:148 #14 0x557bf3c43b8f in test_migrate_auto_converge ../tests/qtest/migration-test.c:1243 ...... Fix: 5e34005571af5 Reported-by: Euler Robot Signed-off-by: Chen Qun Message-Id: <20201023061218.2080844-2-kuhn.chenqun@huawei.com> Signed-off-by: Thomas Huth --- tests/qtest/migration-helpers.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c index b799dba..4ee2601 100644 --- a/tests/qtest/migration-helpers.c +++ b/tests/qtest/migration-helpers.c @@ -32,7 +32,7 @@ static void check_stop_event(QTestState *who) QDict *wait_command_fd(QTestState *who, int fd, const char *command, ...) { va_list ap; - QDict *resp; + QDict *resp, *ret; va_start(ap, command); qtest_qmp_vsend_fds(who, &fd, 1, command, ap); @@ -44,7 +44,11 @@ QDict *wait_command_fd(QTestState *who, int fd, const char *command, ...) g_assert(!qdict_haskey(resp, "error")); g_assert(qdict_haskey(resp, "return")); - return qdict_get_qdict(resp, "return"); + ret = qdict_get_qdict(resp, "return"); + qobject_ref(ret); + qobject_unref(resp); + + return ret; } /* @@ -53,7 +57,7 @@ QDict *wait_command_fd(QTestState *who, int fd, const char *command, ...) QDict *wait_command(QTestState *who, const char *command, ...) { va_list ap; - QDict *resp; + QDict *resp, *ret; va_start(ap, command); resp = qtest_vqmp(who, command, ap); @@ -64,7 +68,11 @@ QDict *wait_command(QTestState *who, const char *command, ...) g_assert(!qdict_haskey(resp, "error")); g_assert(qdict_haskey(resp, "return")); - return qdict_get_qdict(resp, "return"); + ret = qdict_get_qdict(resp, "return"); + qobject_ref(ret); + qobject_unref(resp); + + return ret; } /* -- cgit v1.1 From b7f47e82e2d8ccf368d70fc4fd1467db55a74a32 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Wed, 30 Sep 2020 13:13:52 +0200 Subject: tests/qtest/libqtest: Fix detection of architecture for binaries without path The qtests can be run directly by specifying the QEMU binary with the QTEST_QEMU_BINARY environment variable, for example: $ QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 tests/qtest/test-hmp However, if you specify a binary without a path, for example with QTEST_QEMU_BINARY=qemu-system-x86_64 if the QEMU binary is in your $PATH, then the test currently simply crashes. Let's try a little bit smarter here by looking for the final '-' instead of the slash. Message-Id: <20201012114816.43546-1-thuth@redhat.com> Signed-off-by: Thomas Huth --- tests/qtest/libqtest.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c index 08929f5..b9ff290 100644 --- a/tests/qtest/libqtest.c +++ b/tests/qtest/libqtest.c @@ -870,9 +870,14 @@ char *qtest_hmp(QTestState *s, const char *fmt, ...) const char *qtest_get_arch(void) { const char *qemu = qtest_qemu_binary(); - const char *end = strrchr(qemu, '/'); + const char *end = strrchr(qemu, '-'); - return end + strlen("/qemu-system-"); + if (!end) { + fprintf(stderr, "Can't determine architecture from binary name.\n"); + abort(); + } + + return end + 1; } bool qtest_get_irq(QTestState *s, int num) -- cgit v1.1 From a6b0882ca75b62b2c0840578f790ab65844b749e Mon Sep 17 00:00:00 2001 From: Jason Andryuk Date: Tue, 13 Oct 2020 10:05:09 -0400 Subject: accel: Remove _WIN32 ifdef from qtest-cpus.c dummy-cpus.c is only compiled with CONFIG_POSIX, so the _WIN32 condition will never evaluate true. Remove it. Signed-off-by: Jason Andryuk Message-Id: <20201013140511.5681-2-jandryuk@gmail.com> Acked-by: Paolo Bonzini Reviewed-by: Claudio Fontana Reviewed-by: Thomas Huth Signed-off-by: Thomas Huth --- accel/qtest/qtest-cpus.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/accel/qtest/qtest-cpus.c b/accel/qtest/qtest-cpus.c index 7c5399e..db09420 100644 --- a/accel/qtest/qtest-cpus.c +++ b/accel/qtest/qtest-cpus.c @@ -29,10 +29,6 @@ static void *qtest_cpu_thread_fn(void *arg) { -#ifdef _WIN32 - error_report("qtest is not supported under Windows"); - exit(1); -#else CPUState *cpu = arg; sigset_t waitset; int r; @@ -69,7 +65,6 @@ static void *qtest_cpu_thread_fn(void *arg) qemu_mutex_unlock_iothread(); rcu_unregister_thread(); return NULL; -#endif } static void qtest_start_vcpu_thread(CPUState *cpu) -- cgit v1.1 From 9ce84a0d17d015f059a6750fbbf4b057806751df Mon Sep 17 00:00:00 2001 From: Jason Andryuk Date: Tue, 13 Oct 2020 10:05:10 -0400 Subject: accel: move qtest CpusAccel functions to a common location Move and rename accel/qtest/qtest-cpus.c files to accel/dummy-cpus.c so it can be re-used by Xen. Signed-off-by: Jason Andryuk Message-Id: <20201013140511.5681-3-jandryuk@gmail.com> Reviewed-by: Claudio Fontana Acked-by: Paolo Bonzini Signed-off-by: Thomas Huth --- accel/dummy-cpus.c | 72 ++++++++++++++++++++++++++++++++++++++++ accel/meson.build | 7 ++++ accel/qtest/meson.build | 1 - accel/qtest/qtest-cpus.c | 86 ------------------------------------------------ accel/qtest/qtest-cpus.h | 17 ---------- accel/qtest/qtest.c | 5 ++- include/sysemu/cpus.h | 3 ++ 7 files changed, 86 insertions(+), 105 deletions(-) create mode 100644 accel/dummy-cpus.c delete mode 100644 accel/qtest/qtest-cpus.c delete mode 100644 accel/qtest/qtest-cpus.h diff --git a/accel/dummy-cpus.c b/accel/dummy-cpus.c new file mode 100644 index 0000000..10429fd --- /dev/null +++ b/accel/dummy-cpus.c @@ -0,0 +1,72 @@ +/* + * Dummy cpu thread code + * + * Copyright IBM, Corp. 2011 + * + * Authors: + * Anthony Liguori + * + * 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/rcu.h" +#include "sysemu/cpus.h" +#include "qemu/guest-random.h" +#include "qemu/main-loop.h" +#include "hw/core/cpu.h" + +static void *dummy_cpu_thread_fn(void *arg) +{ + CPUState *cpu = arg; + sigset_t waitset; + int r; + + rcu_register_thread(); + + qemu_mutex_lock_iothread(); + qemu_thread_get_self(cpu->thread); + cpu->thread_id = qemu_get_thread_id(); + cpu->can_do_io = 1; + current_cpu = cpu; + + sigemptyset(&waitset); + sigaddset(&waitset, SIG_IPI); + + /* signal CPU creation */ + cpu_thread_signal_created(cpu); + qemu_guest_random_seed_thread_part2(cpu->random_seed); + + do { + qemu_mutex_unlock_iothread(); + do { + int sig; + r = sigwait(&waitset, &sig); + } while (r == -1 && (errno == EAGAIN || errno == EINTR)); + if (r == -1) { + perror("sigwait"); + exit(1); + } + qemu_mutex_lock_iothread(); + qemu_wait_io_event(cpu); + } while (!cpu->unplug); + + qemu_mutex_unlock_iothread(); + rcu_unregister_thread(); + return NULL; +} + +void dummy_start_vcpu_thread(CPUState *cpu) +{ + char thread_name[VCPU_THREAD_NAME_SIZE]; + + cpu->thread = g_malloc0(sizeof(QemuThread)); + cpu->halt_cond = g_malloc0(sizeof(QemuCond)); + qemu_cond_init(cpu->halt_cond); + snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/DUMMY", + cpu->cpu_index); + qemu_thread_create(cpu->thread, thread_name, dummy_cpu_thread_fn, cpu, + QEMU_THREAD_JOINABLE); +} diff --git a/accel/meson.build b/accel/meson.build index bb00d0f..9a41739 100644 --- a/accel/meson.build +++ b/accel/meson.build @@ -5,3 +5,10 @@ subdir('kvm') subdir('tcg') subdir('xen') subdir('stubs') + +dummy_ss = ss.source_set() +dummy_ss.add(files( + 'dummy-cpus.c', +)) + +specific_ss.add_all(when: ['CONFIG_SOFTMMU', 'CONFIG_POSIX'], if_true: dummy_ss) diff --git a/accel/qtest/meson.build b/accel/qtest/meson.build index e477cb2..a2f3276 100644 --- a/accel/qtest/meson.build +++ b/accel/qtest/meson.build @@ -1,7 +1,6 @@ qtest_ss = ss.source_set() qtest_ss.add(files( 'qtest.c', - 'qtest-cpus.c', )) specific_ss.add_all(when: ['CONFIG_SOFTMMU', 'CONFIG_POSIX'], if_true: qtest_ss) diff --git a/accel/qtest/qtest-cpus.c b/accel/qtest/qtest-cpus.c deleted file mode 100644 index db09420..0000000 --- a/accel/qtest/qtest-cpus.c +++ /dev/null @@ -1,86 +0,0 @@ -/* - * QTest accelerator code - * - * Copyright IBM, Corp. 2011 - * - * Authors: - * Anthony Liguori - * - * 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/rcu.h" -#include "qapi/error.h" -#include "qemu/module.h" -#include "qemu/option.h" -#include "qemu/config-file.h" -#include "sysemu/accel.h" -#include "sysemu/qtest.h" -#include "sysemu/cpus.h" -#include "sysemu/cpu-timers.h" -#include "qemu/guest-random.h" -#include "qemu/main-loop.h" -#include "hw/core/cpu.h" - -#include "qtest-cpus.h" - -static void *qtest_cpu_thread_fn(void *arg) -{ - CPUState *cpu = arg; - sigset_t waitset; - int r; - - rcu_register_thread(); - - qemu_mutex_lock_iothread(); - qemu_thread_get_self(cpu->thread); - cpu->thread_id = qemu_get_thread_id(); - cpu->can_do_io = 1; - current_cpu = cpu; - - sigemptyset(&waitset); - sigaddset(&waitset, SIG_IPI); - - /* signal CPU creation */ - cpu_thread_signal_created(cpu); - qemu_guest_random_seed_thread_part2(cpu->random_seed); - - do { - qemu_mutex_unlock_iothread(); - do { - int sig; - r = sigwait(&waitset, &sig); - } while (r == -1 && (errno == EAGAIN || errno == EINTR)); - if (r == -1) { - perror("sigwait"); - exit(1); - } - qemu_mutex_lock_iothread(); - qemu_wait_io_event(cpu); - } while (!cpu->unplug); - - qemu_mutex_unlock_iothread(); - rcu_unregister_thread(); - return NULL; -} - -static void qtest_start_vcpu_thread(CPUState *cpu) -{ - char thread_name[VCPU_THREAD_NAME_SIZE]; - - cpu->thread = g_malloc0(sizeof(QemuThread)); - cpu->halt_cond = g_malloc0(sizeof(QemuCond)); - qemu_cond_init(cpu->halt_cond); - snprintf(thread_name, VCPU_THREAD_NAME_SIZE, "CPU %d/DUMMY", - cpu->cpu_index); - qemu_thread_create(cpu->thread, thread_name, qtest_cpu_thread_fn, cpu, - QEMU_THREAD_JOINABLE); -} - -const CpusAccel qtest_cpus = { - .create_vcpu_thread = qtest_start_vcpu_thread, - .get_virtual_clock = qtest_get_virtual_clock, -}; diff --git a/accel/qtest/qtest-cpus.h b/accel/qtest/qtest-cpus.h deleted file mode 100644 index 739519a..0000000 --- a/accel/qtest/qtest-cpus.h +++ /dev/null @@ -1,17 +0,0 @@ -/* - * Accelerator CPUS Interface - * - * Copyright 2020 SUSE LLC - * - * This work is licensed under the terms of the GNU GPL, version 2 or later. - * See the COPYING file in the top-level directory. - */ - -#ifndef QTEST_CPUS_H -#define QTEST_CPUS_H - -#include "sysemu/cpus.h" - -extern const CpusAccel qtest_cpus; - -#endif /* QTEST_CPUS_H */ diff --git a/accel/qtest/qtest.c b/accel/qtest/qtest.c index 537e8b4..b282cea 100644 --- a/accel/qtest/qtest.c +++ b/accel/qtest/qtest.c @@ -25,7 +25,10 @@ #include "qemu/main-loop.h" #include "hw/core/cpu.h" -#include "qtest-cpus.h" +const CpusAccel qtest_cpus = { + .create_vcpu_thread = dummy_start_vcpu_thread, + .get_virtual_clock = qtest_get_virtual_clock, +}; static int qtest_init_accel(MachineState *ms) { diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h index 2316859..e815672 100644 --- a/include/sysemu/cpus.h +++ b/include/sysemu/cpus.h @@ -25,6 +25,9 @@ typedef struct CpusAccel { /* register accel-specific cpus interface implementation */ void cpus_register_accel(const CpusAccel *i); +/* Create a dummy vcpu for CpusAccel->create_vcpu_thread */ +void dummy_start_vcpu_thread(CPUState *); + /* interface available for cpus accelerator threads */ /* For temporary buffers for forming a name */ -- cgit v1.1 From efd4d93b530807921b1940e13990c561530618d3 Mon Sep 17 00:00:00 2001 From: Jason Andryuk Date: Tue, 13 Oct 2020 10:05:11 -0400 Subject: accel: Add xen CpusAccel using dummy-cpus Xen was broken by commit 1583a3898853 ("cpus: extract out qtest-specific code to accel/qtest"). Xen relied on qemu_init_vcpu() calling qemu_dummy_start_vcpu() in the default case, but that was replaced by g_assert_not_reached(). Add a minimal "CpusAccel" for Xen using the dummy-cpus implementation used by qtest. Signed-off-by: Jason Andryuk Message-Id: <20201013140511.5681-4-jandryuk@gmail.com> Acked-by: Paolo Bonzini Reviewed-by: Claudio Fontana Acked-by: Anthony PERARD Signed-off-by: Thomas Huth --- accel/meson.build | 1 + accel/xen/xen-all.c | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/accel/meson.build b/accel/meson.build index 9a41739..b26cca2 100644 --- a/accel/meson.build +++ b/accel/meson.build @@ -12,3 +12,4 @@ dummy_ss.add(files( )) specific_ss.add_all(when: ['CONFIG_SOFTMMU', 'CONFIG_POSIX'], if_true: dummy_ss) +specific_ss.add_all(when: ['CONFIG_XEN'], if_true: dummy_ss) diff --git a/accel/xen/xen-all.c b/accel/xen/xen-all.c index 60b971d..878a408 100644 --- a/accel/xen/xen-all.c +++ b/accel/xen/xen-all.c @@ -16,6 +16,7 @@ #include "hw/xen/xen_pt.h" #include "chardev/char.h" #include "sysemu/accel.h" +#include "sysemu/cpus.h" #include "sysemu/xen.h" #include "sysemu/runstate.h" #include "migration/misc.h" @@ -153,6 +154,10 @@ static void xen_setup_post(MachineState *ms, AccelState *accel) } } +const CpusAccel xen_cpus = { + .create_vcpu_thread = dummy_start_vcpu_thread, +}; + static int xen_init(MachineState *ms) { MachineClass *mc = MACHINE_GET_CLASS(ms); @@ -180,6 +185,9 @@ static int xen_init(MachineState *ms) * opt out of system RAM being allocated by generic code */ mc->default_ram_id = NULL; + + cpus_register_accel(&xen_cpus); + return 0; } -- cgit v1.1 From 288c31e30d522dbac4d7998ca254735393c59307 Mon Sep 17 00:00:00 2001 From: Havard Skinnemoen Date: Fri, 23 Oct 2020 14:06:32 -0700 Subject: tests/qtest: Make npcm7xx_timer-test conditional on CONFIG_NPCM7XX This test won't work if qemu was compiled without CONFIG_NPCM7XX, as pointed out by Thomas Huth on a different patch. Signed-off-by: Havard Skinnemoen Message-Id: <20201023210637.351238-2-hskinnemoen@google.com> Signed-off-by: Thomas Huth --- tests/qtest/meson.build | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build index 28d4068..7e0ecaa 100644 --- a/tests/qtest/meson.build +++ b/tests/qtest/meson.build @@ -133,12 +133,13 @@ qtests_sparc64 = \ (config_all_devices.has_key('CONFIG_ISA_TESTDEV') ? ['endianness-test'] : []) + \ ['prom-env-test', 'boot-serial-test'] +qtests_npcm7xx = ['npcm7xx_timer-test'] qtests_arm = \ (config_all_devices.has_key('CONFIG_PFLASH_CFI02') ? ['pflash-cfi02-test'] : []) + \ + (config_all_devices.has_key('CONFIG_NPCM7XX') ? qtests_npcm7xx : []) + \ ['arm-cpu-features', 'microbit-test', 'm25p80-test', - 'npcm7xx_timer-test', 'test-arm-mptimer', 'boot-serial-test', 'hexloader-test'] -- cgit v1.1 From 7f9d519c0d37b8af0b228a4ed49d33ea095e9eb7 Mon Sep 17 00:00:00 2001 From: Maxim Levitsky Date: Mon, 19 Oct 2020 19:37:00 +0300 Subject: libqtest: fix the order of buffered events By a mistake I added the pending events in a wrong order. Fix this by using g_list_append. Signed-off-by: Maxim Levitsky Message-Id: <20201019163702.471239-3-mlevitsk@redhat.com> Signed-off-by: Thomas Huth --- tests/qtest/libqtest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c index b9ff290..0e304bd 100644 --- a/tests/qtest/libqtest.c +++ b/tests/qtest/libqtest.c @@ -621,7 +621,7 @@ QDict *qtest_qmp_receive(QTestState *s) return response; } /* Stash the event for a later consumption */ - s->pending_events = g_list_prepend(s->pending_events, response); + s->pending_events = g_list_append(s->pending_events, response); } } -- cgit v1.1 From d232b87ec6e3a8a04db9b647f61a1e3a6855a58f Mon Sep 17 00:00:00 2001 From: Maxim Levitsky Date: Mon, 19 Oct 2020 19:37:01 +0300 Subject: libqtest: fix memory leak in the qtest_qmp_event_ref The g_list_remove_link doesn't free the link element, opposed to what I thought. Switch to g_list_delete_link that does free it. Also refactor the code a bit. Thanks for Max Reitz for helping me with this. Signed-off-by: Maxim Levitsky Message-Id: <20201019163702.471239-4-mlevitsk@redhat.com> Signed-off-by: Thomas Huth --- tests/qtest/libqtest.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c index 0e304bd..99deff4 100644 --- a/tests/qtest/libqtest.c +++ b/tests/qtest/libqtest.c @@ -795,15 +795,12 @@ void qtest_qmp_send_raw(QTestState *s, const char *fmt, ...) QDict *qtest_qmp_event_ref(QTestState *s, const char *event) { - GList *next = NULL; - QDict *response; - - for (GList *it = s->pending_events; it != NULL; it = next) { + while (s->pending_events) { - next = it->next; - response = (QDict *)it->data; + GList *first = s->pending_events; + QDict *response = (QDict *)first->data; - s->pending_events = g_list_remove_link(s->pending_events, it); + s->pending_events = g_list_delete_link(s->pending_events, first); if (!strcmp(qdict_get_str(response, "event"), event)) { return response; -- cgit v1.1 From fb5ef4eeecd88b583d5a6dc8f7dc217179cbfc98 Mon Sep 17 00:00:00 2001 From: Alexander Bulekov Date: Fri, 23 Oct 2020 11:07:30 -0400 Subject: memory: Add FlatView foreach function Acked-by: Paolo Bonzini Reviewed-by: Darren Kenny Signed-off-by: Alexander Bulekov Message-Id: <20201023150746.107063-2-alxndr@bu.edu> Signed-off-by: Thomas Huth --- include/exec/memory.h | 5 +++++ softmmu/memory.c | 13 +++++++++++++ 2 files changed, 18 insertions(+) diff --git a/include/exec/memory.h b/include/exec/memory.h index 622207b..042918d 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -719,6 +719,11 @@ static inline FlatView *address_space_to_flatview(AddressSpace *as) return qatomic_rcu_read(&as->current_map); } +typedef int (*flatview_cb)(Int128 start, + Int128 len, + const MemoryRegion*, void*); + +void flatview_for_each_range(FlatView *fv, flatview_cb cb , void *opaque); /** * struct MemoryRegionSection: describes a fragment of a #MemoryRegion diff --git a/softmmu/memory.c b/softmmu/memory.c index 403ff3a..a5d1641 100644 --- a/softmmu/memory.c +++ b/softmmu/memory.c @@ -656,6 +656,19 @@ static void render_memory_region(FlatView *view, } } +void flatview_for_each_range(FlatView *fv, flatview_cb cb , void *opaque) +{ + FlatRange *fr; + + assert(fv); + assert(cb); + + FOR_EACH_FLAT_RANGE(fr, fv) { + if (cb(fr->addr.start, fr->addr.size, fr->mr, opaque)) + break; + } +} + static MemoryRegion *memory_region_get_flatview_root(MemoryRegion *mr) { while (mr->enabled) { -- cgit v1.1 From da9bf5319838c193f92a3444bd3258b32c606980 Mon Sep 17 00:00:00 2001 From: Alexander Bulekov Date: Fri, 23 Oct 2020 11:07:31 -0400 Subject: fuzz: Add generic virtual-device fuzzer This is a generic fuzzer designed to fuzz a virtual device's MemoryRegions, as long as they exist within the Memory or Port IO (if it exists) AddressSpaces. The fuzzer's input is interpreted into a sequence of qtest commands (outb, readw, etc). The interpreted commands are separated by a magic seaparator, which should be easy for the fuzzer to guess. Without ASan, the separator can be specified as a "dictionary value" using the -dict argument (see libFuzzer documentation). Reviewed-by: Darren Kenny Signed-off-by: Alexander Bulekov Message-Id: <20201023150746.107063-3-alxndr@bu.edu> Signed-off-by: Thomas Huth --- tests/qtest/fuzz/generic_fuzz.c | 516 ++++++++++++++++++++++++++++++++++++++++ tests/qtest/fuzz/meson.build | 1 + 2 files changed, 517 insertions(+) create mode 100644 tests/qtest/fuzz/generic_fuzz.c diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c new file mode 100644 index 0000000..6e3faf4 --- /dev/null +++ b/tests/qtest/fuzz/generic_fuzz.c @@ -0,0 +1,516 @@ +/* + * Generic Virtual-Device Fuzzing Target + * + * Copyright Red Hat Inc., 2020 + * + * Authors: + * Alexander Bulekov + * + * 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 + +#include "hw/core/cpu.h" +#include "tests/qtest/libqos/libqtest.h" +#include "fuzz.h" +#include "fork_fuzz.h" +#include "exec/address-spaces.h" +#include "string.h" +#include "exec/memory.h" +#include "exec/ramblock.h" +#include "exec/address-spaces.h" +#include "hw/qdev-core.h" + +/* + * SEPARATOR is used to separate "operations" in the fuzz input + */ +#define SEPARATOR "FUZZ" + +enum cmds { + OP_IN, + OP_OUT, + OP_READ, + OP_WRITE, + OP_CLOCK_STEP, +}; + +#define DEFAULT_TIMEOUT_US 100000 +#define USEC_IN_SEC 1000000000 + +typedef struct { + ram_addr_t addr; + ram_addr_t size; /* The number of bytes until the end of the I/O region */ +} address_range; + +static useconds_t timeout = DEFAULT_TIMEOUT_US; + +static bool qtest_log_enabled; + +/* + * List of memory regions that are children of QOM objects specified by the + * user for fuzzing. + */ +static GHashTable *fuzzable_memoryregions; + +struct get_io_cb_info { + int index; + int found; + address_range result; +}; + +static int get_io_address_cb(Int128 start, Int128 size, + const MemoryRegion *mr, void *opaque) { + struct get_io_cb_info *info = opaque; + if (g_hash_table_lookup(fuzzable_memoryregions, mr)) { + if (info->index == 0) { + info->result.addr = (ram_addr_t)start; + info->result.size = (ram_addr_t)size; + info->found = 1; + return 1; + } + info->index--; + } + return 0; +} + +/* + * Here we want to convert a fuzzer-provided [io-region-index, offset] to + * a physical address. To do this, we iterate over all of the matched + * MemoryRegions. Check whether each region exists within the particular io + * space. Return the absolute address of the offset within the index'th region + * that is a subregion of the io_space and the distance until the end of the + * memory region. + */ +static bool get_io_address(address_range *result, AddressSpace *as, + uint8_t index, + uint32_t offset) { + FlatView *view; + view = as->current_map; + g_assert(view); + struct get_io_cb_info cb_info = {}; + + cb_info.index = index; + + /* + * Loop around the FlatView until we match "index" number of + * fuzzable_memoryregions, or until we know that there are no matching + * memory_regions. + */ + do { + flatview_for_each_range(view, get_io_address_cb , &cb_info); + } while (cb_info.index != index && !cb_info.found); + + *result = cb_info.result; + return cb_info.found; +} + +static bool get_pio_address(address_range *result, + uint8_t index, uint16_t offset) +{ + /* + * PIO BARs can be set past the maximum port address (0xFFFF). Thus, result + * can contain an addr that extends past the PIO space. When we pass this + * address to qtest_in/qtest_out, it is cast to a uint16_t, so we might end + * up fuzzing a completely different MemoryRegion/Device. Therefore, check + * that the address here is within the PIO space limits. + */ + bool found = get_io_address(result, &address_space_io, index, offset); + return result->addr <= 0xFFFF ? found : false; +} + +static bool get_mmio_address(address_range *result, + uint8_t index, uint32_t offset) +{ + return get_io_address(result, &address_space_memory, index, offset); +} + +static void op_in(QTestState *s, const unsigned char * data, size_t len) +{ + enum Sizes {Byte, Word, Long, end_sizes}; + struct { + uint8_t size; + uint8_t base; + uint16_t offset; + } a; + address_range abs; + + if (len < sizeof(a)) { + return; + } + memcpy(&a, data, sizeof(a)); + if (get_pio_address(&abs, a.base, a.offset) == 0) { + return; + } + + switch (a.size %= end_sizes) { + case Byte: + qtest_inb(s, abs.addr); + break; + case Word: + if (abs.size >= 2) { + qtest_inw(s, abs.addr); + } + break; + case Long: + if (abs.size >= 4) { + qtest_inl(s, abs.addr); + } + break; + } +} + +static void op_out(QTestState *s, const unsigned char * data, size_t len) +{ + enum Sizes {Byte, Word, Long, end_sizes}; + struct { + uint8_t size; + uint8_t base; + uint16_t offset; + uint32_t value; + } a; + address_range abs; + + if (len < sizeof(a)) { + return; + } + memcpy(&a, data, sizeof(a)); + + if (get_pio_address(&abs, a.base, a.offset) == 0) { + return; + } + + switch (a.size %= end_sizes) { + case Byte: + qtest_outb(s, abs.addr, a.value & 0xFF); + break; + case Word: + if (abs.size >= 2) { + qtest_outw(s, abs.addr, a.value & 0xFFFF); + } + break; + case Long: + if (abs.size >= 4) { + qtest_outl(s, abs.addr, a.value); + } + break; + } +} + +static void op_read(QTestState *s, const unsigned char * data, size_t len) +{ + enum Sizes {Byte, Word, Long, Quad, end_sizes}; + struct { + uint8_t size; + uint8_t base; + uint32_t offset; + } a; + address_range abs; + + if (len < sizeof(a)) { + return; + } + memcpy(&a, data, sizeof(a)); + + if (get_mmio_address(&abs, a.base, a.offset) == 0) { + return; + } + + switch (a.size %= end_sizes) { + case Byte: + qtest_readb(s, abs.addr); + break; + case Word: + if (abs.size >= 2) { + qtest_readw(s, abs.addr); + } + break; + case Long: + if (abs.size >= 4) { + qtest_readl(s, abs.addr); + } + break; + case Quad: + if (abs.size >= 8) { + qtest_readq(s, abs.addr); + } + break; + } +} + +static void op_write(QTestState *s, const unsigned char * data, size_t len) +{ + enum Sizes {Byte, Word, Long, Quad, end_sizes}; + struct { + uint8_t size; + uint8_t base; + uint32_t offset; + uint64_t value; + } a; + address_range abs; + + if (len < sizeof(a)) { + return; + } + memcpy(&a, data, sizeof(a)); + + if (get_mmio_address(&abs, a.base, a.offset) == 0) { + return; + } + + switch (a.size %= end_sizes) { + case Byte: + qtest_writeb(s, abs.addr, a.value & 0xFF); + break; + case Word: + if (abs.size >= 2) { + qtest_writew(s, abs.addr, a.value & 0xFFFF); + } + break; + case Long: + if (abs.size >= 4) { + qtest_writel(s, abs.addr, a.value & 0xFFFFFFFF); + } + break; + case Quad: + if (abs.size >= 8) { + qtest_writeq(s, abs.addr, a.value); + } + break; + } +} + +static void op_clock_step(QTestState *s, const unsigned char *data, size_t len) +{ + qtest_clock_step_next(s); +} + +static void handle_timeout(int sig) +{ + if (qtest_log_enabled) { + fprintf(stderr, "[Timeout]\n"); + fflush(stderr); + } + _Exit(0); +} + +/* + * Here, we interpret random bytes from the fuzzer, as a sequence of commands. + * Some commands can be variable-width, so we use a separator, SEPARATOR, to + * specify the boundaries between commands. SEPARATOR is used to separate + * "operations" in the fuzz input. Why use a separator, instead of just using + * the operations' length to identify operation boundaries? + * 1. This is a simple way to support variable-length operations + * 2. This adds "stability" to the input. + * For example take the input "AbBcgDefg", where there is no separator and + * Opcodes are capitalized. + * Simply, by removing the first byte, we end up with a very different + * sequence: + * BbcGdefg... + * By adding a separator, we avoid this problem: + * Ab SEP Bcg SEP Defg -> B SEP Bcg SEP Defg + * Since B uses two additional bytes as operands, the first "B" will be + * ignored. The fuzzer actively tries to reduce inputs, so such unused + * bytes are likely to be pruned, eventually. + * + * SEPARATOR is trivial for the fuzzer to discover when using ASan. Optionally, + * SEPARATOR can be manually specified as a dictionary value (see libfuzzer's + * -dict), though this should not be necessary. + * + * As a result, the stream of bytes is converted into a sequence of commands. + * In a simplified example where SEPARATOR is 0xFF: + * 00 01 02 FF 03 04 05 06 FF 01 FF ... + * becomes this sequence of commands: + * 00 01 02 -> op00 (0102) -> in (0102, 2) + * 03 04 05 06 -> op03 (040506) -> write (040506, 3) + * 01 -> op01 (-,0) -> out (-,0) + * ... + * + * Note here that it is the job of the individual opcode functions to check + * that enough data was provided. I.e. in the last command out (,0), out needs + * to check that there is not enough data provided to select an address/value + * for the operation. + */ +static void generic_fuzz(QTestState *s, const unsigned char *Data, size_t Size) +{ + void (*ops[]) (QTestState *s, const unsigned char* , size_t) = { + [OP_IN] = op_in, + [OP_OUT] = op_out, + [OP_READ] = op_read, + [OP_WRITE] = op_write, + [OP_CLOCK_STEP] = op_clock_step, + }; + const unsigned char *cmd = Data; + const unsigned char *nextcmd; + size_t cmd_len; + uint8_t op; + + if (fork() == 0) { + /* + * Sometimes the fuzzer will find inputs that take quite a long time to + * process. Often times, these inputs do not result in new coverage. + * Even if these inputs might be interesting, they can slow down the + * fuzzer, overall. Set a timeout to avoid hurting performance, too much + */ + if (timeout) { + struct sigaction sact; + struct itimerval timer; + + sigemptyset(&sact.sa_mask); + sact.sa_flags = SA_NODEFER; + sact.sa_handler = handle_timeout; + sigaction(SIGALRM, &sact, NULL); + + memset(&timer, 0, sizeof(timer)); + timer.it_value.tv_sec = timeout / USEC_IN_SEC; + timer.it_value.tv_usec = timeout % USEC_IN_SEC; + setitimer(ITIMER_VIRTUAL, &timer, NULL); + } + + while (cmd && Size) { + /* Get the length until the next command or end of input */ + nextcmd = memmem(cmd, Size, SEPARATOR, strlen(SEPARATOR)); + cmd_len = nextcmd ? nextcmd - cmd : Size; + + if (cmd_len > 0) { + /* Interpret the first byte of the command as an opcode */ + op = *cmd % (sizeof(ops) / sizeof((ops)[0])); + ops[op](s, cmd + 1, cmd_len - 1); + + /* Run the main loop */ + flush_events(s); + } + /* Advance to the next command */ + cmd = nextcmd ? nextcmd + sizeof(SEPARATOR) - 1 : nextcmd; + Size = Size - (cmd_len + sizeof(SEPARATOR) - 1); + } + _Exit(0); + } else { + flush_events(s); + wait(0); + } +} + +static void usage(void) +{ + printf("Please specify the following environment variables:\n"); + printf("QEMU_FUZZ_ARGS= the command line arguments passed to qemu\n"); + printf("QEMU_FUZZ_OBJECTS= " + "a space separated list of QOM type names for objects to fuzz\n"); + printf("Optionally: QEMU_FUZZ_TIMEOUT= Specify a custom timeout (us). " + "0 to disable. %d by default\n", timeout); + exit(0); +} + +static int locate_fuzz_memory_regions(Object *child, void *opaque) +{ + const char *name; + MemoryRegion *mr; + if (object_dynamic_cast(child, TYPE_MEMORY_REGION)) { + mr = MEMORY_REGION(child); + if ((memory_region_is_ram(mr) || + memory_region_is_ram_device(mr) || + memory_region_is_rom(mr)) == false) { + name = object_get_canonical_path_component(child); + /* + * We don't want duplicate pointers to the same MemoryRegion, so + * try to remove copies of the pointer, before adding it. + */ + g_hash_table_insert(fuzzable_memoryregions, mr, (gpointer)true); + } + } + return 0; +} + +static int locate_fuzz_objects(Object *child, void *opaque) +{ + char *pattern = opaque; + if (g_pattern_match_simple(pattern, object_get_typename(child))) { + /* Find and save ptrs to any child MemoryRegions */ + object_child_foreach_recursive(child, locate_fuzz_memory_regions, NULL); + + } else if (object_dynamic_cast(OBJECT(child), TYPE_MEMORY_REGION)) { + if (g_pattern_match_simple(pattern, + object_get_canonical_path_component(child))) { + MemoryRegion *mr; + mr = MEMORY_REGION(child); + if ((memory_region_is_ram(mr) || + memory_region_is_ram_device(mr) || + memory_region_is_rom(mr)) == false) { + g_hash_table_insert(fuzzable_memoryregions, mr, (gpointer)true); + } + } + } + return 0; +} + +static void generic_pre_fuzz(QTestState *s) +{ + GHashTableIter iter; + MemoryRegion *mr; + char **result; + + if (!getenv("QEMU_FUZZ_OBJECTS")) { + usage(); + } + if (getenv("QTEST_LOG")) { + qtest_log_enabled = 1; + } + if (getenv("QEMU_FUZZ_TIMEOUT")) { + timeout = g_ascii_strtoll(getenv("QEMU_FUZZ_TIMEOUT"), NULL, 0); + } + + fuzzable_memoryregions = g_hash_table_new(NULL, NULL); + + result = g_strsplit(getenv("QEMU_FUZZ_OBJECTS"), " ", -1); + for (int i = 0; result[i] != NULL; i++) { + printf("Matching objects by name %s\n", result[i]); + object_child_foreach_recursive(qdev_get_machine(), + locate_fuzz_objects, + result[i]); + } + g_strfreev(result); + printf("This process will try to fuzz the following MemoryRegions:\n"); + + g_hash_table_iter_init(&iter, fuzzable_memoryregions); + while (g_hash_table_iter_next(&iter, (gpointer)&mr, NULL)) { + printf(" * %s (size %lx)\n", + object_get_canonical_path_component(&(mr->parent_obj)), + (uint64_t)mr->size); + } + + if (!g_hash_table_size(fuzzable_memoryregions)) { + printf("No fuzzable memory regions found...\n"); + exit(1); + } + + counter_shm_init(); +} + +static GString *generic_fuzz_cmdline(FuzzTarget *t) +{ + GString *cmd_line = g_string_new(TARGET_NAME); + if (!getenv("QEMU_FUZZ_ARGS")) { + usage(); + } + g_string_append_printf(cmd_line, " -display none \ + -machine accel=qtest, \ + -m 512M %s ", getenv("QEMU_FUZZ_ARGS")); + return cmd_line; +} + +static void register_generic_fuzz_targets(void) +{ + fuzz_add_target(&(FuzzTarget){ + .name = "generic-fuzz", + .description = "Fuzz based on any qemu command-line args. ", + .get_init_cmdline = generic_fuzz_cmdline, + .pre_fuzz = generic_pre_fuzz, + .fuzz = generic_fuzz, + }); +} + +fuzz_target_init(register_generic_fuzz_targets); diff --git a/tests/qtest/fuzz/meson.build b/tests/qtest/fuzz/meson.build index b31ace7..5162321 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('generic_fuzz.c')) fork_fuzz = declare_dependency( link_args: config_host['FUZZ_EXE_LDFLAGS'].split() + -- cgit v1.1 From 05efbf2497f93415a50347bbf53983689f999282 Mon Sep 17 00:00:00 2001 From: Alexander Bulekov Date: Fri, 23 Oct 2020 11:07:32 -0400 Subject: fuzz: Add PCI features to the generic fuzzer This patch compares TYPE_PCI_DEVICE objects against the user-provided matching pattern. If there is a match, we use some hacks and leverage QOS to map each possible BAR for that device. Now fuzzed inputs might be converted to pci_read/write commands which target specific. This means that we can fuzz a particular device's PCI configuration space, Signed-off-by: Alexander Bulekov Reviewed-by: Darren Kenny Message-Id: <20201023150746.107063-4-alxndr@bu.edu> Signed-off-by: Thomas Huth --- tests/qtest/fuzz/generic_fuzz.c | 81 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c index 6e3faf4..483d41f 100644 --- a/tests/qtest/fuzz/generic_fuzz.c +++ b/tests/qtest/fuzz/generic_fuzz.c @@ -24,6 +24,7 @@ #include "exec/ramblock.h" #include "exec/address-spaces.h" #include "hw/qdev-core.h" +#include "hw/pci/pci.h" /* * SEPARATOR is used to separate "operations" in the fuzz input @@ -35,12 +36,17 @@ enum cmds { OP_OUT, OP_READ, OP_WRITE, + OP_PCI_READ, + OP_PCI_WRITE, OP_CLOCK_STEP, }; #define DEFAULT_TIMEOUT_US 100000 #define USEC_IN_SEC 1000000000 +#define PCI_HOST_BRIDGE_CFG 0xcf8 +#define PCI_HOST_BRIDGE_DATA 0xcfc + typedef struct { ram_addr_t addr; ram_addr_t size; /* The number of bytes until the end of the I/O region */ @@ -55,6 +61,7 @@ static bool qtest_log_enabled; * user for fuzzing. */ static GHashTable *fuzzable_memoryregions; +static GPtrArray *fuzzable_pci_devices; struct get_io_cb_info { int index; @@ -283,6 +290,65 @@ static void op_write(QTestState *s, const unsigned char * data, size_t len) } } +static void op_pci_read(QTestState *s, const unsigned char * data, size_t len) +{ + enum Sizes {Byte, Word, Long, end_sizes}; + struct { + uint8_t size; + uint8_t base; + uint8_t offset; + } a; + if (len < sizeof(a) || fuzzable_pci_devices->len == 0) { + return; + } + memcpy(&a, data, sizeof(a)); + PCIDevice *dev = g_ptr_array_index(fuzzable_pci_devices, + a.base % fuzzable_pci_devices->len); + int devfn = dev->devfn; + qtest_outl(s, PCI_HOST_BRIDGE_CFG, (1U << 31) | (devfn << 8) | a.offset); + switch (a.size %= end_sizes) { + case Byte: + qtest_inb(s, PCI_HOST_BRIDGE_DATA); + break; + case Word: + qtest_inw(s, PCI_HOST_BRIDGE_DATA); + break; + case Long: + qtest_inl(s, PCI_HOST_BRIDGE_DATA); + break; + } +} + +static void op_pci_write(QTestState *s, const unsigned char * data, size_t len) +{ + enum Sizes {Byte, Word, Long, end_sizes}; + struct { + uint8_t size; + uint8_t base; + uint8_t offset; + uint32_t value; + } a; + if (len < sizeof(a) || fuzzable_pci_devices->len == 0) { + return; + } + memcpy(&a, data, sizeof(a)); + PCIDevice *dev = g_ptr_array_index(fuzzable_pci_devices, + a.base % fuzzable_pci_devices->len); + int devfn = dev->devfn; + qtest_outl(s, PCI_HOST_BRIDGE_CFG, (1U << 31) | (devfn << 8) | a.offset); + switch (a.size %= end_sizes) { + case Byte: + qtest_outb(s, PCI_HOST_BRIDGE_DATA, a.value & 0xFF); + break; + case Word: + qtest_outw(s, PCI_HOST_BRIDGE_DATA, a.value & 0xFFFF); + break; + case Long: + qtest_outl(s, PCI_HOST_BRIDGE_DATA, a.value & 0xFFFFFFFF); + break; + } +} + static void op_clock_step(QTestState *s, const unsigned char *data, size_t len) { qtest_clock_step_next(s); @@ -341,6 +407,8 @@ static void generic_fuzz(QTestState *s, const unsigned char *Data, size_t Size) [OP_OUT] = op_out, [OP_READ] = op_read, [OP_WRITE] = op_write, + [OP_PCI_READ] = op_pci_read, + [OP_PCI_WRITE] = op_pci_write, [OP_CLOCK_STEP] = op_clock_step, }; const unsigned char *cmd = Data; @@ -432,6 +500,18 @@ static int locate_fuzz_objects(Object *child, void *opaque) /* Find and save ptrs to any child MemoryRegions */ object_child_foreach_recursive(child, locate_fuzz_memory_regions, NULL); + /* + * We matched an object. If its a PCI device, store a pointer to it so + * we can map BARs and fuzz its config space. + */ + if (object_dynamic_cast(OBJECT(child), TYPE_PCI_DEVICE)) { + /* + * Don't want duplicate pointers to the same PCIDevice, so remove + * copies of the pointer, before adding it. + */ + g_ptr_array_remove_fast(fuzzable_pci_devices, PCI_DEVICE(child)); + g_ptr_array_add(fuzzable_pci_devices, PCI_DEVICE(child)); + } } else if (object_dynamic_cast(OBJECT(child), TYPE_MEMORY_REGION)) { if (g_pattern_match_simple(pattern, object_get_canonical_path_component(child))) { @@ -464,6 +544,7 @@ static void generic_pre_fuzz(QTestState *s) } fuzzable_memoryregions = g_hash_table_new(NULL, NULL); + fuzzable_pci_devices = g_ptr_array_new(); result = g_strsplit(getenv("QEMU_FUZZ_OBJECTS"), " ", -1); for (int i = 0; result[i] != NULL; i++) { -- cgit v1.1 From 20f5a3029386363357e6fa0c2e82b35ac4914d6a Mon Sep 17 00:00:00 2001 From: Alexander Bulekov Date: Fri, 23 Oct 2020 11:07:33 -0400 Subject: fuzz: Add DMA support to the generic-fuzzer When a virtual-device tries to access some buffer in memory over DMA, we add call-backs into the fuzzer(next commit). The fuzzer checks verifies that the DMA request maps to a physical RAM address and fills the memory with fuzzer-provided data. The patterns that we use to fill this memory are specified using add_dma_pattern and clear_dma_patterns operations. Signed-off-by: Alexander Bulekov Reviewed-by: Darren Kenny Message-Id: <20201023150746.107063-5-alxndr@bu.edu> [thuth: Reformatted one comment according to the QEMU coding style] Signed-off-by: Thomas Huth --- include/exec/memory.h | 7 ++ tests/qtest/fuzz/generic_fuzz.c | 230 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 237 insertions(+) diff --git a/include/exec/memory.h b/include/exec/memory.h index 042918d..93d27bf 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -42,6 +42,13 @@ typedef struct IOMMUMemoryRegionClass IOMMUMemoryRegionClass; DECLARE_OBJ_CHECKERS(IOMMUMemoryRegion, IOMMUMemoryRegionClass, IOMMU_MEMORY_REGION, TYPE_IOMMU_MEMORY_REGION) +#ifdef CONFIG_FUZZ +void fuzz_dma_read_cb(size_t addr, + size_t len, + MemoryRegion *mr, + bool is_write); +#endif + extern bool global_dirty_log; typedef struct MemoryRegionOps MemoryRegionOps; diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c index 483d41f..62a94de 100644 --- a/tests/qtest/fuzz/generic_fuzz.c +++ b/tests/qtest/fuzz/generic_fuzz.c @@ -25,6 +25,7 @@ #include "exec/address-spaces.h" #include "hw/qdev-core.h" #include "hw/pci/pci.h" +#include "hw/boards.h" /* * SEPARATOR is used to separate "operations" in the fuzz input @@ -38,12 +39,16 @@ enum cmds { OP_WRITE, OP_PCI_READ, OP_PCI_WRITE, + OP_ADD_DMA_PATTERN, + OP_CLEAR_DMA_PATTERNS, OP_CLOCK_STEP, }; #define DEFAULT_TIMEOUT_US 100000 #define USEC_IN_SEC 1000000000 +#define MAX_DMA_FILL_SIZE 0x10000 + #define PCI_HOST_BRIDGE_CFG 0xcf8 #define PCI_HOST_BRIDGE_DATA 0xcfc @@ -57,6 +62,24 @@ static useconds_t timeout = DEFAULT_TIMEOUT_US; static bool qtest_log_enabled; /* + * A pattern used to populate a DMA region or perform a memwrite. This is + * useful for e.g. populating tables of unique addresses. + * Example {.index = 1; .stride = 2; .len = 3; .data = "\x00\x01\x02"} + * Renders as: 00 01 02 00 03 02 00 05 02 00 07 02 ... + */ +typedef struct { + uint8_t index; /* Index of a byte to increment by stride */ + uint8_t stride; /* Increment each index'th byte by this amount */ + size_t len; + const uint8_t *data; +} pattern; + +/* Avoid filling the same DMA region between MMIO/PIO commands ? */ +static bool avoid_double_fetches; + +static QTestState *qts_global; /* Need a global for the DMA callback */ + +/* * List of memory regions that are children of QOM objects specified by the * user for fuzzing. */ @@ -85,6 +108,169 @@ static int get_io_address_cb(Int128 start, Int128 size, } /* + * List of dma regions populated since the last fuzzing command. Used to ensure + * that we only write to each DMA address once, to avoid race conditions when + * building reproducers. + */ +static GArray *dma_regions; + +static GArray *dma_patterns; +static int dma_pattern_index; + +/* + * Allocate a block of memory and populate it with a pattern. + */ +static void *pattern_alloc(pattern p, size_t len) +{ + int i; + uint8_t *buf = g_malloc(len); + uint8_t sum = 0; + + for (i = 0; i < len; ++i) { + buf[i] = p.data[i % p.len]; + if ((i % p.len) == p.index) { + buf[i] += sum; + sum += p.stride; + } + } + return buf; +} + +static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr) +{ + unsigned access_size_max = mr->ops->valid.max_access_size; + + /* + * Regions are assumed to support 1-4 byte accesses unless + * otherwise specified. + */ + if (access_size_max == 0) { + access_size_max = 4; + } + + /* Bound the maximum access by the alignment of the address. */ + if (!mr->ops->impl.unaligned) { + unsigned align_size_max = addr & -addr; + if (align_size_max != 0 && align_size_max < access_size_max) { + access_size_max = align_size_max; + } + } + + /* Don't attempt accesses larger than the maximum. */ + if (l > access_size_max) { + l = access_size_max; + } + l = pow2floor(l); + + return l; +} + +/* + * Call-back for functions that perform DMA reads from guest memory. Confirm + * that the region has not already been populated since the last loop in + * generic_fuzz(), avoiding potential race-conditions, which we don't have + * a good way for reproducing right now. + */ +void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr, bool is_write) +{ + /* Are we in the generic-fuzzer or are we using another fuzz-target? */ + if (!qts_global) { + return; + } + + /* + * Return immediately if: + * - We have no DMA patterns defined + * - The length of the DMA read request is zero + * - The DMA read is hitting an MR other than the machine's main RAM + * - The DMA request is not a read (what happens for a address_space_map + * with is_write=True? Can the device use the same pointer to do reads?) + * - The DMA request hits past the bounds of our RAM + */ + if (dma_patterns->len == 0 + || len == 0 + /* || mr != MACHINE(qdev_get_machine())->ram */ + || is_write + || addr > current_machine->ram_size) { + return; + } + + /* + * If we overlap with any existing dma_regions, split the range and only + * populate the non-overlapping parts. + */ + address_range region; + bool double_fetch = false; + for (int i = 0; + i < dma_regions->len && (avoid_double_fetches || qtest_log_enabled); + ++i) { + region = g_array_index(dma_regions, address_range, i); + if (addr < region.addr + region.size && addr + len > region.addr) { + double_fetch = true; + if (addr < region.addr + && avoid_double_fetches) { + fuzz_dma_read_cb(addr, region.addr - addr, mr, is_write); + } + if (addr + len > region.addr + region.size + && avoid_double_fetches) { + fuzz_dma_read_cb(region.addr + region.size, + addr + len - (region.addr + region.size), mr, is_write); + } + return; + } + } + + /* Cap the length of the DMA access to something reasonable */ + len = MIN(len, MAX_DMA_FILL_SIZE); + + address_range ar = {addr, len}; + g_array_append_val(dma_regions, ar); + pattern p = g_array_index(dma_patterns, pattern, dma_pattern_index); + void *buf = pattern_alloc(p, ar.size); + hwaddr l, addr1; + MemoryRegion *mr1; + uint8_t *ram_ptr; + while (len > 0) { + l = len; + mr1 = address_space_translate(first_cpu->as, + addr, &addr1, &l, true, + MEMTXATTRS_UNSPECIFIED); + + if (!(memory_region_is_ram(mr1) || + memory_region_is_romd(mr1))) { + l = memory_access_size(mr1, l, addr1); + } else { + /* ROM/RAM case */ + ram_ptr = qemu_map_ram_ptr(mr1->ram_block, addr1); + memcpy(ram_ptr, buf, l); + break; + } + len -= l; + buf += l; + addr += l; + + } + if (qtest_log_enabled) { + /* + * With QTEST_LOG, use a normal, slow QTest memwrite. Prefix the log + * that will be written by qtest.c with a DMA tag, so we can reorder + * the resulting QTest trace so the DMA fills precede the last PIO/MMIO + * command. + */ + fprintf(stderr, "[DMA] "); + if (double_fetch) { + fprintf(stderr, "[DOUBLE-FETCH] "); + } + fflush(stderr); + } + qtest_memwrite(qts_global, ar.addr, buf, ar.size); + g_free(buf); + + /* Increment the index of the pattern for the next DMA access */ + dma_pattern_index = (dma_pattern_index + 1) % dma_patterns->len; +} + +/* * Here we want to convert a fuzzer-provided [io-region-index, offset] to * a physical address. To do this, we iterate over all of the matched * MemoryRegions. Check whether each region exists within the particular io @@ -349,6 +535,35 @@ static void op_pci_write(QTestState *s, const unsigned char * data, size_t len) } } +static void op_add_dma_pattern(QTestState *s, + const unsigned char *data, size_t len) +{ + struct { + /* + * index and stride can be used to increment the index-th byte of the + * pattern by the value stride, for each loop of the pattern. + */ + uint8_t index; + uint8_t stride; + } a; + + if (len < sizeof(a) + 1) { + return; + } + memcpy(&a, data, sizeof(a)); + pattern p = {a.index, a.stride, len - sizeof(a), data + sizeof(a)}; + p.index = a.index % p.len; + g_array_append_val(dma_patterns, p); + return; +} + +static void op_clear_dma_patterns(QTestState *s, + const unsigned char *data, size_t len) +{ + g_array_set_size(dma_patterns, 0); + dma_pattern_index = 0; +} + static void op_clock_step(QTestState *s, const unsigned char *data, size_t len) { qtest_clock_step_next(s); @@ -409,6 +624,8 @@ static void generic_fuzz(QTestState *s, const unsigned char *Data, size_t Size) [OP_WRITE] = op_write, [OP_PCI_READ] = op_pci_read, [OP_PCI_WRITE] = op_pci_write, + [OP_ADD_DMA_PATTERN] = op_add_dma_pattern, + [OP_CLEAR_DMA_PATTERNS] = op_clear_dma_patterns, [OP_CLOCK_STEP] = op_clock_step, }; const unsigned char *cmd = Data; @@ -438,6 +655,8 @@ static void generic_fuzz(QTestState *s, const unsigned char *Data, size_t Size) setitimer(ITIMER_VIRTUAL, &timer, NULL); } + op_clear_dma_patterns(s, NULL, 0); + while (cmd && Size) { /* Get the length until the next command or end of input */ nextcmd = memmem(cmd, Size, SEPARATOR, strlen(SEPARATOR)); @@ -454,6 +673,7 @@ static void generic_fuzz(QTestState *s, const unsigned char *Data, size_t Size) /* Advance to the next command */ cmd = nextcmd ? nextcmd + sizeof(SEPARATOR) - 1 : nextcmd; Size = Size - (cmd_len + sizeof(SEPARATOR) - 1); + g_array_set_size(dma_regions, 0); } _Exit(0); } else { @@ -468,6 +688,9 @@ static void usage(void) printf("QEMU_FUZZ_ARGS= the command line arguments passed to qemu\n"); printf("QEMU_FUZZ_OBJECTS= " "a space separated list of QOM type names for objects to fuzz\n"); + printf("Optionally: QEMU_AVOID_DOUBLE_FETCH= " + "Try to avoid racy DMA double fetch bugs? %d by default\n", + avoid_double_fetches); printf("Optionally: QEMU_FUZZ_TIMEOUT= Specify a custom timeout (us). " "0 to disable. %d by default\n", timeout); exit(0); @@ -539,9 +762,16 @@ static void generic_pre_fuzz(QTestState *s) if (getenv("QTEST_LOG")) { qtest_log_enabled = 1; } + if (getenv("QEMU_AVOID_DOUBLE_FETCH")) { + avoid_double_fetches = 1; + } if (getenv("QEMU_FUZZ_TIMEOUT")) { timeout = g_ascii_strtoll(getenv("QEMU_FUZZ_TIMEOUT"), NULL, 0); } + qts_global = s; + + dma_regions = g_array_new(false, false, sizeof(address_range)); + dma_patterns = g_array_new(false, false, sizeof(pattern)); fuzzable_memoryregions = g_hash_table_new(NULL, NULL); fuzzable_pci_devices = g_ptr_array_new(); -- cgit v1.1 From e7d3222e2e07e2a1a0aac979ef1fa5e8ef59f02c Mon Sep 17 00:00:00 2001 From: Alexander Bulekov Date: Fri, 23 Oct 2020 11:07:34 -0400 Subject: fuzz: Declare DMA Read callback function This patch declares the fuzz_dma_read_cb function and uses the preprocessor and linker(weak symbols) to handle these cases: When we build softmmu/all with --enable-fuzzing, there should be no strong symbol defined for fuzz_dma_read_cb, and we link against a weak stub function. When we build softmmu/fuzz with --enable-fuzzing, we link against the strong symbol in generic_fuzz.c When we build softmmu/all without --enable-fuzzing, fuzz_dma_read_cb is an empty, inlined function. As long as we don't call any other functions when building the arguments, there should be no overhead. Signed-off-by: Alexander Bulekov Reviewed-by: Darren Kenny Message-Id: <20201023150746.107063-6-alxndr@bu.edu> Signed-off-by: Thomas Huth --- include/exec/memory.h | 8 ++++++++ softmmu/memory.c | 13 +++++++++++++ 2 files changed, 21 insertions(+) diff --git a/include/exec/memory.h b/include/exec/memory.h index 93d27bf..4aaf578 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -47,6 +47,14 @@ void fuzz_dma_read_cb(size_t addr, size_t len, MemoryRegion *mr, bool is_write); +#else +static inline void fuzz_dma_read_cb(size_t addr, + size_t len, + MemoryRegion *mr, + bool is_write) +{ + /* Do Nothing */ +} #endif extern bool global_dirty_log; diff --git a/softmmu/memory.c b/softmmu/memory.c index a5d1641..cec0e0f 100644 --- a/softmmu/memory.c +++ b/softmmu/memory.c @@ -3246,6 +3246,19 @@ void memory_region_init_rom_device(MemoryRegion *mr, vmstate_register_ram(mr, owner_dev); } +/* + * Support softmmu builds with CONFIG_FUZZ using a weak symbol and a stub for + * the fuzz_dma_read_cb callback + */ +#ifdef CONFIG_FUZZ +void __attribute__((weak)) fuzz_dma_read_cb(size_t addr, + size_t len, + MemoryRegion *mr, + bool is_write) +{ +} +#endif + static const TypeInfo memory_region_info = { .parent = TYPE_OBJECT, .name = TYPE_MEMORY_REGION, -- cgit v1.1 From a3c20e91dea6f7af64d886b05d678839b7b1a14c Mon Sep 17 00:00:00 2001 From: Alexander Bulekov Date: Fri, 23 Oct 2020 11:07:35 -0400 Subject: fuzz: Add fuzzer callbacks to DMA-read functions We should be careful to not call any functions besides fuzz_dma_read_cb. Without --enable-fuzzing, fuzz_dma_read_cb is an empty inlined function. Signed-off-by: Alexander Bulekov Reviewed-by: Darren Kenny Message-Id: <20201023150746.107063-7-alxndr@bu.edu> Signed-off-by: Thomas Huth --- include/exec/memory.h | 1 + include/exec/memory_ldst_cached.h.inc | 3 +++ memory_ldst.c.inc | 4 ++++ softmmu/memory.c | 1 + softmmu/physmem.c | 2 ++ 5 files changed, 11 insertions(+) diff --git a/include/exec/memory.h b/include/exec/memory.h index 4aaf578..aff6ef7 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -2462,6 +2462,7 @@ address_space_read_cached(MemoryRegionCache *cache, hwaddr addr, void *buf, hwaddr len) { assert(addr < cache->len && len <= cache->len - addr); + fuzz_dma_read_cb(cache->xlat + addr, len, cache->mrs.mr, false); if (likely(cache->ptr)) { memcpy(buf, cache->ptr + addr, len); return MEMTX_OK; diff --git a/include/exec/memory_ldst_cached.h.inc b/include/exec/memory_ldst_cached.h.inc index fd4bbb4..aff5740 100644 --- a/include/exec/memory_ldst_cached.h.inc +++ b/include/exec/memory_ldst_cached.h.inc @@ -28,6 +28,7 @@ static inline uint32_t ADDRESS_SPACE_LD_CACHED(l)(MemoryRegionCache *cache, hwaddr addr, MemTxAttrs attrs, MemTxResult *result) { assert(addr < cache->len && 4 <= cache->len - addr); + fuzz_dma_read_cb(cache->xlat + addr, 4, cache->mrs.mr, false); if (likely(cache->ptr)) { return LD_P(l)(cache->ptr + addr); } else { @@ -39,6 +40,7 @@ static inline uint64_t ADDRESS_SPACE_LD_CACHED(q)(MemoryRegionCache *cache, hwaddr addr, MemTxAttrs attrs, MemTxResult *result) { assert(addr < cache->len && 8 <= cache->len - addr); + fuzz_dma_read_cb(cache->xlat + addr, 8, cache->mrs.mr, false); if (likely(cache->ptr)) { return LD_P(q)(cache->ptr + addr); } else { @@ -50,6 +52,7 @@ static inline uint32_t ADDRESS_SPACE_LD_CACHED(uw)(MemoryRegionCache *cache, hwaddr addr, MemTxAttrs attrs, MemTxResult *result) { assert(addr < cache->len && 2 <= cache->len - addr); + fuzz_dma_read_cb(cache->xlat + addr, 2, cache->mrs.mr, false); if (likely(cache->ptr)) { return LD_P(uw)(cache->ptr + addr); } else { diff --git a/memory_ldst.c.inc b/memory_ldst.c.inc index c54aee4..8d45d2e 100644 --- a/memory_ldst.c.inc +++ b/memory_ldst.c.inc @@ -42,6 +42,7 @@ static inline uint32_t glue(address_space_ldl_internal, SUFFIX)(ARG1_DECL, MO_32 | devend_memop(endian), attrs); } else { /* RAM case */ + fuzz_dma_read_cb(addr, 4, mr, false); ptr = qemu_map_ram_ptr(mr->ram_block, addr1); switch (endian) { case DEVICE_LITTLE_ENDIAN: @@ -110,6 +111,7 @@ static inline uint64_t glue(address_space_ldq_internal, SUFFIX)(ARG1_DECL, MO_64 | devend_memop(endian), attrs); } else { /* RAM case */ + fuzz_dma_read_cb(addr, 8, mr, false); ptr = qemu_map_ram_ptr(mr->ram_block, addr1); switch (endian) { case DEVICE_LITTLE_ENDIAN: @@ -175,6 +177,7 @@ uint32_t glue(address_space_ldub, SUFFIX)(ARG1_DECL, r = memory_region_dispatch_read(mr, addr1, &val, MO_8, attrs); } else { /* RAM case */ + fuzz_dma_read_cb(addr, 1, mr, false); ptr = qemu_map_ram_ptr(mr->ram_block, addr1); val = ldub_p(ptr); r = MEMTX_OK; @@ -212,6 +215,7 @@ static inline uint32_t glue(address_space_lduw_internal, SUFFIX)(ARG1_DECL, MO_16 | devend_memop(endian), attrs); } else { /* RAM case */ + fuzz_dma_read_cb(addr, 2, mr, false); ptr = qemu_map_ram_ptr(mr->ram_block, addr1); switch (endian) { case DEVICE_LITTLE_ENDIAN: diff --git a/softmmu/memory.c b/softmmu/memory.c index cec0e0f..ee4a6bc 100644 --- a/softmmu/memory.c +++ b/softmmu/memory.c @@ -1433,6 +1433,7 @@ MemTxResult memory_region_dispatch_read(MemoryRegion *mr, unsigned size = memop_size(op); MemTxResult r; + fuzz_dma_read_cb(addr, size, mr, false); if (!memory_region_access_valid(mr, addr, size, false, attrs)) { *pval = unassigned_mem_read(mr, addr, size); return MEMTX_DECODE_ERROR; diff --git a/softmmu/physmem.c b/softmmu/physmem.c index e319fb2..a9adedb 100644 --- a/softmmu/physmem.c +++ b/softmmu/physmem.c @@ -2832,6 +2832,7 @@ MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr, stn_he_p(buf, l, val); } else { /* RAM case */ + fuzz_dma_read_cb(addr, len, mr, false); ram_ptr = qemu_ram_ptr_length(mr->ram_block, addr1, &l, false); memcpy(buf, ram_ptr, l); } @@ -3192,6 +3193,7 @@ void *address_space_map(AddressSpace *as, memory_region_ref(mr); *plen = flatview_extend_translation(fv, addr, len, mr, xlat, l, is_write, attrs); + fuzz_dma_read_cb(addr, *plen, mr, is_write); ptr = qemu_ram_ptr_length(mr->ram_block, xlat, plen, true); return ptr; -- cgit v1.1 From f81cb729be3268d84bd5755dd6ce934972a5ac8d Mon Sep 17 00:00:00 2001 From: Alexander Bulekov Date: Fri, 23 Oct 2020 11:07:36 -0400 Subject: fuzz: Add support for custom crossover functions libfuzzer supports a "custom crossover function". Libfuzzer often tries to blend two inputs to create a new interesting input. Sometimes, we have a better idea about how to blend inputs together. This change allows fuzzers to specify a custom function for blending two inputs together. Signed-off-by: Alexander Bulekov Reviewed-by: Darren Kenny Message-Id: <20201023150746.107063-8-alxndr@bu.edu> Signed-off-by: Thomas Huth --- tests/qtest/fuzz/fuzz.c | 13 +++++++++++++ tests/qtest/fuzz/fuzz.h | 27 +++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/tests/qtest/fuzz/fuzz.c b/tests/qtest/fuzz/fuzz.c index eb00704..7be7226 100644 --- a/tests/qtest/fuzz/fuzz.c +++ b/tests/qtest/fuzz/fuzz.c @@ -118,6 +118,19 @@ static FuzzTarget *fuzz_get_target(char* name) } +/* Sometimes called by libfuzzer to mutate two inputs into one */ +size_t LLVMFuzzerCustomCrossOver(const uint8_t *data1, size_t size1, + const uint8_t *data2, size_t size2, + uint8_t *out, size_t max_out_size, + unsigned int seed) +{ + if (fuzz_target->crossover) { + return fuzz_target->crossover(data1, size1, data2, size2, out, + max_out_size, seed); + } + return 0; +} + /* Executed for each fuzzing-input */ int LLVMFuzzerTestOneInput(const unsigned char *Data, size_t Size) { diff --git a/tests/qtest/fuzz/fuzz.h b/tests/qtest/fuzz/fuzz.h index 8eb765e..ed9ce17 100644 --- a/tests/qtest/fuzz/fuzz.h +++ b/tests/qtest/fuzz/fuzz.h @@ -77,6 +77,29 @@ typedef struct FuzzTarget { */ void(*fuzz)(QTestState *, const unsigned char *, size_t); + /* + * The fuzzer can specify a "Custom Crossover" function for combining two + * inputs from the corpus. This function is sometimes called by libfuzzer + * when mutating inputs. + * + * data1: location of first input + * size1: length of first input + * data1: location of second input + * size1: length of second input + * out: where to place the resulting, mutated input + * max_out_size: the maximum length of the input that can be placed in out + * seed: the seed that should be used to make mutations deterministic, when + * needed + * + * See libfuzzer's LLVMFuzzerCustomCrossOver API for more info. + * + * Can be NULL + */ + size_t(*crossover)(const uint8_t *data1, size_t size1, + const uint8_t *data2, size_t size2, + uint8_t *out, size_t max_out_size, + unsigned int seed); + } FuzzTarget; void flush_events(QTestState *); @@ -91,6 +114,10 @@ void fuzz_qtest_set_serialize(bool option); */ void fuzz_add_target(const FuzzTarget *target); +size_t LLVMFuzzerCustomCrossOver(const uint8_t *data1, size_t size1, + const uint8_t *data2, size_t size2, + uint8_t *out, size_t max_out_size, + unsigned int seed); int LLVMFuzzerTestOneInput(const unsigned char *Data, size_t Size); int LLVMFuzzerInitialize(int *argc, char ***argv, char ***envp); -- cgit v1.1 From ccbd4bc8af39096363fd06ab4fe2fe2f43042d76 Mon Sep 17 00:00:00 2001 From: Alexander Bulekov Date: Fri, 23 Oct 2020 11:07:37 -0400 Subject: fuzz: add a DISABLE_PCI op to generic-fuzzer This new operation is used in the next commit, which concatenates two fuzzer-generated inputs. With this operation, we can prevent the second input from clobbering the PCI configuration performed by the first. Signed-off-by: Alexander Bulekov Reviewed-by: Darren Kenny Message-Id: <20201023150746.107063-9-alxndr@bu.edu> Signed-off-by: Thomas Huth --- tests/qtest/fuzz/generic_fuzz.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c index 62a94de..6ec2046 100644 --- a/tests/qtest/fuzz/generic_fuzz.c +++ b/tests/qtest/fuzz/generic_fuzz.c @@ -39,6 +39,7 @@ enum cmds { OP_WRITE, OP_PCI_READ, OP_PCI_WRITE, + OP_DISABLE_PCI, OP_ADD_DMA_PATTERN, OP_CLEAR_DMA_PATTERNS, OP_CLOCK_STEP, @@ -116,6 +117,7 @@ static GArray *dma_regions; static GArray *dma_patterns; static int dma_pattern_index; +static bool pci_disabled; /* * Allocate a block of memory and populate it with a pattern. @@ -484,7 +486,7 @@ static void op_pci_read(QTestState *s, const unsigned char * data, size_t len) uint8_t base; uint8_t offset; } a; - if (len < sizeof(a) || fuzzable_pci_devices->len == 0) { + if (len < sizeof(a) || fuzzable_pci_devices->len == 0 || pci_disabled) { return; } memcpy(&a, data, sizeof(a)); @@ -514,7 +516,7 @@ static void op_pci_write(QTestState *s, const unsigned char * data, size_t len) uint8_t offset; uint32_t value; } a; - if (len < sizeof(a) || fuzzable_pci_devices->len == 0) { + if (len < sizeof(a) || fuzzable_pci_devices->len == 0 || pci_disabled) { return; } memcpy(&a, data, sizeof(a)); @@ -569,6 +571,11 @@ static void op_clock_step(QTestState *s, const unsigned char *data, size_t len) qtest_clock_step_next(s); } +static void op_disable_pci(QTestState *s, const unsigned char *data, size_t len) +{ + pci_disabled = true; +} + static void handle_timeout(int sig) { if (qtest_log_enabled) { @@ -624,6 +631,7 @@ static void generic_fuzz(QTestState *s, const unsigned char *Data, size_t Size) [OP_WRITE] = op_write, [OP_PCI_READ] = op_pci_read, [OP_PCI_WRITE] = op_pci_write, + [OP_DISABLE_PCI] = op_disable_pci, [OP_ADD_DMA_PATTERN] = op_add_dma_pattern, [OP_CLEAR_DMA_PATTERNS] = op_clear_dma_patterns, [OP_CLOCK_STEP] = op_clock_step, @@ -656,6 +664,7 @@ static void generic_fuzz(QTestState *s, const unsigned char *Data, size_t Size) } op_clear_dma_patterns(s, NULL, 0); + pci_disabled = false; while (cmd && Size) { /* Get the length until the next command or end of input */ -- cgit v1.1 From a25393222764c26658a98dbfc20f78c80765bca4 Mon Sep 17 00:00:00 2001 From: Alexander Bulekov Date: Fri, 23 Oct 2020 11:07:38 -0400 Subject: fuzz: add a crossover function to generic-fuzzer Reviewed-by: Darren Kenny Signed-off-by: Alexander Bulekov Message-Id: <20201023150746.107063-10-alxndr@bu.edu> Signed-off-by: Thomas Huth --- tests/qtest/fuzz/generic_fuzz.c | 86 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c index 6ec2046..592b78f 100644 --- a/tests/qtest/fuzz/generic_fuzz.c +++ b/tests/qtest/fuzz/generic_fuzz.c @@ -810,6 +810,91 @@ static void generic_pre_fuzz(QTestState *s) counter_shm_init(); } +/* + * When libfuzzer gives us two inputs to combine, return a new input with the + * following structure: + * + * Input 1 (data1) + * SEPARATOR + * Clear out the DMA Patterns + * SEPARATOR + * Disable the pci_read/write instructions + * SEPARATOR + * Input 2 (data2) + * + * The idea is to collate the core behaviors of the two inputs. + * For example: + * Input 1: maps a device's BARs, sets up three DMA patterns, and triggers + * device functionality A + * Input 2: maps a device's BARs, sets up one DMA pattern, and triggers device + * functionality B + * + * This function attempts to produce an input that: + * Ouptut: maps a device's BARs, set up three DMA patterns, triggers + * functionality A device, replaces the DMA patterns with a single + * patten, and triggers device functionality B. + */ +static size_t generic_fuzz_crossover(const uint8_t *data1, size_t size1, const + uint8_t *data2, size_t size2, uint8_t *out, + size_t max_out_size, unsigned int seed) +{ + size_t copy_len = 0, size = 0; + + /* Check that we have enough space for data1 and at least part of data2 */ + if (max_out_size <= size1 + strlen(SEPARATOR) * 3 + 2) { + return 0; + } + + /* Copy_Len in the first input */ + copy_len = size1; + memcpy(out + size, data1, copy_len); + size += copy_len; + max_out_size -= copy_len; + + /* Append a separator */ + copy_len = strlen(SEPARATOR); + memcpy(out + size, SEPARATOR, copy_len); + size += copy_len; + max_out_size -= copy_len; + + /* Clear out the DMA Patterns */ + copy_len = 1; + if (copy_len) { + out[size] = OP_CLEAR_DMA_PATTERNS; + } + size += copy_len; + max_out_size -= copy_len; + + /* Append a separator */ + copy_len = strlen(SEPARATOR); + memcpy(out + size, SEPARATOR, copy_len); + size += copy_len; + max_out_size -= copy_len; + + /* Disable PCI ops. Assume data1 took care of setting up PCI */ + copy_len = 1; + if (copy_len) { + out[size] = OP_DISABLE_PCI; + } + size += copy_len; + max_out_size -= copy_len; + + /* Append a separator */ + copy_len = strlen(SEPARATOR); + memcpy(out + size, SEPARATOR, copy_len); + size += copy_len; + max_out_size -= copy_len; + + /* Copy_Len over the second input */ + copy_len = MIN(size2, max_out_size); + memcpy(out + size, data2, copy_len); + size += copy_len; + max_out_size -= copy_len; + + return size; +} + + static GString *generic_fuzz_cmdline(FuzzTarget *t) { GString *cmd_line = g_string_new(TARGET_NAME); @@ -830,6 +915,7 @@ static void register_generic_fuzz_targets(void) .get_init_cmdline = generic_fuzz_cmdline, .pre_fuzz = generic_pre_fuzz, .fuzz = generic_fuzz, + .crossover = generic_fuzz_crossover }); } -- cgit v1.1 From 7c9b64ade9d1d3c69250ef1684db9c080a7b7092 Mon Sep 17 00:00:00 2001 From: Alexander Bulekov Date: Fri, 23 Oct 2020 11:07:39 -0400 Subject: scripts/oss-fuzz: Add script to reorder a generic-fuzzer trace The generic-fuzzer uses hooks to fulfill DMA requests just-in-time. This means that if we try to use QTEST_LOG=1 to build a reproducer, the DMA writes will be logged _after_ the in/out/read/write that triggered the DMA read. To work work around this, the generic-fuzzer annotates these just-in time DMA fulfilments with a tag that we can use to discern them. This script simply iterates over a raw qtest trace (including log messages, errors, timestamps etc), filters it and re-orders it so that DMA fulfillments are placed directly _before_ the qtest command that will cause the DMA access. Signed-off-by: Alexander Bulekov Reviewed-by: Darren Kenny Message-Id: <20201023150746.107063-11-alxndr@bu.edu> Signed-off-by: Thomas Huth --- scripts/oss-fuzz/reorder_fuzzer_qtest_trace.py | 103 +++++++++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100755 scripts/oss-fuzz/reorder_fuzzer_qtest_trace.py diff --git a/scripts/oss-fuzz/reorder_fuzzer_qtest_trace.py b/scripts/oss-fuzz/reorder_fuzzer_qtest_trace.py new file mode 100755 index 0000000..890e1de --- /dev/null +++ b/scripts/oss-fuzz/reorder_fuzzer_qtest_trace.py @@ -0,0 +1,103 @@ +#!/usr/bin/env python3 +# -*- coding: utf-8 -*- + +""" +Use this to convert qtest log info from a generic fuzzer input into a qtest +trace that you can feed into a standard qemu-system process. Example usage: + +QEMU_FUZZ_ARGS="-machine q35,accel=qtest" QEMU_FUZZ_OBJECTS="*" \ + ./i386-softmmu/qemu-fuzz-i386 --fuzz-target=generic-pci-fuzz +# .. Finds some crash +QTEST_LOG=1 FUZZ_SERIALIZE_QTEST=1 \ +QEMU_FUZZ_ARGS="-machine q35,accel=qtest" QEMU_FUZZ_OBJECTS="*" \ + ./i386-softmmu/qemu-fuzz-i386 --fuzz-target=generic-pci-fuzz + /path/to/crash 2> qtest_log_output +scripts/oss-fuzz/reorder_fuzzer_qtest_trace.py qtest_log_output > qtest_trace +./i386-softmmu/qemu-fuzz-i386 -machine q35,accel=qtest \ + -qtest stdin < qtest_trace + +### Details ### + +Some fuzzer make use of hooks that allow us to populate some memory range, just +before a DMA read from that range. This means that the fuzzer can produce +activity that looks like: + [start] read from mmio addr + [end] read from mmio addr + [start] write to pio addr + [start] fill a DMA buffer just in time + [end] fill a DMA buffer just in time + [start] fill a DMA buffer just in time + [end] fill a DMA buffer just in time + [end] write to pio addr + [start] read from mmio addr + [end] read from mmio addr + +We annotate these "nested" DMA writes, so with QTEST_LOG=1 the QTest trace +might look something like: +[R +0.028431] readw 0x10000 +[R +0.028434] outl 0xc000 0xbeef # Triggers a DMA read from 0xbeef and 0xbf00 +[DMA][R +0.034639] write 0xbeef 0x2 0xAAAA +[DMA][R +0.034639] write 0xbf00 0x2 0xBBBB +[R +0.028431] readw 0xfc000 + +This script would reorder the above trace so it becomes: +readw 0x10000 +write 0xbeef 0x2 0xAAAA +write 0xbf00 0x2 0xBBBB +outl 0xc000 0xbeef +readw 0xfc000 + +I.e. by the time, 0xc000 tries to read from DMA, those DMA buffers have already +been set up, removing the need for the DMA hooks. We can simply provide this +reordered trace via -qtest stdio to reproduce the input + +Note: this won't work for traces where the device tries to read from the same +DMA region twice in between MMIO/PIO commands. E.g: + [R +0.028434] outl 0xc000 0xbeef + [DMA][R +0.034639] write 0xbeef 0x2 0xAAAA + [DMA][R +0.034639] write 0xbeef 0x2 0xBBBB + +The fuzzer will annotate suspected double-fetches with [DOUBLE-FETCH]. This +script looks for these tags and warns the users that the resulting trace might +not reproduce the bug. +""" + +import sys + +__author__ = "Alexander Bulekov " +__copyright__ = "Copyright (C) 2020, Red Hat, Inc." +__license__ = "GPL version 2 or (at your option) any later version" + +__maintainer__ = "Alexander Bulekov" +__email__ = "alxndr@bu.edu" + + +def usage(): + sys.exit("Usage: {} /path/to/qtest_log_output".format((sys.argv[0]))) + + +def main(filename): + with open(filename, "r") as f: + trace = f.readlines() + + # Leave only lines that look like logged qtest commands + trace[:] = [x.strip() for x in trace if "[R +" in x + or "[S +" in x and "CLOSED" not in x] + + for i in range(len(trace)): + if i+1 < len(trace): + if "[DMA]" in trace[i+1]: + if "[DOUBLE-FETCH]" in trace[i+1]: + sys.stderr.write("Warning: Likely double fetch on line" + "{}.\n There will likely be problems " + "reproducing behavior with the " + "resulting qtest trace\n\n".format(i+1)) + trace[i], trace[i+1] = trace[i+1], trace[i] + for line in trace: + print(line.split("]")[-1].strip()) + + +if __name__ == '__main__': + if len(sys.argv) == 1: + usage() + main(sys.argv[1]) -- cgit v1.1 From cd3f0686ddf3cd18f307fb9f55f9cf21bf185bbf Mon Sep 17 00:00:00 2001 From: Alexander Bulekov Date: Fri, 23 Oct 2020 11:07:40 -0400 Subject: scripts/oss-fuzz: Add crash trace minimization script Once we find a crash, we can convert it into a QTest trace. Usually this trace will contain many operations that are unneeded to reproduce the crash. This script tries to minimize the crashing trace, by removing operations and trimming QTest bufwrite(write addr len data...) commands. Signed-off-by: Alexander Bulekov Reviewed-by: Darren Kenny Message-Id: <20201023150746.107063-12-alxndr@bu.edu> Signed-off-by: Thomas Huth --- scripts/oss-fuzz/minimize_qtest_trace.py | 157 +++++++++++++++++++++++++++++++ 1 file changed, 157 insertions(+) create mode 100755 scripts/oss-fuzz/minimize_qtest_trace.py diff --git a/scripts/oss-fuzz/minimize_qtest_trace.py b/scripts/oss-fuzz/minimize_qtest_trace.py new file mode 100755 index 0000000..5e405a0 --- /dev/null +++ b/scripts/oss-fuzz/minimize_qtest_trace.py @@ -0,0 +1,157 @@ +#!/usr/bin/env python3 +# -*- coding: utf-8 -*- + +""" +This takes a crashing qtest trace and tries to remove superflous operations +""" + +import sys +import os +import subprocess +import time +import struct + +QEMU_ARGS = None +QEMU_PATH = None +TIMEOUT = 5 +CRASH_TOKEN = None + +write_suffix_lookup = {"b": (1, "B"), + "w": (2, "H"), + "l": (4, "L"), + "q": (8, "Q")} + +def usage(): + sys.exit("""\ +Usage: QEMU_PATH="/path/to/qemu" QEMU_ARGS="args" {} input_trace output_trace +By default, will try to use the second-to-last line in the output to identify +whether the crash occred. Optionally, manually set a string that idenitifes the +crash by setting CRASH_TOKEN= +""".format((sys.argv[0]))) + +def check_if_trace_crashes(trace, path): + global CRASH_TOKEN + with open(path, "w") as tracefile: + tracefile.write("".join(trace)) + + rc = subprocess.Popen("timeout -s 9 {timeout}s {qemu_path} {qemu_args} 2>&1\ + < {trace_path}".format(timeout=TIMEOUT, + qemu_path=QEMU_PATH, + qemu_args=QEMU_ARGS, + trace_path=path), + shell=True, + stdin=subprocess.PIPE, + stdout=subprocess.PIPE) + stdo = rc.communicate()[0] + output = stdo.decode('unicode_escape') + if rc.returncode == 137: # Timed Out + return False + if len(output.splitlines()) < 2: + return False + + if CRASH_TOKEN is None: + CRASH_TOKEN = output.splitlines()[-2] + + return CRASH_TOKEN in output + + +def minimize_trace(inpath, outpath): + global TIMEOUT + with open(inpath) as f: + trace = f.readlines() + start = time.time() + if not check_if_trace_crashes(trace, outpath): + sys.exit("The input qtest trace didn't cause a crash...") + end = time.time() + print("Crashed in {} seconds".format(end-start)) + TIMEOUT = (end-start)*5 + print("Setting the timeout for {} seconds".format(TIMEOUT)) + print("Identifying Crashes by this string: {}".format(CRASH_TOKEN)) + + i = 0 + newtrace = trace[:] + # For each line + while i < len(newtrace): + # 1.) Try to remove it completely and reproduce the crash. If it works, + # we're done. + prior = newtrace[i] + print("Trying to remove {}".format(newtrace[i])) + # Try to remove the line completely + newtrace[i] = "" + if check_if_trace_crashes(newtrace, outpath): + i += 1 + continue + newtrace[i] = prior + + # 2.) Try to replace write{bwlq} commands with a write addr, len + # command. Since this can require swapping endianness, try both LE and + # BE options. We do this, so we can "trim" the writes in (3) + if (newtrace[i].startswith("write") and not + newtrace[i].startswith("write ")): + suffix = newtrace[i].split()[0][-1] + assert(suffix in write_suffix_lookup) + addr = int(newtrace[i].split()[1], 16) + value = int(newtrace[i].split()[2], 16) + for endianness in ['<', '>']: + data = struct.pack("{end}{size}".format(end=endianness, + size=write_suffix_lookup[suffix][1]), + value) + newtrace[i] = "write {addr} {size} 0x{data}\n".format( + addr=hex(addr), + size=hex(write_suffix_lookup[suffix][0]), + data=data.hex()) + if(check_if_trace_crashes(newtrace, outpath)): + break + else: + newtrace[i] = prior + + # 3.) If it is a qtest write command: write addr len data, try to split + # it into two separate write commands. If splitting the write down the + # middle does not work, try to move the pivot "left" and retry, until + # there is no space left. The idea is to prune unneccessary bytes from + # long writes, while accommodating arbitrary MemoryRegion access sizes + # and alignments. + if newtrace[i].startswith("write "): + addr = int(newtrace[i].split()[1], 16) + length = int(newtrace[i].split()[2], 16) + data = newtrace[i].split()[3][2:] + if length > 1: + leftlength = int(length/2) + rightlength = length - leftlength + newtrace.insert(i+1, "") + while leftlength > 0: + newtrace[i] = "write {addr} {size} 0x{data}\n".format( + addr=hex(addr), + size=hex(leftlength), + data=data[:leftlength*2]) + newtrace[i+1] = "write {addr} {size} 0x{data}\n".format( + addr=hex(addr+leftlength), + size=hex(rightlength), + data=data[leftlength*2:]) + if check_if_trace_crashes(newtrace, outpath): + break + else: + leftlength -= 1 + rightlength += 1 + if check_if_trace_crashes(newtrace, outpath): + i -= 1 + else: + newtrace[i] = prior + del newtrace[i+1] + i += 1 + check_if_trace_crashes(newtrace, outpath) + + +if __name__ == '__main__': + if len(sys.argv) < 3: + usage() + + QEMU_PATH = os.getenv("QEMU_PATH") + QEMU_ARGS = os.getenv("QEMU_ARGS") + if QEMU_PATH is None or QEMU_ARGS is None: + usage() + # if "accel" not in QEMU_ARGS: + # QEMU_ARGS += " -accel qtest" + CRASH_TOKEN = os.getenv("CRASH_TOKEN") + QEMU_ARGS += " -qtest stdio -monitor none -serial none " + minimize_trace(sys.argv[1], sys.argv[2]) -- cgit v1.1 From 2f2e036ca6e2c4d15841f6d29a17c2ae0961aca8 Mon Sep 17 00:00:00 2001 From: Alexander Bulekov Date: Fri, 23 Oct 2020 11:07:41 -0400 Subject: fuzz: Add instructions for using generic-fuzz Reviewed-by: Darren Kenny Signed-off-by: Alexander Bulekov Message-Id: <20201023150746.107063-13-alxndr@bu.edu> Signed-off-by: Thomas Huth --- docs/devel/fuzzing.txt | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/docs/devel/fuzzing.txt b/docs/devel/fuzzing.txt index 96d71c9..03585c1 100644 --- a/docs/devel/fuzzing.txt +++ b/docs/devel/fuzzing.txt @@ -125,6 +125,45 @@ provided by libfuzzer. Libfuzzer passes a byte array and length. Commonly the fuzzer loops over the byte-array interpreting it as a list of qtest commands, addresses, or values. +== The Generic Fuzzer == +Writing a fuzz target can be a lot of effort (especially if a device driver has +not be built-out within libqos). Many devices can be fuzzed to some degree, +without any device-specific code, using the generic-fuzz target. + +The generic-fuzz target is capable of fuzzing devices over their PIO, MMIO, +and DMA input-spaces. To apply the generic-fuzz to a device, we need to define +two env-variables, at minimum: + +QEMU_FUZZ_ARGS= is the set of QEMU arguments used to configure a machine, with +the device attached. For example, if we want to fuzz the virtio-net device +attached to a pc-i440fx machine, we can specify: +QEMU_FUZZ_ARGS="-M pc -nodefaults -netdev user,id=user0 \ + -device virtio-net,netdev=user0" + +QEMU_FUZZ_OBJECTS= is a set of space-delimited strings used to identify the +MemoryRegions that will be fuzzed. These strings are compared against +MemoryRegion names and MemoryRegion owner names, to decide whether each +MemoryRegion should be fuzzed. These strings support globbing. For the +virtio-net example, we could use QEMU_FUZZ_OBJECTS= + * 'virtio-net' + * 'virtio*' + * 'virtio* pcspk' (Fuzz the virtio devices and the PC speaker...) + * '*' (Fuzz the whole machine) + +The "info mtree" and "info qom-tree" monitor commands can be especially useful +for identifying the MemoryRegion and Object names used for matching. + +As a generic rule-of-thumb, the more MemoryRegions/Devices we match, the greater +the input-space, and the smaller the probability of finding crashing inputs for +individual devices. As such, it is usually a good idea to limit the fuzzer to +only a few MemoryRegions. + +To ensure that these env variables have been configured correctly, we can use: + +./qemu-fuzz-i386 --fuzz-target=generic-fuzz -runs=0 + +The output should contain a complete list of matched MemoryRegions. + = Implementation Details = == The Fuzzer's Lifecycle == -- cgit v1.1 From 82849bcf30b5a1dfac6c1d7642a243c2f7bd6a6f Mon Sep 17 00:00:00 2001 From: Alexander Bulekov Date: Fri, 23 Oct 2020 11:07:42 -0400 Subject: fuzz: add an "opaque" to the FuzzTarget struct It can be useful to register FuzzTargets that have nearly-identical initialization handlers (e.g. for using the same fuzzing code, with different configuration options). Add an opaque pointer to the FuzzTarget struct, so that FuzzTargets can hold some data, useful for storing target-specific configuration options, that can be read by the get_init_cmdline function. Reviewed-by: Darren Kenny Signed-off-by: Alexander Bulekov Message-Id: <20201023150746.107063-14-alxndr@bu.edu> Signed-off-by: Thomas Huth --- tests/qtest/fuzz/fuzz.h | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/qtest/fuzz/fuzz.h b/tests/qtest/fuzz/fuzz.h index ed9ce17..08e9560 100644 --- a/tests/qtest/fuzz/fuzz.h +++ b/tests/qtest/fuzz/fuzz.h @@ -100,6 +100,7 @@ typedef struct FuzzTarget { uint8_t *out, size_t max_out_size, unsigned int seed); + void *opaque; } FuzzTarget; void flush_events(QTestState *); -- cgit v1.1 From 61fc27e0df7b6c3276fbd42c1c61f72e5b49c2b4 Mon Sep 17 00:00:00 2001 From: Alexander Bulekov Date: Fri, 23 Oct 2020 11:07:43 -0400 Subject: fuzz: add generic-fuzz configs for oss-fuzz Predefine some generic-fuzz configs. For each of these, we will create a separate FuzzTarget that can be selected through argv0 and, therefore, fuzzed on oss-fuzz. Reviewed-by: Darren Kenny Signed-off-by: Alexander Bulekov Message-Id: <20201023150746.107063-15-alxndr@bu.edu> Signed-off-by: Thomas Huth --- tests/qtest/fuzz/generic_fuzz_configs.h | 121 ++++++++++++++++++++++++++++++++ 1 file changed, 121 insertions(+) create mode 100644 tests/qtest/fuzz/generic_fuzz_configs.h diff --git a/tests/qtest/fuzz/generic_fuzz_configs.h b/tests/qtest/fuzz/generic_fuzz_configs.h new file mode 100644 index 0000000..c4d925f --- /dev/null +++ b/tests/qtest/fuzz/generic_fuzz_configs.h @@ -0,0 +1,121 @@ +/* + * Generic Virtual-Device Fuzzing Target Configs + * + * Copyright Red Hat Inc., 2020 + * + * Authors: + * Alexander Bulekov + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#ifndef GENERIC_FUZZ_CONFIGS_H +#define GENERIC_FUZZ_CONFIGS_H + +#include "qemu/osdep.h" + +typedef struct generic_fuzz_config { + const char *name, *args, *objects; +} generic_fuzz_config; + +const generic_fuzz_config predefined_configs[] = { + { + .name = "virtio-net-pci-slirp", + .args = "-M q35 -nodefaults " + "-device virtio-net,netdev=net0 -netdev user,id=net0", + .objects = "virtio*", + },{ + .name = "virtio-blk", + .args = "-machine q35 -device virtio-blk,drive=disk0 " + "-drive file=null-co://,id=disk0,if=none,format=raw", + .objects = "virtio*", + },{ + .name = "virtio-scsi", + .args = "-machine q35 -device virtio-scsi,num_queues=8 " + "-device scsi-hd,drive=disk0 " + "-drive file=null-co://,id=disk0,if=none,format=raw", + .objects = "scsi* virtio*", + },{ + .name = "virtio-gpu", + .args = "-machine q35 -nodefaults -device virtio-gpu", + .objects = "virtio*", + },{ + .name = "virtio-vga", + .args = "-machine q35 -nodefaults -device virtio-vga", + .objects = "virtio*", + },{ + .name = "virtio-rng", + .args = "-machine q35 -nodefaults -device virtio-rng", + .objects = "virtio*", + },{ + .name = "virtio-balloon", + .args = "-machine q35 -nodefaults -device virtio-balloon", + .objects = "virtio*", + },{ + .name = "virtio-serial", + .args = "-machine q35 -nodefaults -device virtio-serial", + .objects = "virtio*", + },{ + .name = "virtio-mouse", + .args = "-machine q35 -nodefaults -device virtio-mouse", + .objects = "virtio*", + },{ + .name = "e1000", + .args = "-M q35 -nodefaults " + "-device e1000,netdev=net0 -netdev user,id=net0", + .objects = "e1000", + },{ + .name = "e1000e", + .args = "-M q35 -nodefaults " + "-device e1000e,netdev=net0 -netdev user,id=net0", + .objects = "e1000e", + },{ + .name = "cirrus-vga", + .args = "-machine q35 -nodefaults -device cirrus-vga", + .objects = "cirrus*", + },{ + .name = "bochs-display", + .args = "-machine q35 -nodefaults -device bochs-display", + .objects = "bochs*", + },{ + .name = "intel-hda", + .args = "-machine q35 -nodefaults -device intel-hda,id=hda0 " + "-device hda-output,bus=hda0.0 -device hda-micro,bus=hda0.0 " + "-device hda-duplex,bus=hda0.0", + .objects = "intel-hda", + },{ + .name = "ide-hd", + .args = "-machine q35 -nodefaults " + "-drive file=null-co://,if=none,format=raw,id=disk0 " + "-device ide-hd,drive=disk0", + .objects = "ahci*", + },{ + .name = "floppy", + .args = "-machine pc -nodefaults -device floppy,id=floppy0 " + "-drive id=disk0,file=null-co://,file.read-zeroes=on,if=none " + "-device floppy,drive=disk0,drive-type=288", + .objects = "fd* floppy*", + },{ + .name = "xhci", + .args = "-machine q35 -nodefaults " + "-drive file=null-co://,if=none,format=raw,id=disk0 " + "-device qemu-xhci,id=xhci -device usb-tablet,bus=xhci.0 " + "-device usb-bot -device usb-storage,drive=disk0 " + "-chardev null,id=cd0 -chardev null,id=cd1 " + "-device usb-braille,chardev=cd0 -device usb-ccid -device usb-ccid " + "-device usb-kbd -device usb-mouse -device usb-serial,chardev=cd1 " + "-device usb-tablet -device usb-wacom-tablet -device usb-audio", + .objects = "*usb* *uhci* *xhci*", + },{ + .name = "pc-i440fx", + .args = "-machine pc", + .objects = "*", + },{ + .name = "pc-q35", + .args = "-machine q35", + .objects = "*", + } +}; + +#endif -- cgit v1.1 From 7fdb50538470a0ce60044cf93a55ec5ee3ff6f57 Mon Sep 17 00:00:00 2001 From: Alexander Bulekov Date: Fri, 23 Oct 2020 11:07:44 -0400 Subject: fuzz: register predefined generic-fuzz configs We call get_generic_fuzz_configs, which fills an array with predefined {name, args, objects} triples. For each of these, we add a new FuzzTarget, that uses a small wrapper to set QEMU_FUZZ_{ARGS,OBJECTS} to the corresponding predefined values. Reviewed-by: Darren Kenny Signed-off-by: Alexander Bulekov Message-Id: <20201023150746.107063-16-alxndr@bu.edu> Signed-off-by: Thomas Huth --- tests/qtest/fuzz/generic_fuzz.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/qtest/fuzz/generic_fuzz.c b/tests/qtest/fuzz/generic_fuzz.c index 592b78f..a8f5864 100644 --- a/tests/qtest/fuzz/generic_fuzz.c +++ b/tests/qtest/fuzz/generic_fuzz.c @@ -26,6 +26,7 @@ #include "hw/qdev-core.h" #include "hw/pci/pci.h" #include "hw/boards.h" +#include "generic_fuzz_configs.h" /* * SEPARATOR is used to separate "operations" in the fuzz input @@ -907,6 +908,17 @@ static GString *generic_fuzz_cmdline(FuzzTarget *t) return cmd_line; } +static GString *generic_fuzz_predefined_config_cmdline(FuzzTarget *t) +{ + const generic_fuzz_config *config; + g_assert(t->opaque); + + config = t->opaque; + setenv("QEMU_FUZZ_ARGS", config->args, 1); + setenv("QEMU_FUZZ_OBJECTS", config->objects, 1); + return generic_fuzz_cmdline(t); +} + static void register_generic_fuzz_targets(void) { fuzz_add_target(&(FuzzTarget){ @@ -917,6 +929,26 @@ static void register_generic_fuzz_targets(void) .fuzz = generic_fuzz, .crossover = generic_fuzz_crossover }); + + GString *name; + const generic_fuzz_config *config; + + for (int i = 0; + i < sizeof(predefined_configs) / sizeof(generic_fuzz_config); + i++) { + config = predefined_configs + i; + name = g_string_new("generic-fuzz"); + g_string_append_printf(name, "-%s", config->name); + fuzz_add_target(&(FuzzTarget){ + .name = name->str, + .description = "Predefined generic-fuzz config.", + .get_init_cmdline = generic_fuzz_predefined_config_cmdline, + .pre_fuzz = generic_pre_fuzz, + .fuzz = generic_fuzz, + .crossover = generic_fuzz_crossover, + .opaque = (void *)config + }); + } } fuzz_target_init(register_generic_fuzz_targets); -- cgit v1.1 From a942f64cc4b875c2fe92ea91fea85741e00b12b9 Mon Sep 17 00:00:00 2001 From: Alexander Bulekov Date: Fri, 23 Oct 2020 11:07:45 -0400 Subject: scripts/oss-fuzz: use hardlinks instead of copying Prior to this, fuzzers in the output oss-fuzz directory were exactly the same executable, with a different name to do argv[0]-based fuzz-target selection. This is a waste of space, especially since these binaries can weigh many MB. Instead of copying, use hard links, to cut down on wasted space. We need to place the primary copy of the executable into DEST_DIR, since this is a separate file-system on oss-fuzz. We should not place it directly into $DEST_DIR, since oss-fuzz will treat it as an independent fuzzer and try to run it for fuzzing. Instead, we create a DEST_DIR/bin directory to store the primary copy. Suggested-by: Darren Kenny Signed-off-by: Alexander Bulekov Message-Id: <20201023150746.107063-17-alxndr@bu.edu> Reviewed-by: Darren Kenny Signed-off-by: Thomas Huth --- scripts/oss-fuzz/build.sh | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh index 0c3ca9e..0ce2867 100755 --- a/scripts/oss-fuzz/build.sh +++ b/scripts/oss-fuzz/build.sh @@ -62,6 +62,9 @@ fi mkdir -p "$DEST_DIR/lib/" # Copy the shared libraries here +mkdir -p "$DEST_DIR/bin/" # Copy executables that shouldn't + # be treated as fuzzers by oss-fuzz here + # Build once to get the list of dynamic lib paths, and copy them over ../configure --disable-werror --cc="$CC" --cxx="$CXX" --enable-fuzzing \ --prefix="$DEST_DIR" --bindir="$DEST_DIR" --datadir="$DEST_DIR/data/" \ @@ -88,13 +91,16 @@ make "-j$(nproc)" qemu-fuzz-i386 V=1 # Copy over the datadir cp -r ../pc-bios/ "$DEST_DIR/pc-bios" +cp "./qemu-fuzz-i386" "$DEST_DIR/bin/" + # Run the fuzzer with no arguments, to print the help-string and get the list # of available fuzz-targets. Copy over the qemu-fuzz-i386, naming it according # to each available fuzz target (See 05509c8e6d fuzz: select fuzz target using # executable name) for target in $(./qemu-fuzz-i386 | awk '$1 ~ /\*/ {print $2}'); do - cp qemu-fuzz-i386 "$DEST_DIR/qemu-fuzz-i386-target-$target" + ln "$DEST_DIR/bin/qemu-fuzz-i386" \ + "$DEST_DIR/qemu-fuzz-i386-target-$target" done echo "Done. The fuzzers are located in $DEST_DIR" -- cgit v1.1 From 53e1a50d6b6fe97fafa81ab9f2ddebf92798a57b Mon Sep 17 00:00:00 2001 From: Alexander Bulekov Date: Fri, 23 Oct 2020 11:07:46 -0400 Subject: scripts/oss-fuzz: ignore the generic-fuzz target generic-fuzz is not a standalone fuzzer - it requires some env variables to be set. On oss-fuzz, we set these with some predefined generic-fuzz-{...} targets, that are thin wrappers around generic-fuzz. Do not make a link for the generic-fuzz from the oss-fuzz build, so oss-fuzz does not treat it as a standalone fuzzer. Signed-off-by: Alexander Bulekov Message-Id: <20201023150746.107063-18-alxndr@bu.edu> Reviewed-by: Darren Kenny [thuth: Reformatted one comment to stay within the 80 columns limit] Signed-off-by: Thomas Huth --- scripts/oss-fuzz/build.sh | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh index 0ce2867..fcae4a0 100755 --- a/scripts/oss-fuzz/build.sh +++ b/scripts/oss-fuzz/build.sh @@ -99,8 +99,14 @@ cp "./qemu-fuzz-i386" "$DEST_DIR/bin/" # executable name) for target in $(./qemu-fuzz-i386 | awk '$1 ~ /\*/ {print $2}'); do - ln "$DEST_DIR/bin/qemu-fuzz-i386" \ - "$DEST_DIR/qemu-fuzz-i386-target-$target" + # Ignore the generic-fuzz target, as it requires some environment variables + # to be configured. We have some generic-fuzz-{pc-q35, floppy, ...} targets + # that are thin wrappers around this target that set the required + # environment variables according to predefined configs. + if [ "$target" != "generic-fuzz" ]; then + ln "$DEST_DIR/bin/qemu-fuzz-i386" \ + "$DEST_DIR/qemu-fuzz-i386-target-$target" + fi done echo "Done. The fuzzers are located in $DEST_DIR" -- cgit v1.1 From a60f755c9cb52a2a2dea83b9d69e5bed2276de97 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Thu, 22 Oct 2020 16:19:00 +0200 Subject: tests/acceptance/ppc_prep_40p: Fix the URL to the NetBSD-4.0 archive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The current URL on cdn.netbsd.org is failing - using archive.netbsd.org instead seems to be fine. Message-Id: <20201023073351.251332-2-thuth@redhat.com> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Thomas Huth --- tests/acceptance/ppc_prep_40p.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/acceptance/ppc_prep_40p.py b/tests/acceptance/ppc_prep_40p.py index 1515561..e82755c 100644 --- a/tests/acceptance/ppc_prep_40p.py +++ b/tests/acceptance/ppc_prep_40p.py @@ -35,7 +35,7 @@ class IbmPrep40pMachine(Test): '7020-40p/P12H0456.IMG') bios_hash = '1775face4e6dc27f3a6ed955ef6eb331bf817f03' bios_path = self.fetch_asset(bios_url, asset_hash=bios_hash) - drive_url = ('https://cdn.netbsd.org/pub/NetBSD/NetBSD-archive/' + drive_url = ('https://archive.netbsd.org/pub/NetBSD-archive/' 'NetBSD-4.0/prep/installation/floppy/generic_com0.fs') drive_hash = 'dbcfc09912e71bd5f0d82c7c1ee43082fb596ceb' drive_path = self.fetch_asset(drive_url, asset_hash=drive_hash) -- cgit v1.1 From 1d60f46fc693e8459f700684f0af4e0130a9bcee Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Thu, 22 Oct 2020 14:09:52 +0200 Subject: test/acceptance: Remove the CONTINUOUS_INTEGRATION tags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We are not running the acceptance tests on Travis anymore, so these checks can be removed now. Message-Id: <20201023073351.251332-3-thuth@redhat.com> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Thomas Huth --- tests/acceptance/ppc_prep_40p.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/acceptance/ppc_prep_40p.py b/tests/acceptance/ppc_prep_40p.py index e82755c..96ba13b 100644 --- a/tests/acceptance/ppc_prep_40p.py +++ b/tests/acceptance/ppc_prep_40p.py @@ -22,7 +22,6 @@ class IbmPrep40pMachine(Test): # All rights reserved. # U.S. Government Users Restricted Rights - Use, duplication or disclosure # restricted by GSA ADP Schedule Contract with IBM Corp. - @skipIf(os.getenv('CONTINUOUS_INTEGRATION'), 'Running on Travis-CI') @skipUnless(os.getenv('AVOCADO_ALLOW_UNTRUSTED_CODE'), 'untrusted code') def test_factory_firmware_and_netbsd(self): """ @@ -61,7 +60,6 @@ class IbmPrep40pMachine(Test): wait_for_console_pattern(self, '>> Memory: 192M') wait_for_console_pattern(self, '>> CPU type PowerPC,604') - @skipIf(os.getenv('CONTINUOUS_INTEGRATION'), 'Running on Travis-CI') def test_openbios_and_netbsd(self): """ :avocado: tags=arch:ppc -- cgit v1.1 From 67202baeaa856e9fe66cffc3a9abeeada1f45a43 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Wed, 5 Aug 2020 17:18:00 +0200 Subject: tests/acceptance: Enable AVOCADO_ALLOW_UNTRUSTED_CODE in the gitlab-CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tests are running in containers here, so it should be OK to run with AVOCADO_ALLOW_UNTRUSTED_CODE enabled in this case. Message-Id: <20201023073351.251332-4-thuth@redhat.com> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Thomas Huth --- .gitlab-ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 66ad7aa..5d6773e 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -66,6 +66,7 @@ include: - if [ -d ${CI_PROJECT_DIR}/avocado-cache ]; then du -chs ${CI_PROJECT_DIR}/avocado-cache ; fi + - export AVOCADO_ALLOW_UNTRUSTED_CODE=1 after_script: - cd build - python3 -c 'import json; r = json.load(open("tests/results/latest/results.json")); [print(t["logfile"]) for t in r["tests"] if t["status"] not in ("PASS", "SKIP", "CANCEL")]' | xargs cat -- cgit v1.1 From c4cb1c9f2e15762e05ecf3e06ecf3c839c3a94ce Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Thu, 22 Oct 2020 18:25:44 +0200 Subject: test/docker/dockerfiles: Add missing packages for acceptance tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some of the "check-acceptance" tests are still skipped in the CI since the docker images do not provide the necessary packages, e.g. the netcat binary. Add them to get more test coverage. Message-Id: <20201023073351.251332-5-thuth@redhat.com> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Thomas Huth --- tests/docker/dockerfiles/centos8.docker | 1 + tests/docker/dockerfiles/debian-amd64.docker | 3 +++ tests/docker/dockerfiles/fedora.docker | 1 + tests/docker/dockerfiles/ubuntu2004.docker | 1 + 4 files changed, 6 insertions(+) diff --git a/tests/docker/dockerfiles/centos8.docker b/tests/docker/dockerfiles/centos8.docker index 585dfad..a589142 100644 --- a/tests/docker/dockerfiles/centos8.docker +++ b/tests/docker/dockerfiles/centos8.docker @@ -18,6 +18,7 @@ ENV PACKAGES \ lzo-devel \ make \ mesa-libEGL-devel \ + nmap-ncat \ nettle-devel \ ninja-build \ perl-Test-Harness \ diff --git a/tests/docker/dockerfiles/debian-amd64.docker b/tests/docker/dockerfiles/debian-amd64.docker index 314c6ba..55075d9 100644 --- a/tests/docker/dockerfiles/debian-amd64.docker +++ b/tests/docker/dockerfiles/debian-amd64.docker @@ -23,6 +23,9 @@ RUN apt update && \ libsnappy-dev \ libvte-dev \ netcat-openbsd \ + openssh-client \ + python3-numpy \ + python3-opencv \ python3-venv # virgl diff --git a/tests/docker/dockerfiles/fedora.docker b/tests/docker/dockerfiles/fedora.docker index ac79d95..0b5053f 100644 --- a/tests/docker/dockerfiles/fedora.docker +++ b/tests/docker/dockerfiles/fedora.docker @@ -73,6 +73,7 @@ ENV PACKAGES \ mingw64-pixman \ mingw64-pkg-config \ mingw64-SDL2 \ + nmap-ncat \ ncurses-devel \ nettle-devel \ ninja-build \ diff --git a/tests/docker/dockerfiles/ubuntu2004.docker b/tests/docker/dockerfiles/ubuntu2004.docker index 17b37cd..355bbb3 100644 --- a/tests/docker/dockerfiles/ubuntu2004.docker +++ b/tests/docker/dockerfiles/ubuntu2004.docker @@ -47,6 +47,7 @@ ENV PACKAGES flex bison \ libxen-dev \ libzstd-dev \ make \ + netcat-openbsd \ ninja-build \ python3-numpy \ python3-opencv \ -- cgit v1.1 From 239f0d455bf727d58d3ff52070919de8f8089ace Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Wed, 21 Oct 2020 12:50:30 +0200 Subject: tests/acceptance: Remove unused import MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20201021105035.2477784-2-f4bug@amsat.org> Tested-by: Thomas Huth Signed-off-by: Thomas Huth --- tests/acceptance/machine_m68k_nextcube.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/acceptance/machine_m68k_nextcube.py b/tests/acceptance/machine_m68k_nextcube.py index 32cf571..9d289f2 100644 --- a/tests/acceptance/machine_m68k_nextcube.py +++ b/tests/acceptance/machine_m68k_nextcube.py @@ -9,7 +9,6 @@ import os import re import time import logging -import distutils.spawn from avocado_qemu import Test from avocado import skipUnless -- cgit v1.1 From 28bbe20ce281659e317b807f34f568bde6d99760 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Wed, 21 Oct 2020 12:50:31 +0200 Subject: tests/acceptance: Use .ppm extention for Portable PixMap files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The HMP 'screendump' command generates Portable PixMap files. Make it obvious by using the .ppm file extention. Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20201021105035.2477784-3-f4bug@amsat.org> Reviewed-by: Thomas Huth Signed-off-by: Thomas Huth --- tests/acceptance/machine_m68k_nextcube.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/acceptance/machine_m68k_nextcube.py b/tests/acceptance/machine_m68k_nextcube.py index 9d289f2..2baba5f 100644 --- a/tests/acceptance/machine_m68k_nextcube.py +++ b/tests/acceptance/machine_m68k_nextcube.py @@ -69,7 +69,7 @@ class NextCubeMachine(Test): @skipUnless(PIL_AVAILABLE, 'Python PIL not installed') def test_bootrom_framebuffer_size(self): - screenshot_path = os.path.join(self.workdir, "dump.png") + screenshot_path = os.path.join(self.workdir, "dump.ppm") self.check_bootrom_framebuffer(screenshot_path) width, height = Image.open(screenshot_path).size @@ -78,7 +78,7 @@ class NextCubeMachine(Test): @skipUnless(tesseract_available(3), 'tesseract v3 OCR tool not available') def test_bootrom_framebuffer_ocr_with_tesseract_v3(self): - screenshot_path = os.path.join(self.workdir, "dump.png") + screenshot_path = os.path.join(self.workdir, "dump.ppm") self.check_bootrom_framebuffer(screenshot_path) console_logger = logging.getLogger('console') @@ -94,7 +94,7 @@ class NextCubeMachine(Test): # that it is still alpha-level software. @skipUnless(tesseract_available(4), 'tesseract v4 OCR tool not available') def test_bootrom_framebuffer_ocr_with_tesseract_v4(self): - screenshot_path = os.path.join(self.workdir, "dump.png") + screenshot_path = os.path.join(self.workdir, "dump.ppm") self.check_bootrom_framebuffer(screenshot_path) console_logger = logging.getLogger('console') -- cgit v1.1