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.mayd...@linaro.org> > --- > In discussion on Philippe's attempt at getting rid of this VLA: > https://patchew.org/QEMU/20210505211047.1496765-1-phi...@redhat.com/20210505211047.1496765-7-phi...@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: https://lore.kernel.org/qemu-devel/20230817155847.3605115-5-stefa...@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 <stefa...@redhat.com> > diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c > index da36fcfd0b5..f31ec79d0b2 100644 > --- a/hw/block/dataplane/virtio-blk.c > +++ b/hw/block/dataplane/virtio-blk.c > @@ -59,11 +59,16 @@ static void notify_guest_bh(void *opaque) > { > VirtIOBlockDataPlane *s = opaque; > unsigned nvqs = s->conf->num_queues; > - unsigned long bitmap[BITS_TO_LONGS(nvqs)]; > + DECLARE_BITMAP(bitmap, VIRTIO_QUEUE_MAX); > unsigned j; > > - memcpy(bitmap, s->batch_notify_vqs, sizeof(bitmap)); > - memset(s->batch_notify_vqs, 0, sizeof(bitmap)); > + /* > + * Note that our local 'bitmap' is declared at a fixed > + * worst case size, but s->batch_notify_vqs has only > + * nvqs bits in it. > + */ > + bitmap_copy(bitmap, s->batch_notify_vqs, nvqs); > + bitmap_zero(s->batch_notify_vqs, nvqs); > > for (j = 0; j < nvqs; j += BITS_PER_LONG) { > unsigned long bits = bitmap[j / BITS_PER_LONG]; > -- > 2.34.1 >
signature.asc
Description: PGP signature