diff mbox series

[RFC,2/2] exec: Do not let flatview_read/write_continue do (too) short accesses

Message ID 20200517113804.9063-3-f4bug@amsat.org
State New
Headers show
Series exec: Fix (too) short device accesses | expand

Commit Message

Philippe Mathieu-Daudé May 17, 2020, 11:38 a.m. UTC 
Instead of accessing a device with an invalid short size,
return MEMTX_ERROR to indicate the transaction failed (as
the device won't accept the transaction anyway).

Reported by libFuzzer. sdhci_sdma_transfer_multi_blocks()
ends calling dma_memory_rw() with size < 4, while the DMA
MMIO regions are restricted to 32-bit accesses:

  qemu-fuzz-arm: hw/dma/bcm2835_dma.c:153: uint64_t bcm2835_dma_read(BCM2835DMAState *, hwaddr, unsigned int, unsigned int): Assertion `size == 4' failed.
  ==27332== ERROR: libFuzzer: deadly signal
    #8 0x7f0ffa1f5565 in __GI___assert_fail (/lib64/libc.so.6+0x30565)
    #9 0x562fe3c9c83f in bcm2835_dma_read (qemu-fuzz-arm+0x1d2e83f)
    #10 0x562fe3c9f81b in bcm2835_dma15_read (qemu-fuzz-arm+0x1d3181b)
    #11 0x562fe307d265 in memory_region_read_accessor (qemu-fuzz-arm+0x110f265)
    #12 0x562fe304ecb3 in access_with_adjusted_size (qemu-fuzz-arm+0x10e0cb3)
    #13 0x562fe304cb37 in memory_region_dispatch_read1 (qemu-fuzz-arm+0x10deb37)
    #14 0x562fe304c553 in memory_region_dispatch_read (qemu-fuzz-arm+0x10de553)
    #15 0x562fe2e7fd1d in flatview_read_continue (qemu-fuzz-arm+0xf11d1d)
    #16 0x562fe2e8147d in flatview_read (qemu-fuzz-arm+0xf1347d)
    #17 0x562fe2e80fd4 in address_space_read_full (qemu-fuzz-arm+0xf12fd4)
    #18 0x562fe2e820fa in address_space_rw (qemu-fuzz-arm+0xf140fa)
    #19 0x562fe411e485 in dma_memory_rw_relaxed (qemu-fuzz-arm+0x21b0485)
    #20 0x562fe411deb5 in dma_memory_rw (qemu-fuzz-arm+0x21afeb5)
    #21 0x562fe411d837 in dma_memory_read (qemu-fuzz-arm+0x21af837)
    #22 0x562fe41190a6 in sdhci_sdma_transfer_multi_blocks (qemu-fuzz-arm+0x21ab0a6)
    #23 0x562fe41217c1 in sdhci_write (qemu-fuzz-arm+0x21b37c1)
    #24 0x562fe304f147 in memory_region_write_accessor (qemu-fuzz-arm+0x10e1147)
    #25 0x562fe304ecb3 in access_with_adjusted_size (qemu-fuzz-arm+0x10e0cb3)
    #26 0x562fe304d853 in memory_region_dispatch_write (qemu-fuzz-arm+0x10df853)
    #27 0x562fe2e91e0b in flatview_write_continue (qemu-fuzz-arm+0xf23e0b)
    #28 0x562fe2e81d02 in flatview_write (qemu-fuzz-arm+0xf13d02)
    #29 0x562fe2e81834 in address_space_write (qemu-fuzz-arm+0xf13834)

  qemu-fuzz-arm: hw/dma/bcm2835_dma.c:200: void bcm2835_dma_write(BCM2835DMAState *, hwaddr, uint64_t, unsigned int, unsigned int): Assertion `size == 4' failed.
  ==16113== ERROR: libFuzzer: deadly signal
    #8 0x7fd823d3d565 in __GI___assert_fail (/lib64/libc.so.6+0x30565)
    #9 0x557a62b72ec3 in bcm2835_dma_write (qemu-fuzz-arm+0x1d2eec3)
    #10 0x557a62b725e8 in bcm2835_dma0_write (qemu-fuzz-arm+0x1d2e5e8)
    #11 0x557a61f25147 in memory_region_write_accessor (qemu-fuzz-arm+0x10e1147)
    #12 0x557a61f24cb3 in access_with_adjusted_size (qemu-fuzz-arm+0x10e0cb3)
    #13 0x557a61f23853 in memory_region_dispatch_write (qemu-fuzz-arm+0x10df853)
    #14 0x557a61d67e0b in flatview_write_continue (qemu-fuzz-arm+0xf23e0b)
    #15 0x557a61d57d02 in flatview_write (qemu-fuzz-arm+0xf13d02)
    #16 0x557a61d57834 in address_space_write (qemu-fuzz-arm+0xf13834)
    #17 0x557a61d58054 in address_space_rw (qemu-fuzz-arm+0xf14054)
    #18 0x557a62ff4485 in dma_memory_rw_relaxed (qemu-fuzz-arm+0x21b0485)
    #19 0x557a62ff3eb5 in dma_memory_rw (qemu-fuzz-arm+0x21afeb5)
    #20 0x557a62ff379a in dma_memory_write (qemu-fuzz-arm+0x21af79a)
    #21 0x557a62fee9dc in sdhci_sdma_transfer_multi_blocks (qemu-fuzz-arm+0x21aa9dc)
    #22 0x557a62ff77c1 in sdhci_write (qemu-fuzz-arm+0x21b37c1)
    #23 0x557a61f25147 in memory_region_write_accessor (qemu-fuzz-arm+0x10e1147)
    #24 0x557a61f24cb3 in access_with_adjusted_size (qemu-fuzz-arm+0x10e0cb3)
    #25 0x557a61f23853 in memory_region_dispatch_write (qemu-fuzz-arm+0x10df853)
    #26 0x557a61d67e0b in flatview_write_continue (qemu-fuzz-arm+0xf23e0b)
    #27 0x557a61d57d02 in flatview_write (qemu-fuzz-arm+0xf13d02)
    #28 0x557a61d57834 in address_space_write (qemu-fuzz-arm+0xf13834)

  ==5448==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x619000024380 at pc 0x55aac095e2cc bp 0x7fff9144ead0 sp 0x7fff9144e280
  WRITE of size 4 at 0x619000024380 thread T0
    #0 0x55aac095e2cb in __asan_memcpy (qemu-fuzz-arm+0xeb92cb)
    #1 0x55aac09de163 in stl_he_p (qemu-fuzz-arm+0xf39163)
    #2 0x55aac09b796f in stn_he_p (qemu-fuzz-arm+0xf1296f)
    #3 0x55aac09b6ec5 in flatview_read_continue (qemu-fuzz-arm+0xf11ec5)
    #4 0x55aac09b86dd in flatview_read (qemu-fuzz-arm+0xf136dd)
    #5 0x55aac09b8234 in address_space_read_full (qemu-fuzz-arm+0xf13234)
    #6 0x55aac09b935a in address_space_rw (qemu-fuzz-arm+0xf1435a)
    #7 0x55aac1c55b35 in dma_memory_rw_relaxed (qemu-fuzz-arm+0x21b0b35)
    #8 0x55aac1c55565 in dma_memory_rw (qemu-fuzz-arm+0x21b0565)
    #9 0x55aac1c54ee7 in dma_memory_read (qemu-fuzz-arm+0x21afee7)
    #10 0x55aac1c5074e in sdhci_sdma_transfer_multi_blocks (qemu-fuzz-arm+0x21ab74e)
    #11 0x55aac1c58e71 in sdhci_write (qemu-fuzz-arm+0x21b3e71)
    #12 0x55aac0b86417 in memory_region_write_accessor (qemu-fuzz-arm+0x10e1417)
    #13 0x55aac0b85f87 in access_with_adjusted_size (qemu-fuzz-arm+0x10e0f87)
    #14 0x55aac0b84ab3 in memory_region_dispatch_write (qemu-fuzz-arm+0x10dfab3)
    #15 0x55aac09c906b in flatview_write_continue (qemu-fuzz-arm+0xf2406b)
    #16 0x55aac09b8f62 in flatview_write (qemu-fuzz-arm+0xf13f62)
    #17 0x55aac09b8a94 in address_space_write (qemu-fuzz-arm+0xf13a94)

Reported-by: Clang combined libFuzzer with AddressSanitizer
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 exec.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)
diff mbox series

Patch

diff --git a/exec.c b/exec.c
index d3ec30f995..100c2754f2 100644
--- a/exec.c
+++ b/exec.c
@@ -3136,13 +3136,20 @@  static MemTxResult flatview_write_continue(FlatView *fv, hwaddr addr,
 
     for (;;) {
         if (!memory_access_is_direct(mr, true)) {
+            /* I/O case */
+            hwaddr l2;
+
             release_lock |= prepare_mmio_access(mr);
-            l = memory_access_size(mr, l, addr1);
+            l2 = memory_access_size(mr, l, addr1);
             /* XXX: could force current_cpu to NULL to avoid
                potential bugs */
-            val = ldn_he_p(buf, l);
-            result |= memory_region_dispatch_write(mr, addr1, val,
-                                                   size_memop(l), attrs);
+            if (l <= l2) {
+                val = ldn_he_p(buf, l);
+                result |= memory_region_dispatch_write(mr, addr1, val,
+                                                       size_memop(l), attrs);
+            } else {
+                result = MEMTX_ERROR;
+            }
         } else {
             /* RAM case */
             ram_ptr = qemu_ram_ptr_length(mr->ram_block, addr1, &l, false);
@@ -3202,11 +3209,17 @@  MemTxResult flatview_read_continue(FlatView *fv, hwaddr addr,
     for (;;) {
         if (!memory_access_is_direct(mr, false)) {
             /* I/O case */
+            hwaddr l2;
+
             release_lock |= prepare_mmio_access(mr);
-            l = memory_access_size(mr, l, addr1);
-            result |= memory_region_dispatch_read(mr, addr1, &val,
-                                                  size_memop(l), attrs);
-            stn_he_p(buf, l, val);
+            l2 = memory_access_size(mr, l, addr1);
+            if (l <= l2) {
+                result |= memory_region_dispatch_read(mr, addr1, &val,
+                                                      size_memop(l), attrs);
+                stn_he_p(buf, l, val);
+            } else {
+                result = MEMTX_ERROR;
+            }
         } else {
             /* RAM case */
             ram_ptr = qemu_ram_ptr_length(mr->ram_block, addr1, &l, false);