[PATCH] hw/nvme: Fix memory leak in nvme_dsm
The allocated memory to hold LBA ranges leaks in the nvme_dsm function. This happens because the allocated memory for iocb->range is not freed in all error handling paths. Fix this by adding a free to ensure that the allocated memory is properly freed. ASAN log: ==3075137==ERROR: LeakSanitizer: detected memory leaks Direct leak of 480 byte(s) in 6 object(s) allocated from: #0 0x55f1f8a0eddd in malloc llvm/compiler-rt/lib/asan/asan_malloc_linux.cpp:129:3 #1 0x7f531e0f6738 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x5e738) #2 0x55f1faf1f091 in blk_aio_get block/block-backend.c:2583:12 #3 0x55f1f945c74b in nvme_dsm hw/nvme/ctrl.c:2609:30 #4 0x55f1f945831b in nvme_io_cmd hw/nvme/ctrl.c:4470:16 #5 0x55f1f94561b7 in nvme_process_sq hw/nvme/ctrl.c:7039:29 Signed-off-by: Zheyu Ma --- hw/nvme/ctrl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index 127c3d2383..cf610eab21 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -2592,6 +2592,7 @@ next: done: iocb->aiocb = NULL; iocb->common.cb(iocb->common.opaque, iocb->ret); +g_free(iocb->range); qemu_aio_unref(iocb); } -- 2.34.1
Re: [PATCH] block: m25p80: Fix heap-buffer-overflow in flash_erase function
Hi Philippe,
On Tue, Jun 18, 2024 at 10:34 PM Philippe Mathieu-Daudé
wrote:
> On 18/6/24 21:11, Zheyu Ma wrote:
> > Thanks for your useful advice!
> >
> > So how about report the issue and return:
>
> We might report the issue to the user, but there should
> be a way the hardware report the issue to the guest software
> running. Usually signaled as error condition, irq, ...
> We need to figure out what check wasn't properly done.
> The spec is the source of truth.
>
Sure.
Although I'm unfamiliar with the device, I'll try to figure it out.
Zheyu
>
> > diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> > index 8dec134832..2121b43708 100644
> > --- a/hw/block/m25p80.c
> > +++ b/hw/block/m25p80.c
> > @@ -617,6 +617,12 @@ static void flash_erase(Flash *s, int offset,
> > FlashCMD cmd)
> > abort();
> > }
> >
> > +if (offset + len > s->size) {
> > +qemu_log_mask(LOG_GUEST_ERROR,
> > + "M25P80: Erase operation exceeds storage size\n");
> > +return;
> > +}
> > +
> > trace_m25p80_flash_erase(s, offset, len);
> >
> > if ((s->pi->flags & capa_to_assert) != capa_to_assert) {
> >
> > regards,
> > Zheyu
> >
> > On Tue, Jun 18, 2024 at 5:35 PM Philippe Mathieu-Daudé
> > mailto:phi...@linaro.org>> wrote:
> >
> > Hi Zheyu,
> >
> > On 18/6/24 17:23, Zheyu Ma wrote:
> > > This patch fixes a heap-buffer-overflow issue in the flash_erase
> > function
> > > of the m25p80 flash memory emulation. The overflow occurs when the
> > > combination of offset and length exceeds the allocated memory for
> the
> > > storage. The patch adds a check to ensure that the erase length
> > does not
> > > exceed the storage size and adjusts the length accordingly if
> > necessary.
> > >
> > > Reproducer:
> > > cat << EOF | qemu-system-aarch64 -display none \
> > > -machine accel=qtest, -m 512M -machine kudo-bmc -qtest stdio
> > > writeq 0xc010 0x6
> > > writel 0xc00c 0x9
> > > writeq 0xc010 0xf27f9412
> > > writeq 0xc00f 0x2b5cdc26
> > > writeq 0xc00c 0x
> > > writeq 0xc00c 0x
> > > writeq 0xc00c 0x
> > > writel 0xc00c 0x9
> > > writeq 0xc00c 0x9
> > > EOF
> > >
> > > ASan log:
> > > ==2614308==ERROR: AddressSanitizer: heap-buffer-overflow on
> > address 0x7fd3fb7fc000 at pc 0x55aa77a442dc bp 0x7fffaa155900 sp
> > 0x7fffaa1550c8
> > > WRITE of size 65536 at 0x7fd3fb7fc000 thread T0
> > > #0 0x55aa77a442db in __asan_memset
> > llvm/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:26:3
> > > #1 0x55aa77e7e6b3 in flash_erase hw/block/m25p80.c:631:5
> > > #2 0x55aa77e6f8b1 in complete_collecting_data
> > hw/block/m25p80.c:773:9
> > > #3 0x55aa77e6aaa9 in m25p80_transfer8
> hw/block/m25p80.c:1550:13
> > > #4 0x55aa78e9a691 in ssi_transfer_raw_default
> hw/ssi/ssi.c:92:16
> > > #5 0x55aa78e996c0 in ssi_transfer hw/ssi/ssi.c:165:14
> > > #6 0x55aa78e8d76a in npcm7xx_fiu_uma_transaction
> > hw/ssi/npcm7xx_fiu.c:336:9
> > > #7 0x55aa78e8be4b in npcm7xx_fiu_ctrl_write
> > hw/ssi/npcm7xx_fiu.c:428:13
> > >
> > > Signed-off-by: Zheyu Ma > <mailto:zheyum...@gmail.com>>
> > > ---
> > > hw/block/m25p80.c | 6 ++
> > > 1 file changed, 6 insertions(+)
> > >
> > > diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> > > index 8dec134832..e9a59f6616 100644
> > > --- a/hw/block/m25p80.c
> > > +++ b/hw/block/m25p80.c
> > > @@ -617,6 +617,12 @@ static void flash_erase(Flash *s, int
> > offset, FlashCMD cmd)
> > > abort();
> > > }
> > >
> > > +if (offset + len > s->size) {
> > > +qemu_log_mask(LOG_GUEST_ERROR,
> > > + "M25P80: Erase exceeds storage size,
> > adjusting length\n");
> >
> > Usually hardware doesn't "adjust" but report error earlier.
> >
> > > +len = s->size - offset;
> > > +}
> > > +
> > > trace_m25p80_flash_erase(s, offset, len);
> > >
> > > if ((s->pi->flags & capa_to_assert) != capa_to_assert) {
> >
>
>
Re: [PATCH] block: m25p80: Fix heap-buffer-overflow in flash_erase function
Thanks for your useful advice!
So how about report the issue and return:
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 8dec134832..2121b43708 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -617,6 +617,12 @@ static void flash_erase(Flash *s, int offset, FlashCMD
cmd)
abort();
}
+if (offset + len > s->size) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "M25P80: Erase operation exceeds storage size\n");
+return;
+}
+
trace_m25p80_flash_erase(s, offset, len);
if ((s->pi->flags & capa_to_assert) != capa_to_assert) {
regards,
Zheyu
On Tue, Jun 18, 2024 at 5:35 PM Philippe Mathieu-Daudé
wrote:
> Hi Zheyu,
>
> On 18/6/24 17:23, Zheyu Ma wrote:
> > This patch fixes a heap-buffer-overflow issue in the flash_erase function
> > of the m25p80 flash memory emulation. The overflow occurs when the
> > combination of offset and length exceeds the allocated memory for the
> > storage. The patch adds a check to ensure that the erase length does not
> > exceed the storage size and adjusts the length accordingly if necessary.
> >
> > Reproducer:
> > cat << EOF | qemu-system-aarch64 -display none \
> > -machine accel=qtest, -m 512M -machine kudo-bmc -qtest stdio
> > writeq 0xc010 0x6
> > writel 0xc00c 0x9
> > writeq 0xc010 0xf27f9412
> > writeq 0xc00f 0x2b5cdc26
> > writeq 0xc00c 0x
> > writeq 0xc00c 0x
> > writeq 0xc00c 0x
> > writel 0xc00c 0x9
> > writeq 0xc00c 0x9
> > EOF
> >
> > ASan log:
> > ==2614308==ERROR: AddressSanitizer: heap-buffer-overflow on address
> 0x7fd3fb7fc000 at pc 0x55aa77a442dc bp 0x7fffaa155900 sp 0x7fffaa1550c8
> > WRITE of size 65536 at 0x7fd3fb7fc000 thread T0
> > #0 0x55aa77a442db in __asan_memset
> llvm/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:26:3
> > #1 0x55aa77e7e6b3 in flash_erase hw/block/m25p80.c:631:5
> > #2 0x55aa77e6f8b1 in complete_collecting_data
> hw/block/m25p80.c:773:9
> > #3 0x55aa77e6aaa9 in m25p80_transfer8 hw/block/m25p80.c:1550:13
> > #4 0x55aa78e9a691 in ssi_transfer_raw_default hw/ssi/ssi.c:92:16
> > #5 0x55aa78e996c0 in ssi_transfer hw/ssi/ssi.c:165:14
> > #6 0x55aa78e8d76a in npcm7xx_fiu_uma_transaction
> hw/ssi/npcm7xx_fiu.c:336:9
> > #7 0x55aa78e8be4b in npcm7xx_fiu_ctrl_write
> hw/ssi/npcm7xx_fiu.c:428:13
> >
> > Signed-off-by: Zheyu Ma
> > ---
> > hw/block/m25p80.c | 6 ++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
> > index 8dec134832..e9a59f6616 100644
> > --- a/hw/block/m25p80.c
> > +++ b/hw/block/m25p80.c
> > @@ -617,6 +617,12 @@ static void flash_erase(Flash *s, int offset,
> FlashCMD cmd)
> > abort();
> > }
> >
> > +if (offset + len > s->size) {
> > +qemu_log_mask(LOG_GUEST_ERROR,
> > + "M25P80: Erase exceeds storage size, adjusting
> length\n");
>
> Usually hardware doesn't "adjust" but report error earlier.
>
> > +len = s->size - offset;
> > +}
> > +
> > trace_m25p80_flash_erase(s, offset, len);
> >
> > if ((s->pi->flags & capa_to_assert) != capa_to_assert) {
>
>
[PATCH] block: m25p80: Fix heap-buffer-overflow in flash_erase function
This patch fixes a heap-buffer-overflow issue in the flash_erase function
of the m25p80 flash memory emulation. The overflow occurs when the
combination of offset and length exceeds the allocated memory for the
storage. The patch adds a check to ensure that the erase length does not
exceed the storage size and adjusts the length accordingly if necessary.
Reproducer:
cat << EOF | qemu-system-aarch64 -display none \
-machine accel=qtest, -m 512M -machine kudo-bmc -qtest stdio
writeq 0xc010 0x6
writel 0xc00c 0x9
writeq 0xc010 0xf27f9412
writeq 0xc00f 0x2b5cdc26
writeq 0xc00c 0x
writeq 0xc00c 0x
writeq 0xc00c 0x
writel 0xc00c 0x9
writeq 0xc00c 0x9
EOF
ASan log:
==2614308==ERROR: AddressSanitizer: heap-buffer-overflow on address
0x7fd3fb7fc000 at pc 0x55aa77a442dc bp 0x7fffaa155900 sp 0x7fffaa1550c8
WRITE of size 65536 at 0x7fd3fb7fc000 thread T0
#0 0x55aa77a442db in __asan_memset
llvm/compiler-rt/lib/asan/asan_interceptors_memintrinsics.cpp:26:3
#1 0x55aa77e7e6b3 in flash_erase hw/block/m25p80.c:631:5
#2 0x55aa77e6f8b1 in complete_collecting_data hw/block/m25p80.c:773:9
#3 0x55aa77e6aaa9 in m25p80_transfer8 hw/block/m25p80.c:1550:13
#4 0x55aa78e9a691 in ssi_transfer_raw_default hw/ssi/ssi.c:92:16
#5 0x55aa78e996c0 in ssi_transfer hw/ssi/ssi.c:165:14
#6 0x55aa78e8d76a in npcm7xx_fiu_uma_transaction hw/ssi/npcm7xx_fiu.c:336:9
#7 0x55aa78e8be4b in npcm7xx_fiu_ctrl_write hw/ssi/npcm7xx_fiu.c:428:13
Signed-off-by: Zheyu Ma
---
hw/block/m25p80.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 8dec134832..e9a59f6616 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -617,6 +617,12 @@ static void flash_erase(Flash *s, int offset, FlashCMD cmd)
abort();
}
+if (offset + len > s->size) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "M25P80: Erase exceeds storage size, adjusting
length\n");
+len = s->size - offset;
+}
+
trace_m25p80_flash_erase(s, offset, len);
if ((s->pi->flags & capa_to_assert) != capa_to_assert) {
--
2.34.1
[PATCH] block/virtio-blk: Fix memory leak from virtio_blk_zone_report
This modification ensures that in scenarios where the buffer size is
insufficient for a zone report, the function will now properly set an
error status and proceed to a cleanup label, instead of merely
returning.
The following ASAN log reveals it:
==1767400==ERROR: LeakSanitizer: detected memory leaks
Direct leak of 312 byte(s) in 1 object(s) allocated from:
#0 0x64ac7b3280cd in malloc
llvm/compiler-rt/lib/asan/asan_malloc_linux.cpp:129:3
#1 0x735b02fb9738 in g_malloc
(/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x5e738)
#2 0x64ac7d23be96 in virtqueue_split_pop hw/virtio/virtio.c:1612:12
#3 0x64ac7d23728a in virtqueue_pop hw/virtio/virtio.c:1783:16
#4 0x64ac7cfcaacd in virtio_blk_get_request hw/block/virtio-blk.c:228:27
#5 0x64ac7cfca7c7 in virtio_blk_handle_vq hw/block/virtio-blk.c:1123:23
#6 0x64ac7cfecb95 in virtio_blk_handle_output hw/block/virtio-blk.c:1157:5
Signed-off-by: Zheyu Ma
---
hw/block/virtio-blk.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 92de315f17..bb86e65f65 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -768,7 +768,8 @@ static void virtio_blk_handle_zone_report(VirtIOBlockReq
*req,
sizeof(struct virtio_blk_zone_report) +
sizeof(struct virtio_blk_zone_descriptor)) {
virtio_error(vdev, "in buffer too small for zone report");
-return;
+err_status = VIRTIO_BLK_S_ZONE_INVALID_CMD;
+goto out;
}
/* start byte offset of the zone report */
--
2.34.1
