All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Philippe Mathieu-Daudé" <philmd@linaro.org>
To: qemu-devel@nongnu.org
Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org>,
	qemu-stable@nongnu.org, "Qiang Liu" <cyruscyliu@gmail.com>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"Kevin Wolf" <kwolf@redhat.com>,
	"Hanna Reitz" <hreitz@redhat.com>,
	qemu-block@nongnu.org
Subject: [PULL 08/16] hw/block/nand: Fix out-of-bound access in NAND block buffer
Date: Wed, 10 Apr 2024 11:13:07 +0200	[thread overview]
Message-ID: <20240410091315.57241-9-philmd@linaro.org> (raw)
In-Reply-To: <20240410091315.57241-1-philmd@linaro.org>

nand_command() and nand_getio() don't check @offset points
into the block, nor the available data length (s->iolen) is
not negative.

In order to fix:

- check the offset is in range in nand_blk_load_NAND_PAGE_SIZE(),
- do not set @iolen if blk_load() failed.

Reproducer:

  $ cat << EOF | qemu-system-arm -machine tosa \
                                 -monitor none -serial none \
                                 -display none -qtest stdio
  write 0x10000111 0x1 0xca
  write 0x10000104 0x1 0x47
  write 0x1000ca04 0x1 0xd7
  write 0x1000ca01 0x1 0xe0
  write 0x1000ca04 0x1 0x71
  write 0x1000ca00 0x1 0x50
  write 0x1000ca04 0x1 0xd7
  read 0x1000ca02 0x1
  write 0x1000ca01 0x1 0x10
  EOF

=================================================================
==15750==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61f000000de0
 at pc 0x560e61557210 bp 0x7ffcfc4a59f0 sp 0x7ffcfc4a59e8
READ of size 1 at 0x61f000000de0 thread T0
    #0 0x560e6155720f in mem_and hw/block/nand.c:101:20
    #1 0x560e6155ac9c in nand_blk_write_512 hw/block/nand.c:663:9
    #2 0x560e61544200 in nand_command hw/block/nand.c:293:13
    #3 0x560e6153cc83 in nand_setio hw/block/nand.c:520:13
    #4 0x560e61a0a69e in tc6393xb_nand_writeb hw/display/tc6393xb.c:380:13
    #5 0x560e619f9bf7 in tc6393xb_writeb hw/display/tc6393xb.c:524:9
    #6 0x560e647c7d03 in memory_region_write_accessor softmmu/memory.c:492:5
    #7 0x560e647c7641 in access_with_adjusted_size softmmu/memory.c:554:18
    #8 0x560e647c5f66 in memory_region_dispatch_write softmmu/memory.c:1514:16
    #9 0x560e6485409e in flatview_write_continue softmmu/physmem.c:2825:23
    #10 0x560e648421eb in flatview_write softmmu/physmem.c:2867:12
    #11 0x560e64841ca8 in address_space_write softmmu/physmem.c:2963:18
    #12 0x560e61170162 in qemu_writeb tests/qtest/videzzo/videzzo_qemu.c:1080:5
    #13 0x560e6116eef7 in dispatch_mmio_write tests/qtest/videzzo/videzzo_qemu.c:1227:28

0x61f000000de0 is located 0 bytes to the right of 3424-byte region [0x61f000000080,0x61f000000de0)
allocated by thread T0 here:
    #0 0x560e611276cf in malloc /root/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3
    #1 0x7f7959a87e98 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x57e98)
    #2 0x560e64b98871 in object_new qom/object.c:749:12
    #3 0x560e64b5d1a1 in qdev_new hw/core/qdev.c:153:19
    #4 0x560e61547ea5 in nand_init hw/block/nand.c:639:11
    #5 0x560e619f8772 in tc6393xb_init hw/display/tc6393xb.c:558:16
    #6 0x560e6390bad2 in tosa_init hw/arm/tosa.c:250:12

SUMMARY: AddressSanitizer: heap-buffer-overflow hw/block/nand.c:101:20 in mem_and
==15750==ABORTING

Broken since introduction in commit 3e3d5815cb ("NAND Flash memory
emulation and ECC calculation helpers for use by NAND controllers").

Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1445
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1446
Reported-by: Qiang Liu <cyruscyliu@gmail.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20240409135944.24997-4-philmd@linaro.org>
---
 hw/block/nand.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/block/nand.c b/hw/block/nand.c
index 5a31d78b6b..e2433c25bd 100644
--- a/hw/block/nand.c
+++ b/hw/block/nand.c
@@ -255,7 +255,9 @@ static unsigned nand_load_block(NANDFlashState *s, unsigned offset)
 {
     unsigned iolen;
 
-    s->blk_load(s, s->addr, offset);
+    if (!s->blk_load(s, s->addr, offset)) {
+        return 0;
+    }
 
     iolen = (1 << s->page_shift);
     if (s->gnd) {
@@ -783,6 +785,10 @@ static bool glue(nand_blk_load_, NAND_PAGE_SIZE)(NANDFlashState *s,
         return false;
     }
 
+    if (offset > NAND_PAGE_SIZE + OOB_SIZE) {
+        return false;
+    }
+
     if (s->blk) {
         if (s->mem_oob) {
             if (blk_pread(s->blk, SECTOR(addr) << BDRV_SECTOR_BITS,
-- 
2.41.0



  parent reply	other threads:[~2024-04-10  9:17 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-10  9:12 [PULL 00/16] Misc HW patches for 2024-04-10 Philippe Mathieu-Daudé
2024-04-10  9:13 ` [PULL 01/16] hw/virtio: Introduce virtio_bh_new_guarded() helper Philippe Mathieu-Daudé
2024-04-10  9:13 ` [PULL 02/16] hw/display/virtio-gpu: Protect from DMA re-entrancy bugs Philippe Mathieu-Daudé
2024-04-10  9:13 ` [PULL 03/16] hw/char/virtio-serial-bus: " Philippe Mathieu-Daudé
2024-04-10  9:13 ` [PULL 04/16] hw/virtio/virtio-crypto: " Philippe Mathieu-Daudé
2024-04-10  9:13 ` [PULL 05/16] qemu-options: Fix CXL Fixed Memory Window interleave-granularity typo Philippe Mathieu-Daudé
2024-04-10  9:13 ` [PULL 06/16] hw/block/nand: Factor nand_load_iolen() method out Philippe Mathieu-Daudé
2024-04-10  9:13 ` [PULL 07/16] hw/block/nand: Have blk_load() take unsigned offset and return boolean Philippe Mathieu-Daudé
2024-04-10  9:13 ` Philippe Mathieu-Daudé [this message]
2024-04-10  9:13 ` [PULL 09/16] hw/misc/applesmc: Do not call DeviceReset from DeviceRealize Philippe Mathieu-Daudé
2024-04-10  9:13 ` [PULL 10/16] hw/misc/applesmc: Fix memory leak in reset() handler Philippe Mathieu-Daudé
2024-04-10  9:13 ` [PULL 11/16] backends/cryptodev: Do not abort for invalid session ID Philippe Mathieu-Daudé
2024-04-10  9:13 ` [PULL 12/16] hw/net/lan9118: Replace magic '2048' value by MIL_TXFIFO_SIZE definition Philippe Mathieu-Daudé
2024-04-10  9:13 ` [PULL 13/16] hw/net/lan9118: Fix overflow in MIL TX FIFO Philippe Mathieu-Daudé
2024-04-10  9:13 ` [PULL 14/16] hw/sd/sdhci: Do not update TRNMOD when Command Inhibit (DAT) is set Philippe Mathieu-Daudé
2024-04-10  9:13 ` [PULL 15/16] hw/net/net_tx_pkt: Fix overrun in update_sctp_checksum() Philippe Mathieu-Daudé
2024-04-10  9:13 ` [PULL 16/16] hw/audio/virtio-snd: Remove unused assignment Philippe Mathieu-Daudé
2024-04-10 15:08 ` [PULL 00/16] Misc HW patches for 2024-04-10 Peter Maydell

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=20240410091315.57241-9-philmd@linaro.org \
    --to=philmd@linaro.org \
    --cc=cyruscyliu@gmail.com \
    --cc=hreitz@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    --cc=richard.henderson@linaro.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.