On Jul 28 11:32, Philippe Mathieu-Daudé wrote: > 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". >
Thanks! > > 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? > That was my first fix. But the case we are covering is actually only when the source namespace metadata size is zero. Otherwise, the formats must match (which is checked earlier). But since you are pointing it out, I think MAX is probably more clear. We won't accidentially over-allocate because the formats are checked earlier. > > qemu_iovec_reset(&iocb->iov); > > qemu_iovec_add(&iocb->iov, iocb->bounce, len); > > Should we assert @len <= what we just allocated? > Sure, would be reasonable! > > > > --- > > base-commit: 299e7557ed15a9a325620698add379a3ce2d1d95 > > change-id: 20260728-fix-cross-ns-pract-6934cc8205f2 > > > > Best regards, > >
signature.asc
Description: PGP signature
