[v2] libvhost-user: protect against OOB memory table access

[PATCH v2] libvhost-user: protect against OOB memory table access

Posted by Michael S. Tsirkin 1 week, 4 days ago

SET_MEM_TABLE handler uses nregions from the message as a loop
bound without checking that it is below VHOST_MEMORY_BASELINE_NREGIONS,
so a malformed message causes an out-of-bounds access to the
regions[] and fds[] arrays.

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/4403
Cc: Stefano Garzarella <sgarzare@redhat.com>
Reported-by: Omer Can Vural <can.omer.5306@outlook.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 subprojects/libvhost-user/libvhost-user.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
index 2fed792e85..52817cc733 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -1105,6 +1105,13 @@ vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg)
     vu_remove_all_mem_regs(dev);
 
     DPRINT("Nregions: %u\n", memory->nregions);
+
+    if (memory->nregions > VHOST_MEMORY_BASELINE_NREGIONS) {
+        vu_panic(dev, "Invalid set_mem_table nregions: %u > %u",
+                 memory->nregions, VHOST_MEMORY_BASELINE_NREGIONS);
+        return false;
+    }
+
     for (i = 0; i < memory->nregions; i++) {
         _vu_add_mem_reg(dev, &memory->regions[i], vmsg->fds[i]);
         close(vmsg->fds[i]);
-- 
MST

Re: [PATCH v2] libvhost-user: protect against OOB memory table access

Posted by Stefano Garzarella 1 week, 2 days ago

On Tue, Sep 15, 2026 at 06:46:17AM -0400, Michael S. Tsirkin wrote:
>SET_MEM_TABLE handler uses nregions from the message as a loop
>bound without checking that it is below VHOST_MEMORY_BASELINE_NREGIONS,
>so a malformed message causes an out-of-bounds access to the
>regions[] and fds[] arrays.
>
>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/4403
>Cc: Stefano Garzarella <sgarzare@redhat.com>
>Reported-by: Omer Can Vural <can.omer.5306@outlook.com>
>Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>---
> subprojects/libvhost-user/libvhost-user.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
>diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c
>index 2fed792e85..52817cc733 100644
>--- a/subprojects/libvhost-user/libvhost-user.c
>+++ b/subprojects/libvhost-user/libvhost-user.c
>@@ -1105,6 +1105,13 @@ vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg)
>     vu_remove_all_mem_regs(dev);
>
>     DPRINT("Nregions: %u\n", memory->nregions);
>+
>+    if (memory->nregions > VHOST_MEMORY_BASELINE_NREGIONS) {

Should we also check that it's equal to the number of file descriptors?

Maybe `memory->nregions != (uint32_t)vmsg->fd_num` is
enough for both checks, since IIUC we already assert that
vmsg->fd_numvmsg->fd_num <= VHOST_MEMORY_BASELINE_NREGIONS

Thanks,
Stefano

>+        vu_panic(dev, "Invalid set_mem_table nregions: %u > %u",
>+                 memory->nregions, VHOST_MEMORY_BASELINE_NREGIONS);
>+        return false;
>+    }
>+
>     for (i = 0; i < memory->nregions; i++) {
>         _vu_add_mem_reg(dev, &memory->regions[i], vmsg->fds[i]);
>         close(vmsg->fds[i]);
>-- 
>MST
>