1. Patchew
  2. QEMU
  3. View series

[PATCH v2] hw/nvme: fix cross-namespace copy dif buffer overflow

Klaus Jensen posted 1 patch 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260728-fix-cross-ns-pract-v2-1-4641b842836f@samsung.com
Maintainers: Keith Busch <kbusch@kernel.org>, Klaus Jensen <its@irrelevant.dk>, Jesper Devantier <foss@defmacro.it>
hw/nvme/ctrl.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
[PATCH v2] hw/nvme: fix cross-namespace copy dif buffer overflow
Posted by Klaus Jensen 2 months ago 
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>
Re: [PATCH v2] hw/nvme: fix cross-namespace copy dif buffer overflow
Posted by Philippe Mathieu-Daudé 2 months ago 
On 28/7/26 12:37, 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
> 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)));

Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>

> +
> +    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,
Re: [PATCH v2] hw/nvme: fix cross-namespace copy dif buffer overflow
Posted by Daniel P. Berrangé 2 months ago 
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 :|

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.