[PATCH 0/2] vhost-scsi: fix max_io_vqs handling

[PATCH 0/2] vhost-scsi: fix max_io_vqs handling

[Dongli Zhang]

Avoid high-order kmalloc() allocations for the vhost-scsi virtqueue array,
and reject invalid max_io_vqs updates so the stored value remains within
the supported range.

Dongli Zhang (2):
  vhost-scsi: use kvzalloc for vq array allocation
  vhost-scsi: reject invalid max_io_vqs module parameter

 drivers/vhost/scsi.c | 33 +++++++++++++++++++--------------
 1 file changed, 19 insertions(+), 14 deletions(-)

base-commit: 8ba098e6b6ff0db8edf28528d1552be261af30d4

Thank you very much!

Dongli Zhang

[PATCH 1/2] vhost-scsi: use kvzalloc for vq array allocation

[Dongli Zhang]

vhost_scsi_open() allocates one "struct vhost_scsi_virtqueue" for each
virtqueue. With large max_io_vqs values, this array can require a
high-order contiguous allocation and trigger a page allocator warning.

hv# cat /sys/module/vhost_scsi/parameters/max_io_vqs
256

[  766.075787] ------------[ cut here ]------------
[  766.077030] WARNING: mm/page_alloc.c:5280 at __alloc_frozen_pages_noprof+0x32c/0x15c0, CPU#23: qemu-system-x86/5964
... ...
[  766.080351] RIP: 0010:__alloc_frozen_pages_noprof+0x32c/0x15c0
... ...
[  766.085813] Call Trace:
[  766.085969]  <TASK>
[  766.086098]  ? srso_alias_return_thunk+0x5/0xfbef5
[  766.086365]  ? context_struct_compute_av+0x38a/0x4b0
[  766.086652]  alloc_pages_mpol+0x9f/0x170
[  766.086883]  ___kmalloc_large_node+0xb6/0xd0
[  766.087124]  ? srso_alias_return_thunk+0x5/0xfbef5
[  766.087389]  __kmalloc_large_node_noprof+0x18/0xa0
[  766.087655]  __kmalloc_noprof+0x3a0/0x440
[  766.087877]  ? vhost_scsi_open+0xcb/0x2d0 [vhost_scsi]
[  766.088162]  vhost_scsi_open+0xcb/0x2d0 [vhost_scsi]
[  766.088449]  misc_open+0x123/0x160
[  766.088679]  chrdev_open+0xb1/0x230
[  766.088885]  ? __pfx_chrdev_open+0x10/0x10
[  766.089157]  do_dentry_open+0x11a/0x470
[  766.089389]  vfs_open+0x29/0xf0
[  766.089596]  path_openat+0x7c0/0x1100
[  766.089821]  do_file_open+0xdd/0x190
[  766.090032]  ? srso_alias_return_thunk+0x5/0xfbef5
[  766.090332]  do_sys_openat2+0x7e/0x100
[  766.090601]  __x64_sys_openat+0x51/0xa0
[  766.090857]  do_syscall_64+0xfe/0x590
[  766.091087]  entry_SYSCALL_64_after_hwframe+0x77/0x7f
[  766.091411] RIP: 0033:0x7f9525a11fa6

The array does not require physical contiguity, so allocate it with
kvzalloc_objs() and free it with kvfree().

Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
---
 drivers/vhost/scsi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 9a1253b9d8c5..0c0634eea144 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -2295,7 +2295,7 @@ static int vhost_scsi_open(struct inode *inode, struct file *f)
 	if (!vs->old_inflight)
 		goto err_inflight;
 
-	vs->vqs = kmalloc_objs(*vs->vqs, nvqs, GFP_KERNEL | __GFP_ZERO);
+	vs->vqs = kvzalloc_objs(*vs->vqs, nvqs);
 	if (!vs->vqs)
 		goto err_vqs;
 
@@ -2331,7 +2331,7 @@ static int vhost_scsi_open(struct inode *inode, struct file *f)
 	return 0;
 
 err_local_vqs:
-	kfree(vs->vqs);
+	kvfree(vs->vqs);
 err_vqs:
 	kfree(vs->old_inflight);
 err_inflight:
@@ -2352,7 +2352,7 @@ static int vhost_scsi_release(struct inode *inode, struct file *f)
 	vhost_dev_stop(&vs->dev);
 	vhost_dev_cleanup(&vs->dev);
 	kfree(vs->dev.vqs);
-	kfree(vs->vqs);
+	kvfree(vs->vqs);
 	kfree(vs->old_inflight);
 	kvfree(vs);
 	return 0;
-- 
2.43.5

Re: [PATCH 1/2] vhost-scsi: use kvzalloc for vq array allocation

[Dongli Zhang]

+ virtualization@lists.linux.dev.

On 2026-07-31 3:24 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The `max_io_vqs` module parameter is parsed as an unsigned integer but assigned to a signed `int` in `vhost_scsi_open()`. Setting this parameter to a large unsigned value (e.g., `4294967295`) results in a negative value (e.g., `-1`) that bypasses the maximum bound check. This leads to an undersized allocation followed by statically indexed Out-Of-Bounds (OOB) writes.
> --


I think this pre-existing issue is implicitly fixed by PATCH 2/2, where
max_io_vqs is validated by param_set_uint_minmax(val, kp, 1,
VHOST_SCSI_MAX_IO_VQ) before the value is stored.

Changing the local nvqs variable from int to unsigned int in PATCH 2/2 would
make the type flow clearer, but the issue should no longer be reachable once
both patches are applied.

Dongli Zhang

> 
> --- Patch [1]: [PATCH 1/2] vhost-scsi: use kvzalloc for vq array allocation ---
> commit 3573b684f78f79b1ae3460e4a7625cb917f76044
> Author: Dongli Zhang <dongli.zhang@oracle.com>
> 
> vhost-scsi: use kvzalloc for vq array allocation
> 
> vhost_scsi_open() allocates one struct vhost_scsi_virtqueue for each
> virtqueue. With large max_io_vqs values, this array can require a
> high-order contiguous allocation and trigger a page allocator warning.
> This patch replaces kmalloc_objs with kvzalloc_objs to avoid the
> warning.
> 
>> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
>> index 9a1253b9d8c50..0c0634eea1443 100644
>> --- a/drivers/vhost/scsi.c
>> +++ b/drivers/vhost/scsi.c
>> @@ -2295,7 +2295,7 @@ static int vhost_scsi_open(struct inode *inode, struct file *f)
>>  	if (!vs->old_inflight)
>>  		goto err_inflight;
>>  
>> -	vs->vqs = kmalloc_objs(*vs->vqs, nvqs, GFP_KERNEL | __GFP_ZERO);
>> +	vs->vqs = kvzalloc_objs(*vs->vqs, nvqs);
> 
> [Severity: High]
> This is a pre-existing issue, but could this allocation be undersized due to
> an integer overflow in the nvqs calculation?
> 
> If a privileged user sets the max_io_vqs module parameter to a value that
> overflows the signed integer nvqs (e.g., 4294967295), nvqs could become
> negative (e.g., -1). Because it is signed, this bypasses the maximum bounds
> check against VHOST_SCSI_MAX_IO_VQ.
> 
> When VHOST_SCSI_VQ_IO (2) is added, nvqs becomes 1. The allocation is then
> sized for exactly 1 element:
> 
>     vs->vqs = kvzalloc_objs(*vs->vqs, nvqs);
> 
> However, the code later unconditionally writes to index 1 (VHOST_SCSI_VQ_EVT),
> which would cause a heap out-of-bounds write:
> 
>     vqs[VHOST_SCSI_VQ_EVT] = &vs->vqs[VHOST_SCSI_VQ_EVT].vq;
> 
> Would it be safer to use an unsigned type for nvqs and related variables to
> prevent this bounds check bypass?
> 

[PATCH 2/2] vhost-scsi: reject invalid max_io_vqs module parameter

[Dongli Zhang]

max_io_vqs is currently validated only when a vhost-scsi device is opened.
This allows sysfs to show values larger than the driver will actually use,
e.g. writing 2048 succeeds even though vhost_scsi_open() later clamps it to
VHOST_SCSI_MAX_IO_VQ. This makes the sysfs value differ from the value that
will actually be used.

hv# echo 2048 > /sys/module/vhost_scsi/parameters/max_io_vqs

hv# cat /sys/module/vhost_scsi/parameters/max_io_vqs
2048

[  315.630495] Invalid max_io_vqs of 2048. Using 1024.

Use a module parameter callback to reject out-of-range updates when the
parameter is written. This keeps the visible parameter value consistent
with the effective queue limit.

With the parameter value kept in range, remove the duplicate validation
from vhost_scsi_open().

Invalid values now fail with -EINVAL at write/load time instead of being
accepted and silently adjusted later when the device is opened.

Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
---
 drivers/vhost/scsi.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 0c0634eea144..c138c29ce3dd 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -210,7 +210,20 @@ static const int vhost_scsi_bits[] = {
 #define VHOST_SCSI_MAX_EVENT	128
 
 static unsigned vhost_scsi_max_io_vqs = 128;
-module_param_named(max_io_vqs, vhost_scsi_max_io_vqs, uint, 0644);
+
+static int vhost_scsi_set_max_io_vqs(const char *val,
+				     const struct kernel_param *kp)
+{
+	return param_set_uint_minmax(val, kp, 1, VHOST_SCSI_MAX_IO_VQ);
+}
+
+static const struct kernel_param_ops vhost_scsi_max_io_vqs_op = {
+	.set = vhost_scsi_set_max_io_vqs,
+	.get = param_get_uint,
+};
+
+module_param_cb(max_io_vqs, &vhost_scsi_max_io_vqs_op,
+		&vhost_scsi_max_io_vqs, 0644);
 MODULE_PARM_DESC(max_io_vqs, "Set the max number of IO virtqueues a vhost scsi device can support. The default is 128. The max is 1024.");
 
 struct vhost_scsi_virtqueue {
@@ -2273,22 +2286,14 @@ static int vhost_scsi_open(struct inode *inode, struct file *f)
 	struct vhost_scsi_virtqueue *svq;
 	struct vhost_scsi *vs;
 	struct vhost_virtqueue **vqs;
-	int r = -ENOMEM, i, nvqs = vhost_scsi_max_io_vqs;
+	int r = -ENOMEM, i, nvqs;
 
 	vs = kvzalloc_obj(*vs);
 	if (!vs)
 		goto err_vs;
 	vs->inline_sg_cnt = vhost_scsi_inline_sg_cnt;
 
-	if (nvqs > VHOST_SCSI_MAX_IO_VQ) {
-		pr_err("Invalid max_io_vqs of %d. Using %d.\n", nvqs,
-		       VHOST_SCSI_MAX_IO_VQ);
-		nvqs = VHOST_SCSI_MAX_IO_VQ;
-	} else if (nvqs == 0) {
-		pr_err("Invalid max_io_vqs of %d. Using 1.\n", nvqs);
-		nvqs = 1;
-	}
-	nvqs += VHOST_SCSI_VQ_IO;
+	nvqs = vhost_scsi_max_io_vqs + VHOST_SCSI_VQ_IO;
 
 	vs->old_inflight = kmalloc_objs(*vs->old_inflight, nvqs,
 					GFP_KERNEL | __GFP_ZERO);
-- 
2.43.5

Re: [PATCH 2/2] vhost-scsi: reject invalid max_io_vqs module parameter

[Dongli Zhang]

+ virtualization@lists.linux.dev

On 2026-07-31 3:26 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Missing READ_ONCE() on module parameter allows compiler rematerialization, leading to TOCTOU array out-of-bounds access.
> --
> 
> --- Patch [2]: [PATCH 2/2] vhost-scsi: reject invalid max_io_vqs module parameter ---
> commit 93f3d55f364233d31773cdb70b64bd78dafbf480
> Author: Dongli Zhang <dongli.zhang@oracle.com>
> 
> vhost-scsi: reject invalid max_io_vqs module parameter
> 
> This commit shifts the validation of the max_io_vqs module parameter from
> device open time to when the parameter is written. It removes the
> bounds-checking code from vhost_scsi_open() and uses a module parameter
> callback to reject invalid sysfs writes instead.
> 
>> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
>> index 0c0634eea1443..c138c29ce3dd4 100644
>> --- a/drivers/vhost/scsi.c
>> +++ b/drivers/vhost/scsi.c
> [ ... ]
>> @@ -2273,22 +2286,14 @@ static int vhost_scsi_open(struct inode *inode, struct file *f)
>>  	struct vhost_scsi_virtqueue *svq;
>>  	struct vhost_scsi *vs;
>>  	struct vhost_virtqueue **vqs;
>> -	int r = -ENOMEM, i, nvqs = vhost_scsi_max_io_vqs;
>> +	int r = -ENOMEM, i, nvqs;
>>  
>>  	vs = kvzalloc_obj(*vs);
>>  	if (!vs)
>>  		goto err_vs;
>>  	vs->inline_sg_cnt = vhost_scsi_inline_sg_cnt;
>>  
>> -	if (nvqs > VHOST_SCSI_MAX_IO_VQ) {
>> -		pr_err("Invalid max_io_vqs of %d. Using %d.\n", nvqs,
>> -		       VHOST_SCSI_MAX_IO_VQ);
>> -		nvqs = VHOST_SCSI_MAX_IO_VQ;
>> -	} else if (nvqs == 0) {
>> -		pr_err("Invalid max_io_vqs of %d. Using 1.\n", nvqs);
>> -		nvqs = 1;
>> -	}
>> -	nvqs += VHOST_SCSI_VQ_IO;
>> +	nvqs = vhost_scsi_max_io_vqs + VHOST_SCSI_VQ_IO;
> 
> [Severity: Medium]
> Since vhost_scsi_max_io_vqs is modifiable via sysfs (0644 permissions), could
> the compiler rematerialize nvqs by reloading the global variable from memory
> multiple times in this function?
> 
> Without a READ_ONCE(), the compiler is permitted to reload
> vhost_scsi_max_io_vqs instead of spilling nvqs to the stack across the
> subsequent kmalloc_objs() and kvzalloc_objs() calls. If a concurrent sysfs
> write occurs, the reloaded values can differ.
> 
> Could this cause the array allocation sizes to be smaller than the nvqs
> value used in the subsequent initialization loop, resulting in an
> out-of-bounds access?
> 
> For example, if the compiler decides to reload the variable:
> 
> vs->old_inflight = kmalloc_objs(*vs->old_inflight, nvqs, ...);
> // ...
> vs->vqs = kvzalloc_objs(*vs->vqs, nvqs);
> // ...
> for (i = VHOST_SCSI_VQ_IO; i < nvqs; i++) {
>         svq = &vs->vqs[i];
> 
> Previously, the complex bounds-checking block likely prevented the compiler
> from reloading the variable, but with the check removed, the value is read
> directly. Should READ_ONCE(vhost_scsi_max_io_vqs) be used here?
> 

I can make the change below, either in v2 of PATCH 2/2, or as an additional
follow-up patch, depending on whether the maintainer would prefer a v2:

nvqs = READ_ONCE(vhost_scsi_max_io_vqs) + VHOST_SCSI_VQ_IO;

Thank you very much!

Dongli Zhang

Re: [PATCH 2/2] vhost-scsi: reject invalid max_io_vqs module parameter

[Michael S. Tsirkin]

On Fri, Jul 31, 2026 at 03:10:24PM -0700, Dongli Zhang wrote:
> max_io_vqs is currently validated only when a vhost-scsi device is opened.
> This allows sysfs to show values larger than the driver will actually use,
> e.g. writing 2048 succeeds even though vhost_scsi_open() later clamps it to
> VHOST_SCSI_MAX_IO_VQ. This makes the sysfs value differ from the value that
> will actually be used.
> 
> hv# echo 2048 > /sys/module/vhost_scsi/parameters/max_io_vqs
> 
> hv# cat /sys/module/vhost_scsi/parameters/max_io_vqs
> 2048
> 
> [  315.630495] Invalid max_io_vqs of 2048. Using 1024.
> 
> Use a module parameter callback to reject out-of-range updates when the
> parameter is written. This keeps the visible parameter value consistent
> with the effective queue limit.
> 
> With the parameter value kept in range, remove the duplicate validation
> from vhost_scsi_open().
> 
> Invalid values now fail with -EINVAL at write/load time instead of being
> accepted and silently adjusted later when the device is opened.
> 
> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>

why should we change userspace visible behaviour now?

> ---
>  drivers/vhost/scsi.c | 27 ++++++++++++++++-----------
>  1 file changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index 0c0634eea144..c138c29ce3dd 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -210,7 +210,20 @@ static const int vhost_scsi_bits[] = {
>  #define VHOST_SCSI_MAX_EVENT	128
>  
>  static unsigned vhost_scsi_max_io_vqs = 128;
> -module_param_named(max_io_vqs, vhost_scsi_max_io_vqs, uint, 0644);
> +
> +static int vhost_scsi_set_max_io_vqs(const char *val,
> +				     const struct kernel_param *kp)
> +{
> +	return param_set_uint_minmax(val, kp, 1, VHOST_SCSI_MAX_IO_VQ);
> +}
> +
> +static const struct kernel_param_ops vhost_scsi_max_io_vqs_op = {
> +	.set = vhost_scsi_set_max_io_vqs,
> +	.get = param_get_uint,
> +};
> +
> +module_param_cb(max_io_vqs, &vhost_scsi_max_io_vqs_op,
> +		&vhost_scsi_max_io_vqs, 0644);
>  MODULE_PARM_DESC(max_io_vqs, "Set the max number of IO virtqueues a vhost scsi device can support. The default is 128. The max is 1024.");
>  
>  struct vhost_scsi_virtqueue {
> @@ -2273,22 +2286,14 @@ static int vhost_scsi_open(struct inode *inode, struct file *f)
>  	struct vhost_scsi_virtqueue *svq;
>  	struct vhost_scsi *vs;
>  	struct vhost_virtqueue **vqs;
> -	int r = -ENOMEM, i, nvqs = vhost_scsi_max_io_vqs;
> +	int r = -ENOMEM, i, nvqs;
>  
>  	vs = kvzalloc_obj(*vs);
>  	if (!vs)
>  		goto err_vs;
>  	vs->inline_sg_cnt = vhost_scsi_inline_sg_cnt;
>  
> -	if (nvqs > VHOST_SCSI_MAX_IO_VQ) {
> -		pr_err("Invalid max_io_vqs of %d. Using %d.\n", nvqs,
> -		       VHOST_SCSI_MAX_IO_VQ);
> -		nvqs = VHOST_SCSI_MAX_IO_VQ;
> -	} else if (nvqs == 0) {
> -		pr_err("Invalid max_io_vqs of %d. Using 1.\n", nvqs);
> -		nvqs = 1;
> -	}
> -	nvqs += VHOST_SCSI_VQ_IO;
> +	nvqs = vhost_scsi_max_io_vqs + VHOST_SCSI_VQ_IO;
>  
>  	vs->old_inflight = kmalloc_objs(*vs->old_inflight, nvqs,
>  					GFP_KERNEL | __GFP_ZERO);
> -- 
> 2.43.5