[PATCH] hw/char/virtio-serial-bus: Fix Heap-buffer-overflow in set_config()

Philippe Mathieu-Daudé posted 1 patch 1 month, 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260216205527.45938-1-philmd@linaro.org
Maintainers: Laurent Vivier <lvivier@redhat.com>, Amit Shah <amit@kernel.org>, "Michael S. Tsirkin" <mst@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
hw/char/virtio-serial-bus.c | 4 ----
1 file changed, 4 deletions(-)
[PATCH] hw/char/virtio-serial-bus: Fix Heap-buffer-overflow in set_config()
Posted by Philippe Mathieu-Daudé 1 month, 3 weeks ago
When removing the 'emergency-write' property in commit d0660e5b7fc
we neglected to remove the code reducing the virtio_console_config
structure size, allowing to access up to the unallocated 'emerg_wr'
field.

Can be reproduced running:

  $ cat << EOF | qemu-system-i386 -nodefaults \
                     -machine q35 -m 512M \
                     -device virtio-serial \
                     -display none \
                     -machine accel=qtest -qtest stdio
  outl 0xcf8 0x80000810
  outl 0xcfc 0xc000
  outl 0xcf8 0x80000804
  outw 0xcfc 0x01
  outl 0xc014 0x00
  EOF
  ==3210206==ERROR: AddressSanitizer: heap-buffer-overflow
      on address 0x502000090858 at pc 0x5638f1300a9b bp 0x7fff6b525b80 sp 0x7fff6b525b70
  READ of size 4 at 0x502000090858 thread T0
      #0 0x5638f1300a9a in set_config hw/char/virtio-serial-bus.c:590
      #1 0x5638f0bccdcf in virtio_config_writel hw/virtio/virtio-config-io.c:104
      #2 0x5638f0bd0c89 in virtio_pci_config_write hw/virtio/virtio-pci.c:637
      #3 0x5638f0cf90cf in memory_region_write_accessor system/memory.c:491
      #4 0x5638f0cf975b in access_with_adjusted_size system/memory.c:567
      #5 0x5638f0d01d3f in memory_region_dispatch_write system/memory.c:1547
      #6 0x5638f0d2fa1e in address_space_stm_internal system/memory_ldst.c.inc:85
      #7 0x5638f0d30013 in address_space_stl_le system/memory_ldst_endian.c.inc:53
      #8 0x5638f0ceb568 in cpu_outl system/ioport.c:79
      #9 0x5638f0d3c0f9 in qtest_process_command system/qtest.c:483

  0x502000090858 is located 0 bytes to the right of 8-byte region [0x502000090850,0x502000090858)
  allocated by thread T0 here:
      #0 0x7f0dc32cba57 in __interceptor_calloc src/libsanitizer/asan/asan_malloc_linux.cpp:154
      #1 0x7f0dc2382c50 in g_malloc0 (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x5ec50)
      #2 0x5638f1303c27 in virtio_serial_device_realize hw/char/virtio-serial-bus.c:1046
      #3 0x5638f1396a9c in virtio_device_realize hw/virtio/virtio.c:4053
      #4 0x5638f13ea370 in device_set_realized hw/core/qdev.c:523
      #5 0x5638f13fdaf6 in property_set_bool qom/object.c:2376
      #6 0x5638f13f9098 in object_property_set qom/object.c:1450
      #7 0x5638f140283c in object_property_set_qobject qom/qom-qobject.c:28
      #8 0x5638f13f9616 in object_property_set_bool qom/object.c:1520
      #9 0x5638f13e91cc in qdev_realize hw/core/qdev.c:276
      #10 0x5638f0c3d94b in virtio_serial_pci_realize hw/virtio/virtio-serial-pci.c:69
      #11 0x5638f0bda886 in virtio_pci_realize hw/virtio/virtio-pci.c:2351
      #12 0x5638f09bc2ae in pci_qdev_realize hw/pci/pci.c:2310
      #13 0x5638f0bdb2f2 in virtio_pci_dc_realize hw/virtio/virtio-pci.c:2473
      #14 0x5638f13ea370 in device_set_realized hw/core/qdev.c:523

    SUMMARY: AddressSanitizer: heap-buffer-overflow hw/char/virtio-serial-bus.c:590 in set_config

Fixes: d0660e5b7fc ("hw/char/virtio-serial: Do not expose the 'emergency-write' property")
Reported-by: Alexander Bulekov <alxndr@bu.edu>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3303
Buglink: https://issues.oss-fuzz.com/issues/484647006
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/char/virtio-serial-bus.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index b7c57ea9678..cd234dc6db1 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -1039,10 +1039,6 @@ static void virtio_serial_device_realize(DeviceState *dev, Error **errp)
         return;
     }
 
-    if (!virtio_has_feature(vdev->host_features,
-                            VIRTIO_CONSOLE_F_EMERG_WRITE)) {
-        config_size = offsetof(struct virtio_console_config, emerg_wr);
-    }
     virtio_init(vdev, VIRTIO_ID_CONSOLE, config_size);
 
     /* Spawn a new virtio-serial bus on which the ports will ride as devices */
-- 
2.52.0


Re: [PATCH] hw/char/virtio-serial-bus: Fix Heap-buffer-overflow in set_config()
Posted by Alexander Bulekov 1 month, 3 weeks ago
On 260216 2155, Philippe Mathieu-Daudé wrote:
> When removing the 'emergency-write' property in commit d0660e5b7fc
> we neglected to remove the code reducing the virtio_console_config
> structure size, allowing to access up to the unallocated 'emerg_wr'
> field.
>
> Can be reproduced running:
>
>   $ cat << EOF | qemu-system-i386 -nodefaults \
>                      -machine q35 -m 512M \
>                      -device virtio-serial \
>                      -display none \
>                      -machine accel=qtest -qtest stdio
>   outl 0xcf8 0x80000810
>   outl 0xcfc 0xc000
>   outl 0xcf8 0x80000804
>   outw 0xcfc 0x01
>   outl 0xc014 0x00
>   EOF
>   ==3210206==ERROR: AddressSanitizer: heap-buffer-overflow
>       on address 0x502000090858 at pc 0x5638f1300a9b bp 0x7fff6b525b80 sp 0x7fff6b525b70
>   READ of size 4 at 0x502000090858 thread T0
>       #0 0x5638f1300a9a in set_config hw/char/virtio-serial-bus.c:590
>       #1 0x5638f0bccdcf in virtio_config_writel hw/virtio/virtio-config-io.c:104
>       #2 0x5638f0bd0c89 in virtio_pci_config_write hw/virtio/virtio-pci.c:637
>       #3 0x5638f0cf90cf in memory_region_write_accessor system/memory.c:491
>       #4 0x5638f0cf975b in access_with_adjusted_size system/memory.c:567
>       #5 0x5638f0d01d3f in memory_region_dispatch_write system/memory.c:1547
>       #6 0x5638f0d2fa1e in address_space_stm_internal system/memory_ldst.c.inc:85
>       #7 0x5638f0d30013 in address_space_stl_le system/memory_ldst_endian.c.inc:53
>       #8 0x5638f0ceb568 in cpu_outl system/ioport.c:79
>       #9 0x5638f0d3c0f9 in qtest_process_command system/qtest.c:483
>
>   0x502000090858 is located 0 bytes to the right of 8-byte region [0x502000090850,0x502000090858)
>   allocated by thread T0 here:
>       #0 0x7f0dc32cba57 in __interceptor_calloc src/libsanitizer/asan/asan_malloc_linux.cpp:154
>       #1 0x7f0dc2382c50 in g_malloc0 (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x5ec50)
>       #2 0x5638f1303c27 in virtio_serial_device_realize hw/char/virtio-serial-bus.c:1046
>       #3 0x5638f1396a9c in virtio_device_realize hw/virtio/virtio.c:4053
>       #4 0x5638f13ea370 in device_set_realized hw/core/qdev.c:523
>       #5 0x5638f13fdaf6 in property_set_bool qom/object.c:2376
>       #6 0x5638f13f9098 in object_property_set qom/object.c:1450
>       #7 0x5638f140283c in object_property_set_qobject qom/qom-qobject.c:28
>       #8 0x5638f13f9616 in object_property_set_bool qom/object.c:1520
>       #9 0x5638f13e91cc in qdev_realize hw/core/qdev.c:276
>       #10 0x5638f0c3d94b in virtio_serial_pci_realize hw/virtio/virtio-serial-pci.c:69
>       #11 0x5638f0bda886 in virtio_pci_realize hw/virtio/virtio-pci.c:2351
>       #12 0x5638f09bc2ae in pci_qdev_realize hw/pci/pci.c:2310
>       #13 0x5638f0bdb2f2 in virtio_pci_dc_realize hw/virtio/virtio-pci.c:2473
>       #14 0x5638f13ea370 in device_set_realized hw/core/qdev.c:523
>
>     SUMMARY: AddressSanitizer: heap-buffer-overflow hw/char/virtio-serial-bus.c:590 in set_config
>
> Fixes: d0660e5b7fc ("hw/char/virtio-serial: Do not expose the 'emergency-write' property")
> Reported-by: Alexander Bulekov <alxndr@bu.edu>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3303
> Buglink: https://issues.oss-fuzz.com/issues/484647006
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Tested-by: Alexander Bulekov <alxndr@bu.edu>

Thank you

> ---
>  hw/char/virtio-serial-bus.c | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
> index b7c57ea9678..cd234dc6db1 100644
> --- a/hw/char/virtio-serial-bus.c
> +++ b/hw/char/virtio-serial-bus.c
> @@ -1039,10 +1039,6 @@ static void virtio_serial_device_realize(DeviceState *dev, Error **errp)
>          return;
>      }
>
> -    if (!virtio_has_feature(vdev->host_features,
> -                            VIRTIO_CONSOLE_F_EMERG_WRITE)) {
> -        config_size = offsetof(struct virtio_console_config, emerg_wr);
> -    }
>      virtio_init(vdev, VIRTIO_ID_CONSOLE, config_size);
>
>      /* Spawn a new virtio-serial bus on which the ports will ride as devices */
> --
> 2.52.0
>