| Message ID | 20210319050906.14875-1-alxndr@bu.edu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] floppy: add a regression test for CVE-2020-25741 | expand |
Alexander Bulekov <alxndr@bu.edu> writes: > dd if=/dev/zero of=/tmp/fda.img bs=1024 count=1440 > cat << EOF | ./qemu-system-i386 -nographic -m 512M -nodefaults \ > -accel qtest -fda /tmp/fda.img -qtest stdio > outw 0x3f4 0x0500 > outb 0x3f5 0x00 > outb 0x3f5 0x00 > outw 0x3f4 0x00 > outb 0x3f5 0x00 > outw 0x3f1 0x0400 > outw 0x3f4 0x0 > outw 0x3f4 0x00 > outb 0x3f5 0x0 > outb 0x3f5 0x01 > outw 0x3f1 0x0500 > outb 0x3f5 0x00 > EOF > > Signed-off-by: Alexander Bulekov <alxndr@bu.edu> I guess this is a reproducer. Please also describe actual and expected result. Same for PATCH 2.
On 19/03/21 06:53, Markus Armbruster wrote: > I guess this is a reproducer. Please also describe actual and expected > result. Same for PATCH 2. Isn't it in the patch itself? Alexander, I think these reproducers are self-contained enough (no writes to PCI configuration space to set up the device) that we can place them in fdc-test.c. Paolo
Paolo Bonzini <pbonzini@redhat.com> writes: > On 19/03/21 06:53, Markus Armbruster wrote: >> I guess this is a reproducer. Please also describe actual and expected >> result. Same for PATCH 2. > > Isn't it in the patch itself? A commit message should tell me what the patch is trying to accomplish. This commit message's title tells me it's a test for a CVE. Okay. The body additionally gives me the reproducer. To be useful, a reproducer needs to come with actual and expected result. Yes, I can find those in the patch. But I could find the reproducer there, too. If you're nice enough to save me the trouble of digging through the patch for the reproducer (thanks), please consider saving me the trouble digging for the information I need to make use of it (thanks again). That's all :) [...]
On 19/03/21 10:54, Markus Armbruster wrote: > A commit message should tell me what the patch is trying to accomplish. > > This commit message's title tells me it's a test for a CVE. Okay. The > body additionally gives me the reproducer. To be useful, a reproducer > needs to come with actual and expected result. Yes, I can find those in > the patch. But I could find the reproducer there, too. If you're nice > enough to save me the trouble of digging through the patch for the > reproducer (thanks), please consider saving me the trouble digging for > the information I need to make use of it (thanks again). That's all:) FWIW I don't think in this case there is an expected result other than not crashing, but yeah it may be worth adding in the commit message "for CVE-2020-25741, a {crash,undefined behavior,ASAN violation} in func_name". Paolo
On 210319 1054, Markus Armbruster wrote: > Paolo Bonzini <pbonzini@redhat.com> writes: > > > On 19/03/21 06:53, Markus Armbruster wrote: > >> I guess this is a reproducer. Please also describe actual and expected > >> result. Same for PATCH 2. > > > > Isn't it in the patch itself? > > A commit message should tell me what the patch is trying to accomplish. > > This commit message's title tells me it's a test for a CVE. Okay. The > body additionally gives me the reproducer. To be useful, a reproducer > needs to come with actual and expected result. Yes, I can find those in > the patch. But I could find the reproducer there, too. If you're nice > enough to save me the trouble of digging through the patch for the > reproducer (thanks), please consider saving me the trouble digging for > the information I need to make use of it (thanks again). That's all :) > > [...] > Ok sounds good. I posted this in-reply-to patch [1] from August 2020, which had a stacktrace, and I hoped that would provide enough context. However, that depends on the email-viewer and I see how that context would be lost if/once these reproducer patches are applied. [1] https://lore.kernel.org/qemu-devel/20200827113806.1850687-1-ppandit@redhat.com/ (https://lists.nongnu.org/archive doesn't display this discussion as a child of that message)
On 210319 1026, Paolo Bonzini wrote: > On 19/03/21 06:53, Markus Armbruster wrote: > > I guess this is a reproducer. Please also describe actual and expected > > result. Same for PATCH 2. > > Isn't it in the patch itself? > > Alexander, I think these reproducers are self-contained enough (no writes to > PCI configuration space to set up the device) that we can place them in > fdc-test.c. > Will do -Alex > Paolo >
diff --git a/tests/qtest/fuzz-test.c b/tests/qtest/fuzz-test.c index 00149abec7..62ececc66f 100644 --- a/tests/qtest/fuzz-test.c +++ b/tests/qtest/fuzz-test.c @@ -24,6 +24,58 @@ static void test_lp1878642_pci_bus_get_irq_level_assert(void) qtest_quit(s); } +/* + * ERROR: AddressSanitizer: SEGV on unknown address 0x000000000344 + * The signal is caused by a WRITE memory access. + * #0 0x555559d7f519 in blk_inc_in_flight /block/block-backend.c:1356:5 + * #1 0x555559d7f519 in blk_prw /block/block-backend.c:1328:5 + * #2 0x555559d81673 in blk_pwrite /block/block-backend.c:1501:15 + * #3 0x555558fc763f in fdctrl_write_data /hw/block/fdc.c:2414:17 + * #4 0x555558fc763f in fdctrl_write /hw/block/fdc.c:967:9 + * #5 0x5555594a622c in memory_region_write_accessor /softmmu/memory.c:491:5 + * #6 0x5555594a5c93 in access_with_adjusted_size /softmmu/memory.c:552:18 + * #7 0x5555594a54f0 in memory_region_dispatch_write /softmmu/memory.c + * #8 0x55555964a686 in flatview_write_continue /softmmu/physmem.c:2776:23 + * #9 0x55555963fde8 in flatview_write /softmmu/physmem.c:2816:14 + * #10 0x55555963fde8 in address_space_write /softmmu/physmem.c:2908:18 + * #11 0x55555968f8a1 in cpu_outb /softmmu/ioport.c:60:5 + * #12 0x5555597d5619 in qtest_process_command /softmmu/qtest.c:479:13 + * #13 0x5555597d34bf in qtest_process_inbuf /softmmu/qtest.c:797:9 + * #14 0x555559b4539d in fd_chr_read /chardev/char-fd.c:68:9 + * #15 0x7ffff7b7dc3e in g_main_context_dispatch + * #16 0x55555a4facdc in glib_pollfds_poll /util/main-loop.c:231:9 + * #17 0x55555a4facdc in os_host_main_loop_wait /util/main-loop.c:254:5 + * #18 0x55555a4facdc in main_loop_wait /util/main-loop.c:530:11 + * #19 0x555559a6dec9 in qemu_main_loop /softmmu/runstate.c:725:9 + * #20 0x5555581a3085 in main /softmmu/main.c:50:5 + */ +static void test_fdc_cve_2020_25741(void) +{ + QTestState *s; + int fd; + char tmpdisk[] = "/tmp/fda.XXXXXX"; + fd = mkstemp(tmpdisk); + assert(fd >= 0); + ftruncate(fd, 1440 * 1024); + close(fd); + + s = qtest_initf("-nographic -m 512M -nodefaults " + "-drive file=%s,format=raw,if=floppy", tmpdisk); + qtest_outw(s, 0x3f4, 0x0500); + qtest_outb(s, 0x3f5, 0x00); + qtest_outb(s, 0x3f5, 0x00); + qtest_outw(s, 0x3f4, 0x00); + qtest_outb(s, 0x3f5, 0x00); + qtest_outw(s, 0x3f1, 0x0400); + qtest_outw(s, 0x3f4, 0x0); + qtest_outw(s, 0x3f4, 0x00); + qtest_outb(s, 0x3f5, 0x0); + qtest_outb(s, 0x3f5, 0x01); + qtest_outw(s, 0x3f1, 0x0500); + qtest_outb(s, 0x3f5, 0x00); + qtest_quit(s); +} + int main(int argc, char **argv) { const char *arch = qtest_get_arch(); @@ -33,6 +85,8 @@ int main(int argc, char **argv) if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) { qtest_add_func("fuzz/test_lp1878642_pci_bus_get_irq_level_assert", test_lp1878642_pci_bus_get_irq_level_assert); + qtest_add_func("fuzz/test_fdc_cve_2020_25741", + test_fdc_cve_2020_25741); } return g_test_run();
dd if=/dev/zero of=/tmp/fda.img bs=1024 count=1440 cat << EOF | ./qemu-system-i386 -nographic -m 512M -nodefaults \ -accel qtest -fda /tmp/fda.img -qtest stdio outw 0x3f4 0x0500 outb 0x3f5 0x00 outb 0x3f5 0x00 outw 0x3f4 0x00 outb 0x3f5 0x00 outw 0x3f1 0x0400 outw 0x3f4 0x0 outw 0x3f4 0x00 outb 0x3f5 0x0 outb 0x3f5 0x01 outw 0x3f1 0x0500 outb 0x3f5 0x00 EOF Signed-off-by: Alexander Bulekov <alxndr@bu.edu> --- Might be useful for reproducing/regression testing tests/qtest/fuzz-test.c | 54 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+)