hw/vfio-user/device.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-)
This patch series fixes somes bugs where the vfio-user client doesn't properly validate information returned by the vfio-user server. Thanos Makatos (5): vfio-user: vfio_user_get_region_info: prevent buffer overflow vfio-user: vfio_user_get_region_info: respect max_xfer_size vfio-user: vfio_user_get_region_info: reject unreasonably short struct vfio-user: vfio_user_get_region_info: prevent excessive malloc vfio-user: vfio_user_device_io_get_region_info: fix capability check hw/vfio-user/device.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) -- 2.34.1
This patch series fixes some bugs where the vfio-user client doesn't properly validate information returned by the vfio-user server. Thanos Makatos (6): vfio-user: vfio_user_get_region_info: prevent buffer overflow vfio-user: vfio_user_get_region_info: respect max_xfer_size vfio-user: vfio_user_get_region_info: reject unreasonably short struct vfio-user: vfio_user_get_region_info: prevent excessive malloc vfio-user: vfio_user_device_io_get_region_info: fix capability check vfio-user: vfio_user_device_io_device_feature: prevent buffer overflow hw/vfio-user/device.c | 40 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) Changes since v1: - added 'Resolves:' in patches 1 and 5 - added missing SOB from patch 1 - removed reassignment in patch 1 - added a new patch to address the same issue as in patch 1 -- 2.34.1
Hello Thanos, On 7/20/26 15:58, Thanos Makatos wrote: > This patch series fixes some bugs where the vfio-user client doesn't > properly validate information returned by the vfio-user server. > > Thanos Makatos (6): > vfio-user: vfio_user_get_region_info: prevent buffer overflow > vfio-user: vfio_user_get_region_info: respect max_xfer_size > vfio-user: vfio_user_get_region_info: reject unreasonably short struct > vfio-user: vfio_user_get_region_info: prevent excessive malloc > vfio-user: vfio_user_device_io_get_region_info: fix capability check > vfio-user: vfio_user_device_io_device_feature: prevent buffer overflow > > hw/vfio-user/device.c | 40 +++++++++++++++++++++++++++++++++++++--- > 1 file changed, 37 insertions(+), 3 deletions(-) > > Changes since v1: > - added 'Resolves:' in patches 1 and 5 > - added missing SOB from patch 1 > - removed reassignment in patch 1 > - added a new patch to address the same issue as in patch 1 > Could you please add 'Fixes:' tags ? No need to resend, just reply to the email you sent. Thanks, C.
On 7/20/26 17:55, Cédric Le Goater wrote:
> Hello Thanos,
>
> On 7/20/26 15:58, Thanos Makatos wrote:
>> This patch series fixes some bugs where the vfio-user client doesn't
>> properly validate information returned by the vfio-user server.
>>
>> Thanos Makatos (6):
>> vfio-user: vfio_user_get_region_info: prevent buffer overflow
>> vfio-user: vfio_user_get_region_info: respect max_xfer_size
>> vfio-user: vfio_user_get_region_info: reject unreasonably short struct
>> vfio-user: vfio_user_get_region_info: prevent excessive malloc
>> vfio-user: vfio_user_device_io_get_region_info: fix capability check
>> vfio-user: vfio_user_device_io_device_feature: prevent buffer overflow
>>
>> hw/vfio-user/device.c | 40 +++++++++++++++++++++++++++++++++++++---
>> 1 file changed, 37 insertions(+), 3 deletions(-)
>>
>> Changes since v1:
>> - added 'Resolves:' in patches 1 and 5
>> - added missing SOB from patch 1
>> - removed reassignment in patch 1
>> - added a new patch to address the same issue as in patch 1
>>
>
> Could you please add 'Fixes:' tags ? No need to resend, just
> reply to the email you sent.
Patches 1-5 address issues introduced by:
667866d66620 ("vfio-user: implement VFIO_USER_DEVICE_GET_REGION_INFO")
Patch 6 addresses an issue introduced by:
e2358af5838d ("vfio-user: support VFIO_USER_DEVICE_FEATURE")
Also, in patch 6, the vfio_user_device_io_device_feature() code does
memcpy(feature, &msgp->argsz, feature->argsz)
without the hardening as done in region_info. That would be a good
addition.
vfio_user_device_io_set_irqs() is vulnerable.
I should to send a PR this week. Please tell me if you want plan
to respin soon. Or I will send v2 and we will address the rest
later.
Thanks,
C.
> -----Original Message-----
> From: Cédric Le Goater <clg@redhat.com>
> Sent: 21 July 2026 08:04
> To: Thanos Makatos <thanos.makatos@nutanix.com>; qemu-
> devel@nongnu.org
> Cc: John Levon <john.levon@nutanix.com>
> Subject: Re: [PATCH v2 0/6] vfio-user: validate server responses
>
> !-------------------------------------------------------------------|
> CAUTION: External Email
>
> |-------------------------------------------------------------------!
>
> On 7/20/26 17:55, Cédric Le Goater wrote:
> > Hello Thanos,
> >
> > On 7/20/26 15:58, Thanos Makatos wrote:
> >> This patch series fixes some bugs where the vfio-user client doesn't
> >> properly validate information returned by the vfio-user server.
> >>
> >> Thanos Makatos (6):
> >> vfio-user: vfio_user_get_region_info: prevent buffer overflow
> >> vfio-user: vfio_user_get_region_info: respect max_xfer_size
> >> vfio-user: vfio_user_get_region_info: reject unreasonably short struct
> >> vfio-user: vfio_user_get_region_info: prevent excessive malloc
> >> vfio-user: vfio_user_device_io_get_region_info: fix capability check
> >> vfio-user: vfio_user_device_io_device_feature: prevent buffer overflow
> >>
> >> hw/vfio-user/device.c | 40
> +++++++++++++++++++++++++++++++++++++---
> >> 1 file changed, 37 insertions(+), 3 deletions(-)
> >>
> >> Changes since v1:
> >> - added 'Resolves:' in patches 1 and 5
> >> - added missing SOB from patch 1
> >> - removed reassignment in patch 1
> >> - added a new patch to address the same issue as in patch 1
> >>
> >
> > Could you please add 'Fixes:' tags ? No need to resend, just
> > reply to the email you sent.
>
>
> Patches 1-5 address issues introduced by:
> 667866d66620 ("vfio-user: implement
> VFIO_USER_DEVICE_GET_REGION_INFO")
>
> Patch 6 addresses an issue introduced by:
> e2358af5838d ("vfio-user: support VFIO_USER_DEVICE_FEATURE")
>
> Also, in patch 6, the vfio_user_device_io_device_feature() code does
>
> memcpy(feature, &msgp->argsz, feature->argsz)
>
> without the hardening as done in region_info. That would be a good
> addition.
Added a patch.
>
> vfio_user_device_io_set_irqs() is vulnerable.
Added two patches.
>
> I should to send a PR this week. Please tell me if you want plan
> to respin soon. Or I will send v2 and we will address the rest
> later.
No worries I've just sent v3. I can fold all the buffer overflow patches into a single patch and all the excessive malloc patches into a single patch if you prefer, I don't mind either way.
>
> Thanks,
>
> C.
>
This patch series fixes some bugs where the vfio-user client doesn't
properly validate information returned by the vfio-user server.
Thanos Makatos (9):
vfio-user: vfio_user_get_region_info: prevent buffer overflow
vfio-user: vfio_user_get_region_info: respect max_xfer_size
vfio-user: vfio_user_get_region_info: reject unreasonably short struct
vfio-user: vfio_user_get_region_info: prevent excessive malloc
vfio-user: vfio_user_device_io_get_region_info: fix capability check
vfio-user: vfio_user_device_io_device_feature: prevent buffer overflow
vfio-user: vfio_user_device_io_device_feature: prevent excessive
malloc
vfio-user: vfio_user_device_io_set_irqs: prevent buffer overflow
vfio-user: vfio_user_device_io_set_irqs: prevent excessive malloc
hw/vfio-user/device.c | 54 +++++++++++++++++++++++++++++++++++++++----
1 file changed, 50 insertions(+), 4 deletions(-)
Changes since v1:
- added 'Resolves:' in patches 1 and 5
- added missing SOB from patch 1
- removed reassignment in patch 1
- added a new patch to address the same issue as in patch 1
Changes since v2:
- expanded cover letter
- added 'Fixes' tags
- improved check in patch 5
- removed extraneous new line in patch 6
- added patch 7 and 9 with same check as in patch 4
- added patch 8 with same check as in patch 6
Patches 1-5 address issues introduced by:
667866d66620 ("vfio-user: implement VFIO_USER_DEVICE_GET_REGION_INFO")
Patches 6-7 address issues introduced by:
e2358af5838d ("vfio-user: support VFIO_USER_DEVICE_FEATURE")
Patch 8-9 addresses issues introduced by:
ca1add1696 ("vfio-user: implement VFIO_USER_DEVICE_GET/SET_IRQ*")
--
2.34.1
On 7/21/26 14:26, Thanos Makatos wrote:
> This patch series fixes some bugs where the vfio-user client doesn't
> properly validate information returned by the vfio-user server.
>
>
> Thanos Makatos (9):
> vfio-user: vfio_user_get_region_info: prevent buffer overflow
> vfio-user: vfio_user_get_region_info: respect max_xfer_size
> vfio-user: vfio_user_get_region_info: reject unreasonably short struct
> vfio-user: vfio_user_get_region_info: prevent excessive malloc
> vfio-user: vfio_user_device_io_get_region_info: fix capability check
> vfio-user: vfio_user_device_io_device_feature: prevent buffer overflow
> vfio-user: vfio_user_device_io_device_feature: prevent excessive
> malloc
> vfio-user: vfio_user_device_io_set_irqs: prevent buffer overflow
> vfio-user: vfio_user_device_io_set_irqs: prevent excessive malloc
>
> hw/vfio-user/device.c | 54 +++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 50 insertions(+), 4 deletions(-)
>
> Changes since v1:
> - added 'Resolves:' in patches 1 and 5
> - added missing SOB from patch 1
> - removed reassignment in patch 1
> - added a new patch to address the same issue as in patch 1
>
> Changes since v2:
> - expanded cover letter
> - added 'Fixes' tags
> - improved check in patch 5
> - removed extraneous new line in patch 6
> - added patch 7 and 9 with same check as in patch 4
> - added patch 8 with same check as in patch 6
>
> Patches 1-5 address issues introduced by:
> 667866d66620 ("vfio-user: implement VFIO_USER_DEVICE_GET_REGION_INFO")
>
> Patches 6-7 address issues introduced by:
> e2358af5838d ("vfio-user: support VFIO_USER_DEVICE_FEATURE")
>
> Patch 8-9 addresses issues introduced by:
> ca1add1696 ("vfio-user: implement VFIO_USER_DEVICE_GET/SET_IRQ*")
>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Thanks,
C.
If the vfio-user responds with a value large enough such that adding
the header size to it overflows, a smaller buffer would be
inadvertently allocated, leading to buffer overflow.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3867
Fixes: 667866d66620 ("vfio-user: implement VFIO_USER_DEVICE_GET_REGION_INFO")
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
---
hw/vfio-user/device.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/hw/vfio-user/device.c b/hw/vfio-user/device.c
index b8d2b7c1a8..e58167a818 100644
--- a/hw/vfio-user/device.c
+++ b/hw/vfio-user/device.c
@@ -128,12 +128,21 @@ static int vfio_user_get_region_info(VFIOUserProxy *proxy,
error_printf("vfio_user_get_region_info argsz too small\n");
return -E2BIG;
}
+
+ /*
+ * Ensure that size doesn't overflow, otherwise we'll allocate a much
+ * smaller buffer than we need.
+ */
+ if (__builtin_add_overflow(info->argsz, sizeof(VFIOUserHdr), &size)) {
+ error_printf("vfio_user_get_region_info argsz too large\n");
+ return -E2BIG;
+ }
+
if (fds != NULL && fds->send_fds != 0) {
error_printf("vfio_user_get_region_info can't send FDs\n");
return -EINVAL;
}
- size = info->argsz + sizeof(VFIOUserHdr);
msgp = g_malloc0(size);
vfio_user_request_msg(&msgp->hdr, VFIO_USER_DEVICE_GET_REGION_INFO,
--
2.34.1
Fixes: 667866d66620 ("vfio-user: implement VFIO_USER_DEVICE_GET_REGION_INFO")
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
---
hw/vfio-user/device.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/hw/vfio-user/device.c b/hw/vfio-user/device.c
index e58167a818..8350bb2c01 100644
--- a/hw/vfio-user/device.c
+++ b/hw/vfio-user/device.c
@@ -137,6 +137,10 @@ static int vfio_user_get_region_info(VFIOUserProxy *proxy,
error_printf("vfio_user_get_region_info argsz too large\n");
return -E2BIG;
}
+ if (size > proxy->max_xfer_size) {
+ error_printf("vfio_user_get_region_info argsz too large\n");
+ return -E2BIG;
+ }
if (fds != NULL && fds->send_fds != 0) {
error_printf("vfio_user_get_region_info can't send FDs\n");
--
2.34.1
While this isn't technically a bug, it's highly unlikely that the
server wouldn't be writing an entire struct.
Fixes: 667866d66620 ("vfio-user: implement VFIO_USER_DEVICE_GET_REGION_INFO")
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
---
hw/vfio-user/device.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/hw/vfio-user/device.c b/hw/vfio-user/device.c
index 8350bb2c01..3d12e8b2a2 100644
--- a/hw/vfio-user/device.c
+++ b/hw/vfio-user/device.c
@@ -165,6 +165,11 @@ static int vfio_user_get_region_info(VFIOUserProxy *proxy,
}
trace_vfio_user_get_region_info(msgp->index, msgp->flags, msgp->size);
+ if (msgp->argsz < sizeof(*info)) {
+ error_printf("vfio_user_get_region_info reply argsz too small\n");
+ return -EINVAL;
+ }
+
memcpy(info, &msgp->argsz, info->argsz);
/*
--
2.34.1
If the vfio-user server responds with a value larger than max_xfer_size
vfio_device_get_region_info() blindly uses it in the next loop in
g_realloc. An value larger than max_xfer_size is anyway rejected by the
check at the beginning of vfio_user_get_region_info(), however that only
happens _after_ the g_realloc, and if that value is excessively large it
can cause g_realloc to fail, so check it here.
Fixes: 667866d66620 ("vfio-user: implement VFIO_USER_DEVICE_GET_REGION_INFO")
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
---
hw/vfio-user/device.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/hw/vfio-user/device.c b/hw/vfio-user/device.c
index 3d12e8b2a2..008e5cf687 100644
--- a/hw/vfio-user/device.c
+++ b/hw/vfio-user/device.c
@@ -170,6 +170,16 @@ static int vfio_user_get_region_info(VFIOUserProxy *proxy,
return -EINVAL;
}
+ /*
+ * The server can respond with a larger argsz in the reply to request a
+ * larger buffer on the next iteration via vfio_device_get_region_info().
+ * Reject values that would trigger an oversized realloc.
+ */
+ if (msgp->argsz > proxy->max_xfer_size) {
+ error_printf("vfio_user_get_region_info reply argsz too large\n");
+ return -E2BIG;
+ }
+
memcpy(info, &msgp->argsz, info->argsz);
/*
--
2.34.1
The existing check for PCI capabilities misses the case where
info->cap_offset == info->argsz, which results in accessing unallocated
memory. Fix the comparison.
Fixes: 667866d66620 ("vfio-user: implement VFIO_USER_DEVICE_GET_REGION_INFO")
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3865
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
---
hw/vfio-user/device.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/vfio-user/device.c b/hw/vfio-user/device.c
index 008e5cf687..3d0dbe8406 100644
--- a/hw/vfio-user/device.c
+++ b/hw/vfio-user/device.c
@@ -214,7 +214,8 @@ static int vfio_user_device_io_get_region_info(VFIODevice *vbasedev,
/* cap_offset in valid area */
if ((info->flags & VFIO_REGION_INFO_FLAG_CAPS) &&
- (info->cap_offset < sizeof(*info) || info->cap_offset > info->argsz)) {
+ (info->cap_offset < sizeof(*info)
+ || info->cap_offset + sizeof(struct vfio_info_cap_header) > info->argsz)) {
return -EINVAL;
}
--
2.34.1
This isn't in practise a problem since feature->argsz is not
externally provided, it's a good hardening step nonetheless.
Fixes: e2358af5838d ("vfio-user: support VFIO_USER_DEVICE_FEATURE")
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
---
hw/vfio-user/device.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/hw/vfio-user/device.c b/hw/vfio-user/device.c
index 3d0dbe8406..353fa5ee21 100644
--- a/hw/vfio-user/device.c
+++ b/hw/vfio-user/device.c
@@ -79,9 +79,14 @@ vfio_user_device_io_device_feature(VFIODevice *vbasedev,
struct vfio_device_feature *feature)
{
g_autofree VFIOUserDeviceFeature *msgp = NULL;
- int size = sizeof(VFIOUserHdr) + feature->argsz;
VFIOUserProxy *proxy = vbasedev->proxy;
Error *local_err = NULL;
+ int size;
+
+ if (__builtin_add_overflow(feature->argsz, sizeof(VFIOUserHdr), &size)) {
+ error_printf("vfio_user_device_io_device_feature argsz too large\n");
+ return -E2BIG;
+ }
msgp = g_malloc0(size);
--
2.34.1
This isn't in practise a problem since feature->argsz is not externally
provided, it's a good hardening step nonetheless.
Fixes: e2358af5838d ("vfio-user: support VFIO_USER_DEVICE_FEATURE")
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
---
hw/vfio-user/device.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/hw/vfio-user/device.c b/hw/vfio-user/device.c
index 353fa5ee21..8feaa56875 100644
--- a/hw/vfio-user/device.c
+++ b/hw/vfio-user/device.c
@@ -87,6 +87,10 @@ vfio_user_device_io_device_feature(VFIODevice *vbasedev,
error_printf("vfio_user_device_io_device_feature argsz too large\n");
return -E2BIG;
}
+ if (size > proxy->max_xfer_size) {
+ error_printf("vfio_user_device_io_device_feature argsz too large\n");
+ return -E2BIG;
+ }
msgp = g_malloc0(size);
--
2.34.1
This isn't in practise a problem since irq->argsz is not
externally provided, it's a good hardening step nonetheless.
Fixes: ca1add1696 ("vfio-user: implement VFIO_USER_DEVICE_GET/SET_IRQ*")
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
---
hw/vfio-user/device.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/vfio-user/device.c b/hw/vfio-user/device.c
index 8feaa56875..daac0e21b6 100644
--- a/hw/vfio-user/device.c
+++ b/hw/vfio-user/device.c
@@ -293,7 +293,10 @@ static int vfio_user_device_io_set_irqs(VFIODevice *vbasedev,
* Handle simple case
*/
if ((irq->flags & VFIO_IRQ_SET_DATA_EVENTFD) == 0) {
- size = sizeof(VFIOUserHdr) + irq->argsz;
+ if (__builtin_add_overflow(irq->argsz, sizeof(VFIOUserHdr), &size)) {
+ error_printf("vfio_user_set_irqs argsz too large\n");
+ return -E2BIG;
+ }
msgp = g_malloc0(size);
vfio_user_request_msg(&msgp->hdr, VFIO_USER_DEVICE_SET_IRQS, size, 0);
--
2.34.1
This isn't in practise a problem since irq->argsz is not externally
provided, it's a good hardening step nonetheless.
Fixes: ca1add1696 ("vfio-user: implement VFIO_USER_DEVICE_GET/SET_IRQ*")
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
---
hw/vfio-user/device.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/hw/vfio-user/device.c b/hw/vfio-user/device.c
index daac0e21b6..1a01e748cd 100644
--- a/hw/vfio-user/device.c
+++ b/hw/vfio-user/device.c
@@ -297,6 +297,11 @@ static int vfio_user_device_io_set_irqs(VFIODevice *vbasedev,
error_printf("vfio_user_set_irqs argsz too large\n");
return -E2BIG;
}
+ if (size > proxy->max_xfer_size) {
+ error_printf("vfio_user_device_io_set_irqs argsz too large\n");
+ return -E2BIG;
+ }
+
msgp = g_malloc0(size);
vfio_user_request_msg(&msgp->hdr, VFIO_USER_DEVICE_SET_IRQS, size, 0);
--
2.34.1
On 7/20/26 16:58, Thanos Makatos wrote: > This patch series fixes some bugs where the vfio-user client doesn't > properly validate information returned by the vfio-user server. > > Thanos Makatos (6): > vfio-user: vfio_user_get_region_info: prevent buffer overflow > vfio-user: vfio_user_get_region_info: respect max_xfer_size > vfio-user: vfio_user_get_region_info: reject unreasonably short struct > vfio-user: vfio_user_get_region_info: prevent excessive malloc > vfio-user: vfio_user_device_io_get_region_info: fix capability check > vfio-user: vfio_user_device_io_device_feature: prevent buffer overflow > > hw/vfio-user/device.c | 40 +++++++++++++++++++++++++++++++++++++--- > 1 file changed, 37 insertions(+), 3 deletions(-) I'm picking this patch set for the stable-11.0 branch, except of the last change which fixes a problem introduced after v11.0.0. Please let me know if I shouldn't. Thanks, /mjt
> -----Original Message----- > From: Michael Tokarev <mjt@tls.msk.ru> > Sent: 24 July 2026 09:51 > To: Thanos Makatos <thanos.makatos@nutanix.com>; qemu- > devel@nongnu.org > Cc: John Levon <john.levon@nutanix.com>; clg@redhat.com; QEMU Stable > <qemu-stable@nongnu.org> > Subject: Re: [PATCH v2 0/6] vfio-user: validate server responses > > !-------------------------------------------------------------------| > CAUTION: External Email > > |-------------------------------------------------------------------! > > On 7/20/26 16:58, Thanos Makatos wrote: > > This patch series fixes some bugs where the vfio-user client doesn't > > properly validate information returned by the vfio-user server. > > > > Thanos Makatos (6): > > vfio-user: vfio_user_get_region_info: prevent buffer overflow > > vfio-user: vfio_user_get_region_info: respect max_xfer_size > > vfio-user: vfio_user_get_region_info: reject unreasonably short struct > > vfio-user: vfio_user_get_region_info: prevent excessive malloc > > vfio-user: vfio_user_device_io_get_region_info: fix capability check > > vfio-user: vfio_user_device_io_device_feature: prevent buffer overflow > > > > hw/vfio-user/device.c | 40 > +++++++++++++++++++++++++++++++++++++--- > > 1 file changed, 37 insertions(+), 3 deletions(-) > > I'm picking this patch set for the stable-11.0 branch, except of the > last change which fixes a problem introduced after v11.0.0. > Please let me know if I shouldn't. Sounds good to me. > > Thanks, > > /mjt
If the vfio-user responds with a value large enough such that adding
the header size to it overflows, a smaller buffer would be
inadvertently allocated, leading to buffer overflow.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3867
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
---
hw/vfio-user/device.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/hw/vfio-user/device.c b/hw/vfio-user/device.c
index b8d2b7c1a8..e58167a818 100644
--- a/hw/vfio-user/device.c
+++ b/hw/vfio-user/device.c
@@ -128,12 +128,21 @@ static int vfio_user_get_region_info(VFIOUserProxy *proxy,
error_printf("vfio_user_get_region_info argsz too small\n");
return -E2BIG;
}
+
+ /*
+ * Ensure that size doesn't overflow, otherwise we'll allocate a much
+ * smaller buffer than we need.
+ */
+ if (__builtin_add_overflow(info->argsz, sizeof(VFIOUserHdr), &size)) {
+ error_printf("vfio_user_get_region_info argsz too large\n");
+ return -E2BIG;
+ }
+
if (fds != NULL && fds->send_fds != 0) {
error_printf("vfio_user_get_region_info can't send FDs\n");
return -EINVAL;
}
- size = info->argsz + sizeof(VFIOUserHdr);
msgp = g_malloc0(size);
vfio_user_request_msg(&msgp->hdr, VFIO_USER_DEVICE_GET_REGION_INFO,
--
2.34.1
On 7/20/26 15:58, Thanos Makatos wrote:
> If the vfio-user responds with a value large enough such that adding
> the header size to it overflows, a smaller buffer would be
> inadvertently allocated, leading to buffer overflow.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3867
>
> Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
> ---
> hw/vfio-user/device.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/hw/vfio-user/device.c b/hw/vfio-user/device.c
> index b8d2b7c1a8..e58167a818 100644
> --- a/hw/vfio-user/device.c
> +++ b/hw/vfio-user/device.c
> @@ -128,12 +128,21 @@ static int vfio_user_get_region_info(VFIOUserProxy *proxy,
> error_printf("vfio_user_get_region_info argsz too small\n");
> return -E2BIG;
> }
> +
> + /*
> + * Ensure that size doesn't overflow, otherwise we'll allocate a much
> + * smaller buffer than we need.
> + */
> + if (__builtin_add_overflow(info->argsz, sizeof(VFIOUserHdr), &size)) {
> + error_printf("vfio_user_get_region_info argsz too large\n");
> + return -E2BIG;
> + }
> +
> if (fds != NULL && fds->send_fds != 0) {
> error_printf("vfio_user_get_region_info can't send FDs\n");
> return -EINVAL;
> }
>
> - size = info->argsz + sizeof(VFIOUserHdr);
> msgp = g_malloc0(size);
>
> vfio_user_request_msg(&msgp->hdr, VFIO_USER_DEVICE_GET_REGION_INFO,
Fixes: 667866d66620 ("vfio-user: implement VFIO_USER_DEVICE_GET_REGION_INFO")
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Thanks,
C.
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
---
hw/vfio-user/device.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/hw/vfio-user/device.c b/hw/vfio-user/device.c
index e58167a818..8350bb2c01 100644
--- a/hw/vfio-user/device.c
+++ b/hw/vfio-user/device.c
@@ -137,6 +137,10 @@ static int vfio_user_get_region_info(VFIOUserProxy *proxy,
error_printf("vfio_user_get_region_info argsz too large\n");
return -E2BIG;
}
+ if (size > proxy->max_xfer_size) {
+ error_printf("vfio_user_get_region_info argsz too large\n");
+ return -E2BIG;
+ }
if (fds != NULL && fds->send_fds != 0) {
error_printf("vfio_user_get_region_info can't send FDs\n");
--
2.34.1
On 7/20/26 15:58, Thanos Makatos wrote:
> Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
> ---
> hw/vfio-user/device.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/hw/vfio-user/device.c b/hw/vfio-user/device.c
> index e58167a818..8350bb2c01 100644
> --- a/hw/vfio-user/device.c
> +++ b/hw/vfio-user/device.c
> @@ -137,6 +137,10 @@ static int vfio_user_get_region_info(VFIOUserProxy *proxy,
> error_printf("vfio_user_get_region_info argsz too large\n");
> return -E2BIG;
> }
> + if (size > proxy->max_xfer_size) {
> + error_printf("vfio_user_get_region_info argsz too large\n");
> + return -E2BIG;
> + }
>
> if (fds != NULL && fds->send_fds != 0) {
> error_printf("vfio_user_get_region_info can't send FDs\n");
Fixes: 667866d66620 ("vfio-user: implement VFIO_USER_DEVICE_GET_REGION_INFO")
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Thanks,
C.
While this isn't technically a bug, it's highly unlikely that the
server wouldn't be writing an entire struct.
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
---
hw/vfio-user/device.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/hw/vfio-user/device.c b/hw/vfio-user/device.c
index 8350bb2c01..3d12e8b2a2 100644
--- a/hw/vfio-user/device.c
+++ b/hw/vfio-user/device.c
@@ -165,6 +165,11 @@ static int vfio_user_get_region_info(VFIOUserProxy *proxy,
}
trace_vfio_user_get_region_info(msgp->index, msgp->flags, msgp->size);
+ if (msgp->argsz < sizeof(*info)) {
+ error_printf("vfio_user_get_region_info reply argsz too small\n");
+ return -EINVAL;
+ }
+
memcpy(info, &msgp->argsz, info->argsz);
/*
--
2.34.1
On 7/20/26 15:58, Thanos Makatos wrote:
> While this isn't technically a bug, it's highly unlikely that the
> server wouldn't be writing an entire struct.
>
> Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
> ---
> hw/vfio-user/device.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/hw/vfio-user/device.c b/hw/vfio-user/device.c
> index 8350bb2c01..3d12e8b2a2 100644
> --- a/hw/vfio-user/device.c
> +++ b/hw/vfio-user/device.c
> @@ -165,6 +165,11 @@ static int vfio_user_get_region_info(VFIOUserProxy *proxy,
> }
> trace_vfio_user_get_region_info(msgp->index, msgp->flags, msgp->size);
>
> + if (msgp->argsz < sizeof(*info)) {
> + error_printf("vfio_user_get_region_info reply argsz too small\n");
> + return -EINVAL;
> + }
> +
> memcpy(info, &msgp->argsz, info->argsz);
>
> /*
Fixes: 667866d66620 ("vfio-user: implement VFIO_USER_DEVICE_GET_REGION_INFO")
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Thanks,
C.
If the vfio-user server responds with a value larger than max_xfer_size
vfio_device_get_region_info() blindly uses it in the next loop in
g_realloc. An value larger than max_xfer_size is anyway rejected by the
check at the beginning of vfio_user_get_region_info(), however that only
happens _after_ the g_realloc, and if that value is excessively large it
can cause g_realloc to fail, so check it here.
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
---
hw/vfio-user/device.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/hw/vfio-user/device.c b/hw/vfio-user/device.c
index 3d12e8b2a2..008e5cf687 100644
--- a/hw/vfio-user/device.c
+++ b/hw/vfio-user/device.c
@@ -170,6 +170,16 @@ static int vfio_user_get_region_info(VFIOUserProxy *proxy,
return -EINVAL;
}
+ /*
+ * The server can respond with a larger argsz in the reply to request a
+ * larger buffer on the next iteration via vfio_device_get_region_info().
+ * Reject values that would trigger an oversized realloc.
+ */
+ if (msgp->argsz > proxy->max_xfer_size) {
+ error_printf("vfio_user_get_region_info reply argsz too large\n");
+ return -E2BIG;
+ }
+
memcpy(info, &msgp->argsz, info->argsz);
/*
--
2.34.1
On 7/20/26 15:58, Thanos Makatos wrote:
> If the vfio-user server responds with a value larger than max_xfer_size
> vfio_device_get_region_info() blindly uses it in the next loop in
> g_realloc. An value larger than max_xfer_size is anyway rejected by the
> check at the beginning of vfio_user_get_region_info(), however that only
> happens _after_ the g_realloc, and if that value is excessively large it
> can cause g_realloc to fail, so check it here.
>
> Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
> ---
> hw/vfio-user/device.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/hw/vfio-user/device.c b/hw/vfio-user/device.c
> index 3d12e8b2a2..008e5cf687 100644
> --- a/hw/vfio-user/device.c
> +++ b/hw/vfio-user/device.c
> @@ -170,6 +170,16 @@ static int vfio_user_get_region_info(VFIOUserProxy *proxy,
> return -EINVAL;
> }
>
> + /*
> + * The server can respond with a larger argsz in the reply to request a
> + * larger buffer on the next iteration via vfio_device_get_region_info().
> + * Reject values that would trigger an oversized realloc.
> + */
> + if (msgp->argsz > proxy->max_xfer_size) {
> + error_printf("vfio_user_get_region_info reply argsz too large\n");
> + return -E2BIG;
> + }
> +
> memcpy(info, &msgp->argsz, info->argsz);
>
> /*
Fixes: 667866d66620 ("vfio-user: implement VFIO_USER_DEVICE_GET_REGION_INFO")
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Thanks,
C.
The existing check for PCI capabilities misses the case where
info->cap_offset == info->argsz, which results in accessing unallocated
memory. Fix the comparison.
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3865
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
---
hw/vfio-user/device.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/vfio-user/device.c b/hw/vfio-user/device.c
index 008e5cf687..67736383b8 100644
--- a/hw/vfio-user/device.c
+++ b/hw/vfio-user/device.c
@@ -214,7 +214,7 @@ static int vfio_user_device_io_get_region_info(VFIODevice *vbasedev,
/* cap_offset in valid area */
if ((info->flags & VFIO_REGION_INFO_FLAG_CAPS) &&
- (info->cap_offset < sizeof(*info) || info->cap_offset > info->argsz)) {
+ (info->cap_offset < sizeof(*info) || info->cap_offset >= info->argsz)) {
return -EINVAL;
}
--
2.34.1
On 7/20/26 15:58, Thanos Makatos wrote:
> The existing check for PCI capabilities misses the case where
> info->cap_offset == info->argsz, which results in accessing unallocated
> memory. Fix the comparison.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3865
>
> Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
> ---
> hw/vfio-user/device.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/vfio-user/device.c b/hw/vfio-user/device.c
> index 008e5cf687..67736383b8 100644
> --- a/hw/vfio-user/device.c
> +++ b/hw/vfio-user/device.c
> @@ -214,7 +214,7 @@ static int vfio_user_device_io_get_region_info(VFIODevice *vbasedev,
>
> /* cap_offset in valid area */
> if ((info->flags & VFIO_REGION_INFO_FLAG_CAPS) &&
> - (info->cap_offset < sizeof(*info) || info->cap_offset > info->argsz)) {
> + (info->cap_offset < sizeof(*info) || info->cap_offset >= info->argsz)) {
> return -EINVAL;
> }
>
Fixes: 667866d66620 ("vfio-user: implement VFIO_USER_DEVICE_GET_REGION_INFO")
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Thanks,
C.
This isn't in practise a problem since feature->argsz is not
externally provided, it's a good hardening step nonetheless.
Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
---
hw/vfio-user/device.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/hw/vfio-user/device.c b/hw/vfio-user/device.c
index 67736383b8..131be7af61 100644
--- a/hw/vfio-user/device.c
+++ b/hw/vfio-user/device.c
@@ -79,9 +79,15 @@ vfio_user_device_io_device_feature(VFIODevice *vbasedev,
struct vfio_device_feature *feature)
{
g_autofree VFIOUserDeviceFeature *msgp = NULL;
- int size = sizeof(VFIOUserHdr) + feature->argsz;
VFIOUserProxy *proxy = vbasedev->proxy;
Error *local_err = NULL;
+ int size;
+
+ if (__builtin_add_overflow(feature->argsz, sizeof(VFIOUserHdr), &size)) {
+ error_printf("vfio_user_device_io_device_feature argsz too large\n");
+ return -E2BIG;
+ }
+
msgp = g_malloc0(size);
--
2.34.1
On 7/20/26 16:58, Thanos Makatos wrote: > This isn't in practise a problem since feature->argsz is not > externally provided, it's a good hardening step nonetheless. "In practiCe", fwiw (it's applied already). Thanks, /mjt
On 7/20/26 15:58, Thanos Makatos wrote:
> This isn't in practise a problem since feature->argsz is not
> externally provided, it's a good hardening step nonetheless.
>
> Signed-off-by: Thanos Makatos <thanos.makatos@nutanix.com>
> ---
> hw/vfio-user/device.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/hw/vfio-user/device.c b/hw/vfio-user/device.c
> index 67736383b8..131be7af61 100644
> --- a/hw/vfio-user/device.c
> +++ b/hw/vfio-user/device.c
> @@ -79,9 +79,15 @@ vfio_user_device_io_device_feature(VFIODevice *vbasedev,
> struct vfio_device_feature *feature)
> {
> g_autofree VFIOUserDeviceFeature *msgp = NULL;
> - int size = sizeof(VFIOUserHdr) + feature->argsz;
> VFIOUserProxy *proxy = vbasedev->proxy;
> Error *local_err = NULL;
> + int size;
> +
> + if (__builtin_add_overflow(feature->argsz, sizeof(VFIOUserHdr), &size)) {
> + error_printf("vfio_user_device_io_device_feature argsz too large\n");
> + return -E2BIG;
> + }
> +
>
> msgp = g_malloc0(size);
>
Fixes: e2358af5838d ("vfio-user: support VFIO_USER_DEVICE_FEATURE")
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Thanks,
C.
© 2016 - 2026 Red Hat, Inc.