vu_set_inflight_fd() trusts the num_queues value from the
VHOST_USER_SET_INFLIGHT_FD message without checking it against
dev->max_queues, so an oversized value causes out-of-bounds writes to
dev->vq.
Front end is generally trusted so not a security problem, but OOB isn't
a nice way to handle frontend bugs. Let's harden this a bit:
check num_queues and panic if it's invalid.
Fixes: 5f9ff1eff3 ("libvhost-user: Support tracking inflight I/O in shared memory")
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3740
Cc: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
subprojects/libvhost-user/libvhost-user.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
index 2c35bddd6f..8208265588 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -1998,6 +1998,8 @@ vu_get_inflight_fd(VuDev *dev, VhostUserMsg *vmsg)
if (vmsg->size != sizeof(vmsg->payload.inflight)) {
vu_panic(dev, "Invalid get_inflight_fd message:%d", vmsg->size);
+ vmsg_close_fds(vmsg);
+ vmsg->fd_num = 0;
vmsg->payload.inflight.mmap_size = 0;
return true;
}
@@ -2005,6 +2007,15 @@ vu_get_inflight_fd(VuDev *dev, VhostUserMsg *vmsg)
num_queues = vmsg->payload.inflight.num_queues;
queue_size = vmsg->payload.inflight.queue_size;
+ if (num_queues > dev->max_queues) {
+ vu_panic(dev, "Invalid get_inflight_fd num_queues: %"PRId16,
+ num_queues);
+ vmsg_close_fds(vmsg);
+ vmsg->fd_num = 0;
+ vmsg->payload.inflight.mmap_size = 0;
+ return true;
+ }
+
DPRINT("set_inflight_fd num_queues: %"PRId16"\n", num_queues);
DPRINT("set_inflight_fd queue_size: %"PRId16"\n", queue_size);
@@ -2052,6 +2063,7 @@ vu_set_inflight_fd(VuDev *dev, VhostUserMsg *vmsg)
vmsg->size != sizeof(vmsg->payload.inflight)) {
vu_panic(dev, "Invalid set_inflight_fd message size:%d fds:%d",
vmsg->size, vmsg->fd_num);
+ vmsg_close_fds(vmsg);
return false;
}
@@ -2061,6 +2073,13 @@ vu_set_inflight_fd(VuDev *dev, VhostUserMsg *vmsg)
num_queues = vmsg->payload.inflight.num_queues;
queue_size = vmsg->payload.inflight.queue_size;
+ if (num_queues > dev->max_queues) {
+ vu_panic(dev, "Invalid set_inflight_fd num_queues: %"PRId16,
+ num_queues);
+ close(fd);
+ return false;
+ }
+
DPRINT("set_inflight_fd mmap_size: %"PRId64"\n", mmap_size);
DPRINT("set_inflight_fd mmap_offset: %"PRId64"\n", mmap_offset);
DPRINT("set_inflight_fd num_queues: %"PRId16"\n", num_queues);
@@ -2071,6 +2090,7 @@ vu_set_inflight_fd(VuDev *dev, VhostUserMsg *vmsg)
if (rc == MAP_FAILED) {
vu_panic(dev, "set_inflight_fd mmap error: %s", strerror(errno));
+ close(fd);
return false;
}
--
MST