SET_VRING_NUM, SET_VRING_ADDR, SET_VRING_BASE, and GET_VRING_BASE
handlers all use the queue index from the message to access dev->vq[]
without checking that it is below dev->max_queues, so a malformed
message causes an out-of-bounds heap access.
Frontend is trusted so not a security problem, but
an OOB access is not a nice way to handle errors.
Check, and panic.
Fixes: 7b2e5c65f4 ("contrib: add libvhost-user")
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3741
Cc: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
subprojects/libvhost-user/libvhost-user.c | 28 ++++++++++++++++++++++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
index 8208265588..42a5e2f1f5 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -1200,6 +1200,12 @@ vu_set_vring_num_exec(VuDev *dev, VhostUserMsg *vmsg)
DPRINT("State.index: %u\n", index);
DPRINT("State.num: %u\n", num);
+
+ if (index >= dev->max_queues) {
+ vu_panic(dev, "Invalid vring_num index: %u", index);
+ return false;
+ }
+
dev->vq[index].vring.num = num;
return false;
@@ -1210,7 +1216,7 @@ vu_set_vring_addr_exec(VuDev *dev, VhostUserMsg *vmsg)
{
struct vhost_vring_addr addr = vmsg->payload.addr, *vra = &addr;
unsigned int index = vra->index;
- VuVirtq *vq = &dev->vq[index];
+ VuVirtq *vq;
DPRINT("vhost_vring_addr:\n");
DPRINT(" index: %d\n", vra->index);
@@ -1220,6 +1226,12 @@ vu_set_vring_addr_exec(VuDev *dev, VhostUserMsg *vmsg)
DPRINT(" avail_user_addr: 0x%016" PRIx64 "\n", (uint64_t)vra->avail_user_addr);
DPRINT(" log_guest_addr: 0x%016" PRIx64 "\n", (uint64_t)vra->log_guest_addr);
+ if (index >= dev->max_queues) {
+ vu_panic(dev, "Invalid vring_addr index: %u", index);
+ return false;
+ }
+
+ vq = &dev->vq[index];
vq->vra = *vra;
vq->vring.flags = vra->flags;
vq->vring.log_guest_addr = vra->log_guest_addr;
@@ -1256,6 +1268,12 @@ vu_set_vring_base_exec(VuDev *dev, VhostUserMsg *vmsg)
DPRINT("State.index: %u\n", index);
DPRINT("State.num: %u\n", num);
+
+ if (index >= dev->max_queues) {
+ vu_panic(dev, "Invalid vring_base index: %u", index);
+ return false;
+ }
+
dev->vq[index].shadow_avail_idx = dev->vq[index].last_avail_idx = num;
return false;
@@ -1267,6 +1285,14 @@ vu_get_vring_base_exec(VuDev *dev, VhostUserMsg *vmsg)
unsigned int index = vmsg->payload.state.index;
DPRINT("State.index: %u\n", index);
+
+ if (index >= dev->max_queues) {
+ vu_panic(dev, "Invalid vring_base index: %u", index);
+ vmsg->payload.state.num = 0;
+ vmsg->size = sizeof(vmsg->payload.state);
+ return true;
+ }
+
vmsg->payload.state.num = dev->vq[index].last_avail_idx;
vmsg->size = sizeof(vmsg->payload.state);
--
MST