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

[Peter Maydell <peter.maydell@linaro.org>]

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(-)

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