+ [email protected]. On 2026-07-31 3:24 PM, [email protected] 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 <[email protected]> > > 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? >
