qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
To: qemu-devel@nongnu.org
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	"Li Zhijian" <lizhijian@cn.fujitsu.com>,
	"Tony Nguyen" <tony.nguyen@bt.com>,
	"Alexey Kardashevskiy" <aik@ozlabs.ru>,
	"Julia Suvorova" <jusual@redhat.com>,
	"Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Peter Xu" <peterx@redhat.com>,
	"Alexander Bulekov" <alxndr@bu.edu>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Richard Henderson" <rth@twiddle.net>,
	"Stefano Garzarella" <sgarzare@redhat.com>
Subject: [RFC PATCH 2/2] exec: Do not let flatview_read/write_continue do (too) short accesses
Date: Sun, 17 May 2020 13:38:04 +0200	[thread overview]
Message-ID: <20200517113804.9063-3-f4bug@amsat.org> (raw)
In-Reply-To: <20200517113804.9063-1-f4bug@amsat.org>

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 --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);
-- 
2.21.3



  parent reply	other threads:[~2020-05-17 11:41 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-17 11:38 [RFC PATCH 0/2] exec: Fix (too) short device accesses Philippe Mathieu-Daudé
2020-05-17 11:38 ` [RFC PATCH 1/2] exec: Let memory_access_size() consider minimum valid access size Philippe Mathieu-Daudé
2020-05-17 11:38 ` Philippe Mathieu-Daudé [this message]
2020-05-17 13:51 ` [RFC PATCH 0/2] exec: Fix (too) short device accesses no-reply
2020-05-17 15:50   ` Philippe Mathieu-Daudé
2020-05-17 14:20 ` no-reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200517113804.9063-3-f4bug@amsat.org \
    --to=f4bug@amsat.org \
    --cc=aik@ozlabs.ru \
    --cc=alxndr@bu.edu \
    --cc=jusual@redhat.com \
    --cc=lizhijian@cn.fujitsu.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=peterx@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    --cc=sgarzare@redhat.com \
    --cc=tony.nguyen@bt.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).