On Tue, 9 Apr 2024 at 15:55, Philippe Mathieu-Daudé <phi...@linaro.org> wrote: > > Per "SD Host Controller Standard Specification Version 3.00": > > * 2.2.5 Transfer Mode Register (Offset 00Ch) > > Writes to this register shall be ignored when the Command > Inhibit (DAT) in the Present State register is 1. > > Do not update the TRNMOD register when Command Inhibit (DAT) > bit is set to avoid the present-status register going out of > sync, leading to malicious guest using DMA mode and overflowing > the FIFO buffer: > > $ cat << EOF | qemu-system-i386 \ > -display none -nodefaults \ > -machine accel=qtest -m 512M \ > -device sdhci-pci,sd-spec-version=3 \ > -device sd-card,drive=mydrive \ > -drive > if=none,index=0,file=null-co://,format=raw,id=mydrive -nographic \ > -qtest stdio > outl 0xcf8 0x80001013 > outl 0xcfc 0x91 > outl 0xcf8 0x80001001 > outl 0xcfc 0x06000000 > write 0x9100002c 0x1 0x05 > write 0x91000058 0x1 0x16 > write 0x91000005 0x1 0x04 > write 0x91000028 0x1 0x08 > write 0x16 0x1 0x21 > write 0x19 0x1 0x20 > write 0x9100000c 0x1 0x01 > write 0x9100000e 0x1 0x20 > write 0x9100000f 0x1 0x00 > write 0x9100000c 0x1 0x00 > write 0x91000020 0x1 0x00 > EOF > > Stack trace (part): > ================================================================= > ==89993==ERROR: AddressSanitizer: heap-buffer-overflow on address > 0x615000029900 at pc 0x55d5f885700d bp 0x7ffc1e1e9470 sp 0x7ffc1e1e9468 > WRITE of size 1 at 0x615000029900 thread T0 > #0 0x55d5f885700c in sdhci_write_dataport hw/sd/sdhci.c:564:39 > #1 0x55d5f8849150 in sdhci_write hw/sd/sdhci.c:1223:13 > #2 0x55d5fa01db63 in memory_region_write_accessor system/memory.c:497:5 > #3 0x55d5fa01d245 in access_with_adjusted_size system/memory.c:573:18 > #4 0x55d5fa01b1a9 in memory_region_dispatch_write system/memory.c:1521:16 > #5 0x55d5fa09f5c9 in flatview_write_continue system/physmem.c:2711:23 > #6 0x55d5fa08f78b in flatview_write system/physmem.c:2753:12 > #7 0x55d5fa08f258 in address_space_write system/physmem.c:2860:18 > ... > 0x615000029900 is located 0 bytes to the right of 512-byte region > [0x615000029700,0x615000029900) allocated by thread T0 here: > #0 0x55d5f7237b27 in __interceptor_calloc > #1 0x7f9e36dd4c50 in g_malloc0 > #2 0x55d5f88672f7 in sdhci_pci_realize hw/sd/sdhci-pci.c:36:5 > #3 0x55d5f844b582 in pci_qdev_realize hw/pci/pci.c:2092:9 > #4 0x55d5fa2ee74b in device_set_realized hw/core/qdev.c:510:13 > #5 0x55d5fa325bfb in property_set_bool qom/object.c:2358:5 > #6 0x55d5fa31ea45 in object_property_set qom/object.c:1472:5 > #7 0x55d5fa332509 in object_property_set_qobject om/qom-qobject.c:28:10 > #8 0x55d5fa31f6ed in object_property_set_bool qom/object.c:1541:15 > #9 0x55d5fa2e2948 in qdev_realize hw/core/qdev.c:292:12 > #10 0x55d5f8eed3f1 in qdev_device_add_from_qdict > system/qdev-monitor.c:719:10 > #11 0x55d5f8eef7ff in qdev_device_add system/qdev-monitor.c:738:11 > #12 0x55d5f8f211f0 in device_init_func system/vl.c:1200:11 > #13 0x55d5fad0877d in qemu_opts_foreach util/qemu-option.c:1135:14 > #14 0x55d5f8f0df9c in qemu_create_cli_devices system/vl.c:2638:5 > #15 0x55d5f8f0db24 in qmp_x_exit_preconfig system/vl.c:2706:5 > #16 0x55d5f8f14dc0 in qemu_init system/vl.c:3737:9 > ... > SUMMARY: AddressSanitizer: heap-buffer-overflow hw/sd/sdhci.c:564:39 > in sdhci_write_dataport > > Add assertions to ensure the fifo_buffer[] is not overflowed by > malicious accesses to the Buffer Data Port register. > > Fixes: CVE-2024-3447 > Cc: qemu-sta...@nongnu.org > Fixes: d7dfca0807 ("hw/sdhci: introduce standard SD host controller") > Buglink: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=58813 > Reported-by: Alexander Bulekov <alx...@bu.edu> > Reported-by: Chuhong Yuan <hsleste...@gmail.com> > Suggested-by: Peter Maydell <peter.mayd...@linaro.org> > Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org> > --- > Peter, since it is your patch, can I replace the Suggested-by your > S-o-b tag?
Sure, you can have my Signed-off-by or Reviewed-by tag, whichever you prefer. thanks -- PMM