qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] hw/block/dataplane/virtio-block: Avoid dynamic stack allocat


From: Peter Maydell
Subject: Re: [PATCH] hw/block/dataplane/virtio-block: Avoid dynamic stack allocation
Date: Tue, 29 Aug 2023 10:52:29 +0100



On Thu, 24 Aug 2023 at 18:15, Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> On Thu, Aug 24, 2023 at 05:57:40PM +0100, Peter Maydell wrote:
> > Instead of using a variable length array in notify_guest_bh(), always
> > use a fixed sized bitmap (this will be 128 bytes).  This means we
> > need to avoid assuming that bitmap and the s->batch_notify_vqs bitmap
> > are the same size; the neatest way to do this is to switch to using
> > bitmap.h APIs to declare, copy and clear, because then we can specify
> > the length in bits, exactly as we did when creating
> > s->batch_notify_vqs with bitmap_new().
> >
> > The codebase has very few VLAs, and if we can get rid of them all we
> > can make the compiler error on new additions.  This is a defensive
> > measure against security bugs where an on-stack dynamic allocation
> > isn't correctly size-checked (e.g.  CVE-2021-3527).
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> > In discussion on Philippe's attempt at getting rid of this VLA:
> > https://patchew.org/QEMU/20210505211047.1496765-1-philmd@redhat.com/20210505211047.1496765-7-philmd@redhat.com/
> > Stefan suggested getting rid of the local bitmap array entirely.
> > But I don't know this code at all and have no idea of the
> > implications (presumably there is a reason we have the local
> > array rather than iterating directly on batch_notify_vqs),
> > so I have opted for the more minimal change.
> >
> > Usual disclaimer: tested only with "make check" and
> > "make check-avocado".
> > ---
> >  hw/block/dataplane/virtio-blk.c | 11 ++++++++---
> >  1 file changed, 8 insertions(+), 3 deletions(-)
>
> Hi Peter,
> I recently sent a patch series that removes notify_guest_bh() completely:
> 20230817155847.3605115-5-stefanha@redhat.com/">https://lore.kernel.org/qemu-devel/20230817155847.3605115-5-stefanha@redhat.com/
>
> If it's urgent we can merge your patch immediately, though I hope my
> series will be merged soon anyway:
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Great. There's no hurry here, so your change looks like the
best approach.

-- PMM


reply via email to

[Prev in Thread] Current Thread [Next in Thread]