| Message ID | 20211124161536.631563-4-philmd@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/block/fdc: Fix CVE-2021-20196 | expand |
On 24.11.21 17:15, Philippe Mathieu-Daudé wrote: > Without the previous commit, when running 'make check-qtest-i386' > with QEMU configured with '--enable-sanitizers' we get: > > AddressSanitizer:DEADLYSIGNAL > ================================================================= > ==287878==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000344 > ==287878==The signal is caused by a WRITE memory access. > ==287878==Hint: address points to the zero page. > #0 0x564b2e5bac27 in blk_inc_in_flight block/block-backend.c:1346:5 > #1 0x564b2e5bb228 in blk_pwritev_part block/block-backend.c:1317:5 > #2 0x564b2e5bcd57 in blk_pwrite block/block-backend.c:1498:11 > #3 0x564b2ca1cdd3 in fdctrl_write_data hw/block/fdc.c:2221:17 > #4 0x564b2ca1b2f7 in fdctrl_write hw/block/fdc.c:829:9 > #5 0x564b2dc49503 in portio_write softmmu/ioport.c:201:9 > > Add the reproducer for CVE-2021-20196. > > Suggested-by: Alexander Bulekov <alxndr@bu.edu> > Reviewed-by: Darren Kenny <darren.kenny@oracle.com> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > tests/qtest/fdc-test.c | 38 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 38 insertions(+) > > diff --git a/tests/qtest/fdc-test.c b/tests/qtest/fdc-test.c > index 26b69f7c5cd..8f6eee84a47 100644 > --- a/tests/qtest/fdc-test.c > +++ b/tests/qtest/fdc-test.c > @@ -32,6 +32,9 @@ > /* TODO actually test the results and get rid of this */ > #define qmp_discard_response(...) qobject_unref(qmp(__VA_ARGS__)) > > +#define DRIVE_FLOPPY_BLANK \ > + "-drive if=floppy,file=null-co://,file.read-zeroes=on,format=raw,size=1440k" > + > #define TEST_IMAGE_SIZE 1440 * 1024 > > #define FLOPPY_BASE 0x3f0 > @@ -546,6 +549,40 @@ static void fuzz_registers(void) > } > } > > +static bool qtest_check_clang_sanitizer(void) > +{ > +#if defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer) > + return true; > +#else > + g_test_skip("QEMU not configured using --enable-sanitizers"); > + return false; > +#endif > +} > +static void test_cve_2021_20196(void) > +{ > + QTestState *s; > + > + if (!qtest_check_clang_sanitizer()) { > + return; > + } > + > + s = qtest_initf("-nographic -m 32M -nodefaults " DRIVE_FLOPPY_BLANK); > + > + qtest_outw(s, 0x3f4, 0x0500); > + qtest_outb(s, 0x3f5, 0x00); > + qtest_outb(s, 0x3f5, 0x00); > + qtest_outw(s, 0x3f4, 0x0000); > + qtest_outb(s, 0x3f5, 0x00); > + qtest_outw(s, 0x3f1, 0x0400); > + qtest_outw(s, 0x3f4, 0x0000); > + qtest_outw(s, 0x3f4, 0x0000); > + qtest_outb(s, 0x3f5, 0x00); > + qtest_outb(s, 0x3f5, 0x01); > + qtest_outw(s, 0x3f1, 0x0500); > + qtest_outb(s, 0x3f5, 0x00); > + qtest_quit(s); > +} > + Now this works as a reproducer for me, but... this is a completely different I/O sequence now, right? Can’t complain, though, I didn’t understand the previous one, I can’t claim I need to understand this one or why they’re different. All the rest looks good to me, so all in all: Reviewed-by: Hanna Reitz <hreitz@redhat.com>
On 11/25/21 12:57, Hanna Reitz wrote: > On 24.11.21 17:15, Philippe Mathieu-Daudé wrote: >> Without the previous commit, when running 'make check-qtest-i386' >> with QEMU configured with '--enable-sanitizers' we get: >> >> AddressSanitizer:DEADLYSIGNAL >> ================================================================= >> ==287878==ERROR: AddressSanitizer: SEGV on unknown address >> 0x000000000344 >> ==287878==The signal is caused by a WRITE memory access. >> ==287878==Hint: address points to the zero page. >> #0 0x564b2e5bac27 in blk_inc_in_flight >> block/block-backend.c:1346:5 >> #1 0x564b2e5bb228 in blk_pwritev_part block/block-backend.c:1317:5 >> #2 0x564b2e5bcd57 in blk_pwrite block/block-backend.c:1498:11 >> #3 0x564b2ca1cdd3 in fdctrl_write_data hw/block/fdc.c:2221:17 >> #4 0x564b2ca1b2f7 in fdctrl_write hw/block/fdc.c:829:9 >> #5 0x564b2dc49503 in portio_write softmmu/ioport.c:201:9 >> >> Add the reproducer for CVE-2021-20196. >> >> Suggested-by: Alexander Bulekov <alxndr@bu.edu> >> Reviewed-by: Darren Kenny <darren.kenny@oracle.com> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> --- >> tests/qtest/fdc-test.c | 38 ++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 38 insertions(+) >> >> diff --git a/tests/qtest/fdc-test.c b/tests/qtest/fdc-test.c >> index 26b69f7c5cd..8f6eee84a47 100644 >> --- a/tests/qtest/fdc-test.c >> +++ b/tests/qtest/fdc-test.c >> @@ -32,6 +32,9 @@ >> /* TODO actually test the results and get rid of this */ >> #define qmp_discard_response(...) qobject_unref(qmp(__VA_ARGS__)) >> +#define DRIVE_FLOPPY_BLANK \ >> + "-drive >> if=floppy,file=null-co://,file.read-zeroes=on,format=raw,size=1440k" >> + >> #define TEST_IMAGE_SIZE 1440 * 1024 >> #define FLOPPY_BASE 0x3f0 >> @@ -546,6 +549,40 @@ static void fuzz_registers(void) >> } >> } >> +static bool qtest_check_clang_sanitizer(void) >> +{ >> +#if defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer) >> + return true; >> +#else >> + g_test_skip("QEMU not configured using --enable-sanitizers"); >> + return false; >> +#endif >> +} >> +static void test_cve_2021_20196(void) >> +{ >> + QTestState *s; >> + >> + if (!qtest_check_clang_sanitizer()) { >> + return; >> + } >> + >> + s = qtest_initf("-nographic -m 32M -nodefaults " >> DRIVE_FLOPPY_BLANK); >> + >> + qtest_outw(s, 0x3f4, 0x0500); >> + qtest_outb(s, 0x3f5, 0x00); >> + qtest_outb(s, 0x3f5, 0x00); >> + qtest_outw(s, 0x3f4, 0x0000); >> + qtest_outb(s, 0x3f5, 0x00); >> + qtest_outw(s, 0x3f1, 0x0400); >> + qtest_outw(s, 0x3f4, 0x0000); >> + qtest_outw(s, 0x3f4, 0x0000); >> + qtest_outb(s, 0x3f5, 0x00); >> + qtest_outb(s, 0x3f5, 0x01); >> + qtest_outw(s, 0x3f1, 0x0500); >> + qtest_outb(s, 0x3f5, 0x00); >> + qtest_quit(s); >> +} >> + > > Now this works as a reproducer for me, but... this is a completely > different I/O sequence now, right? The patch Alexander sent [*] was indeed not working, but I could manually reproduce, then I figure while the commit *description* was working, the patch *content* was not accurate. This patch uses the commit description. [1] https://www.mail-archive.com/qemu-block@nongnu.org/msg82825.html > Can’t complain, though, I didn’t understand the previous one, I can’t > claim I need to understand this one or why they’re different. Same here =) > All the rest looks good to me, so all in all: > > Reviewed-by: Hanna Reitz <hreitz@redhat.com> Thank you!
diff --git a/tests/qtest/fdc-test.c b/tests/qtest/fdc-test.c index 26b69f7c5cd..8f6eee84a47 100644 --- a/tests/qtest/fdc-test.c +++ b/tests/qtest/fdc-test.c @@ -32,6 +32,9 @@ /* TODO actually test the results and get rid of this */ #define qmp_discard_response(...) qobject_unref(qmp(__VA_ARGS__)) +#define DRIVE_FLOPPY_BLANK \ + "-drive if=floppy,file=null-co://,file.read-zeroes=on,format=raw,size=1440k" + #define TEST_IMAGE_SIZE 1440 * 1024 #define FLOPPY_BASE 0x3f0 @@ -546,6 +549,40 @@ static void fuzz_registers(void) } } +static bool qtest_check_clang_sanitizer(void) +{ +#if defined(__SANITIZE_ADDRESS__) || __has_feature(address_sanitizer) + return true; +#else + g_test_skip("QEMU not configured using --enable-sanitizers"); + return false; +#endif +} +static void test_cve_2021_20196(void) +{ + QTestState *s; + + if (!qtest_check_clang_sanitizer()) { + return; + } + + s = qtest_initf("-nographic -m 32M -nodefaults " DRIVE_FLOPPY_BLANK); + + qtest_outw(s, 0x3f4, 0x0500); + qtest_outb(s, 0x3f5, 0x00); + qtest_outb(s, 0x3f5, 0x00); + qtest_outw(s, 0x3f4, 0x0000); + qtest_outb(s, 0x3f5, 0x00); + qtest_outw(s, 0x3f1, 0x0400); + qtest_outw(s, 0x3f4, 0x0000); + qtest_outw(s, 0x3f4, 0x0000); + qtest_outb(s, 0x3f5, 0x00); + qtest_outb(s, 0x3f5, 0x01); + qtest_outw(s, 0x3f1, 0x0500); + qtest_outb(s, 0x3f5, 0x00); + qtest_quit(s); +} + int main(int argc, char **argv) { int fd; @@ -576,6 +613,7 @@ int main(int argc, char **argv) qtest_add_func("/fdc/read_no_dma_18", test_read_no_dma_18); qtest_add_func("/fdc/read_no_dma_19", test_read_no_dma_19); qtest_add_func("/fdc/fuzz-registers", fuzz_registers); + qtest_add_func("/fdc/fuzz/cve_2021_20196", test_cve_2021_20196); ret = g_test_run();
Without the previous commit, when running 'make check-qtest-i386' with QEMU configured with '--enable-sanitizers' we get: AddressSanitizer:DEADLYSIGNAL ================================================================= ==287878==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000344 ==287878==The signal is caused by a WRITE memory access. ==287878==Hint: address points to the zero page. #0 0x564b2e5bac27 in blk_inc_in_flight block/block-backend.c:1346:5 #1 0x564b2e5bb228 in blk_pwritev_part block/block-backend.c:1317:5 #2 0x564b2e5bcd57 in blk_pwrite block/block-backend.c:1498:11 #3 0x564b2ca1cdd3 in fdctrl_write_data hw/block/fdc.c:2221:17 #4 0x564b2ca1b2f7 in fdctrl_write hw/block/fdc.c:829:9 #5 0x564b2dc49503 in portio_write softmmu/ioport.c:201:9 Add the reproducer for CVE-2021-20196. Suggested-by: Alexander Bulekov <alxndr@bu.edu> Reviewed-by: Darren Kenny <darren.kenny@oracle.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- tests/qtest/fdc-test.c | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)