On 05.02.24 18:26, Stefan Hajnoczi wrote:
Hanna Czenczek <hre...@redhat.com> noticed that the safety of
`vq_aio_context[vq->value] = ctx;` with user-defined vq->value inputs is
not obvious.

The code is structured in validate() + apply() steps so input validation
is there, but it happens way earlier and there is nothing that
guarantees apply() can only be called with validated inputs.

This patch moves the validate() call inside the apply() function so
validation is guaranteed. I also added the bounds checking assertion
that Hanna suggested.

Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com>
---
  hw/block/virtio-blk.c | 192 +++++++++++++++++++++++-------------------
  1 file changed, 107 insertions(+), 85 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 227d83569f..e8b37fd5f4 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c

[...]

@@ -1660,6 +1681,14 @@ static bool virtio_blk_vq_aio_context_init(VirtIOBlock 
*s, Error **errp)
      BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
      VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+ if (conf->iothread && conf->iothread_vq_mapping_list) {
+        if (conf->iothread) {

This inner condition should probably be dropped.  With that done:

Reviewed-by: Hanna Czenczek <hre...@redhat.com>

+            error_setg(errp, "iothread and iothread-vq-mapping properties "
+                             "cannot be set at the same time");
+            return false;
+        }
+    }
+
      if (conf->iothread || conf->iothread_vq_mapping_list) {
          if (!k->set_guest_notifiers || !k->ioeventfd_assign) {
              error_setg(errp,


Reply via email to