* [PATCH v2] hw/nvme: fix cross-namespace copy dif buffer overflow
@ 2026-07-28 10:37 Klaus Jensen
2026-07-28 10:42 ` Daniel P. Berrangé
0 siblings, 1 reply; 2+ messages in thread
From: Klaus Jensen @ 2026-07-28 10:37 UTC (permalink / raw)
To: qemu-devel, Keith Busch, Klaus Jensen, Jesper Devantier,
qemu-block
Cc: Klaus Jensen, qemu-stable, Jihe Wang, boy juju, contact,
david korczynski, Brian Chastain (off_by_one / Curious-Keeper)
From: Klaus Jensen <k.jensen@samsung.com>
The NVMe specification allows a controller with multiple namespaces to
use different LBA formats per namespace. One implication of this is that
the destination namespace may have a metadata area for PI, but the
source does not. In that case, the controller shall generate the
protection information, but the bounce buffer is erroneously allocated
without space for that, causing a buffer overflow.
Fix the allocation.
Cc: qemu-stable@nongnu.org
Fixes: d522aef88d42 ("hw/nvme: add cross namespace copy support")
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3387
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3692
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3841
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3853
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3936
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4031
Reported-by: Jihe Wang <wangjihe.mail@gmail.com>
Reported-by: boy juju <agx1657748706@gmail.com>
Reported-by: contact <contact@xchglabs.com>
Reported-by: david korczynski <david@adalogics.com>
Reported-by: Brian Chastain (off_by_one / Curious-Keeper) <brian@scalingsuccess.io>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
Changes in v2:
- use MAX instead of the tertiary if
- assert that we don't
- Link to v1: https://lore.kernel.org/qemu-devel/20260728-fix-cross-ns-pract-v1-1-20f66766337f@samsung.com
---
hw/nvme/ctrl.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index a67e1598891c..7f28d2e3eb49 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -3210,7 +3210,7 @@ static void nvme_do_copy(NvmeCopyAIOCB *iocb)
uint16_t prinfow = ((copy->control[2] >> 2) & 0xf);
uint64_t slba;
uint32_t nlb;
- size_t len;
+ size_t len, blen;
uint16_t status;
uint32_t dnsid = le32_to_cpu(req->cmd.nsid);
uint32_t snsid = dnsid;
@@ -3331,10 +3331,13 @@ static void nvme_do_copy(NvmeCopyAIOCB *iocb)
}
g_free(iocb->bounce);
- iocb->bounce = g_malloc_n(le16_to_cpu(sns->id_ns.mssrl),
- sns->lbasz + sns->lbaf.ms);
+ assert(g_size_checked_mul(&blen, le16_to_cpu(sns->id_ns.mssrl),
+ sns->lbasz + MAX(sns->lbaf.ms, dns->lbaf.ms)));
+
+ iocb->bounce = g_malloc(blen);
qemu_iovec_reset(&iocb->iov);
+ assert(len <= blen);
qemu_iovec_add(&iocb->iov, iocb->bounce, len);
block_acct_start(blk_get_stats(sns->blkconf.blk), &iocb->acct.read, 0,
---
base-commit: 299e7557ed15a9a325620698add379a3ce2d1d95
change-id: 20260728-fix-cross-ns-pract-6934cc8205f2
Best regards,
--
Klaus Jensen <k.jensen@samsung.com>
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH v2] hw/nvme: fix cross-namespace copy dif buffer overflow
2026-07-28 10:37 [PATCH v2] hw/nvme: fix cross-namespace copy dif buffer overflow Klaus Jensen
@ 2026-07-28 10:42 ` Daniel P. Berrangé
0 siblings, 0 replies; 2+ messages in thread
From: Daniel P. Berrangé @ 2026-07-28 10:42 UTC (permalink / raw)
To: Klaus Jensen
Cc: qemu-devel, Keith Busch, Jesper Devantier, qemu-block,
Klaus Jensen, qemu-stable, Jihe Wang, boy juju, contact,
david korczynski, Brian Chastain (off_by_one / Curious-Keeper)
On Tue, Jul 28, 2026 at 12:37:57PM +0200, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
>
> The NVMe specification allows a controller with multiple namespaces to
> use different LBA formats per namespace. One implication of this is that
> the destination namespace may have a metadata area for PI, but the
> source does not. In that case, the controller shall generate the
> protection information, but the bounce buffer is erroneously allocated
> without space for that, causing a buffer overflow.
>
> Fix the allocation.
>
> Cc: qemu-stable@nongnu.org
> Fixes: d522aef88d42 ("hw/nvme: add cross namespace copy support")
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3387
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3692
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3841
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3853
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3936
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4031
FWIW, if you see people filing the same bug over & over you can
mark the newer ones as duplicates to stop them cluttering the
issue dashboard
The comment box type
/duplicate #23423
and submit the comment, with no other text, and it'll close this
bug and link to #23423.
Patches then only need mention the one bug that remains open.
> Reported-by: Jihe Wang <wangjihe.mail@gmail.com>
> Reported-by: boy juju <agx1657748706@gmail.com>
> Reported-by: contact <contact@xchglabs.com>
> Reported-by: david korczynski <david@adalogics.com>
> Reported-by: Brian Chastain (off_by_one / Curious-Keeper) <brian@scalingsuccess.io>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---
> Changes in v2:
> - use MAX instead of the tertiary if
> - assert that we don't
> - Link to v1: https://lore.kernel.org/qemu-devel/20260728-fix-cross-ns-pract-v1-1-20f66766337f@samsung.com
> ---
> hw/nvme/ctrl.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> index a67e1598891c..7f28d2e3eb49 100644
> --- a/hw/nvme/ctrl.c
> +++ b/hw/nvme/ctrl.c
> @@ -3210,7 +3210,7 @@ static void nvme_do_copy(NvmeCopyAIOCB *iocb)
> uint16_t prinfow = ((copy->control[2] >> 2) & 0xf);
> uint64_t slba;
> uint32_t nlb;
> - size_t len;
> + size_t len, blen;
> uint16_t status;
> uint32_t dnsid = le32_to_cpu(req->cmd.nsid);
> uint32_t snsid = dnsid;
> @@ -3331,10 +3331,13 @@ static void nvme_do_copy(NvmeCopyAIOCB *iocb)
> }
>
> g_free(iocb->bounce);
> - iocb->bounce = g_malloc_n(le16_to_cpu(sns->id_ns.mssrl),
> - sns->lbasz + sns->lbaf.ms);
> + assert(g_size_checked_mul(&blen, le16_to_cpu(sns->id_ns.mssrl),
> + sns->lbasz + MAX(sns->lbaf.ms, dns->lbaf.ms)));
> +
> + iocb->bounce = g_malloc(blen);
>
> qemu_iovec_reset(&iocb->iov);
> + assert(len <= blen);
> qemu_iovec_add(&iocb->iov, iocb->bounce, len);
>
> block_acct_start(blk_get_stats(sns->blkconf.blk), &iocb->acct.read, 0,
>
> ---
> base-commit: 299e7557ed15a9a325620698add379a3ce2d1d95
> change-id: 20260728-fix-cross-ns-pract-6934cc8205f2
>
> Best regards,
> --
> Klaus Jensen <k.jensen@samsung.com>
>
>
With regards,
Daniel
--
|: https://berrange.com ~~ https://hachyderm.io/@berrange :|
|: https://libvirt.org ~~ https://entangle-photo.org :|
|: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-07-28 10:43 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-28 10:37 [PATCH v2] hw/nvme: fix cross-namespace copy dif buffer overflow Klaus Jensen
2026-07-28 10:42 ` Daniel P. Berrangé
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.