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

On 28/7/26 12:37, 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
> 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)));

Reviewed-by: Philippe Mathieu-Daudé <[email protected]>

> +
> +    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,