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

Daniel P. Berrangé <[email protected]>
Newsgroups gmane.comp.emulators.qemu,gmane.comp.emulators.qemu.block,gmane.comp.emulators.qemu.stable
Message-ID <[email protected]>
On Tue, Jul 28, 2026 at 12:37:57PM +0200, Klaus Jensen wrote:
> From: Klaus Jensen <[email protected]>
> 
> 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: [email protected]
> 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 <[email protected]>
> Reported-by: boy juju <[email protected]>
> Reported-by: contact <[email protected]>
> Reported-by: david korczynski <[email protected]>
> Reported-by: Brian Chastain (off_by_one / Curious-Keeper) <[email protected]>
> Signed-off-by: Klaus Jensen <[email protected]>
> ---
> Changes in v2:
> - use MAX instead of the tertiary if
> - assert that we don't 
> - Link to v1: https://lore.kernel.org/qemu-devel/[email protected]
> ---
>  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 <[email protected]>
> 
> 

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 :|
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.