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

Hi Klaus,

On 28/7/26 11:18, Klaus Jensen wrote:
> From: Klaus Jensen <[email protected]>
> 
> The NVMe specification allows a controller with multiple namespaces to
> use different LBA formats per namespace. On implication of this is that

Typo "One".

> 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 errornously allocated

Typo "erroneously".

> 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/4031
> Reported-by: Jihe Wang <[email protected]>
> Reported-by: boy juju <[email protected]>
> Reported-by: contact <[email protected]>
> Reported-by: david korczynski <[email protected]>
> Signed-off-by: Klaus Jensen <[email protected]>
> ---
>   hw/nvme/ctrl.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
> index a67e1598891c..409aeb9921c2 100644
> --- a/hw/nvme/ctrl.c
> +++ b/hw/nvme/ctrl.c
> @@ -3332,7 +3332,8 @@ 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);
> +                              sns->lbasz + (sns->lbaf.ms == 0 ? dns->lbaf.ms :
> +                                            sns->lbaf.ms));

   sns->lbasz + (sns->lbaf.ms ? : dns->lbaf.ms)

Could MAX() be simpler?

>   
>       qemu_iovec_reset(&iocb->iov);
>       qemu_iovec_add(&iocb->iov, iocb->bounce, len);

Should we assert @len <= what we just allocated?

> 
> ---
> base-commit: 299e7557ed15a9a325620698add379a3ce2d1d95
> change-id: 20260728-fix-cross-ns-pract-6934cc8205f2
> 
> Best regards,