[PATCH v3] staging: vme_user: validate slave window size against buffer size

[PATCH v3] staging: vme_user: validate slave window size against buffer size

[Rion Kiguchi]

The VME_SET_SLAVE ioctl in drivers/staging/vme_user/vme_user.c accepts
a user-controlled slave.size and forwards it to vme_slave_set() without
comparing it against image[minor].size_buf. The slave-image kernel
buffer is allocated at probe time with a fixed size of PCI_BUF_SIZE
(0x20000 / 128 KiB), but the configured VME window size can be made
much larger via the ioctl.

The subsequent read() / write() handlers (vme_user_read /
vme_user_write) clamp the I/O range against vme_get_size(), which
returns the size the bridge driver has programmed for the window
(i.e. the attacker-supplied slave.size). vme_get_size() does not
consult size_buf, so an oversized window passes the existing bounds
checks, and buffer_to_user() / buffer_from_user() then index
image[minor].kern_buf with offsets beyond the actual allocation.

Result: a local user with read/write access to /dev/bus/vme/s* can
trigger out-of-bounds read and write of the kernel slab adjacent to
the slave-image buffer.

Fix: reject slave.size > size_buf in the VME_SET_SLAVE handler.
With this check in place, the existing bounds checks in
vme_user_read() / vme_user_write() against vme_get_size() are
sufficient to prevent OOB access; no additional checks in
buffer_to_user() / buffer_from_user() are needed.

Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Rion Kiguchi <kiguchi.r.sec@gmail.com>
---
Changes in v3:
 - Drop redundant checks in buffer_to_user() / buffer_from_user();
   the existing vme_get_size()-based bounds checks in vme_user_read()
   / vme_user_write() are sufficient once VME_SET_SLAVE rejects
   oversized windows (Greg's review feedback)
 - Reword commit message to explain why vme_get_size() does not
   already catch this

Changes in v2:
 - Use git send-email instead of Gmail web compose (v1 corrupted
   the diff)
 - Drop redundant Reported-by tag (author == reporter)
 - Add Assisted-by tag per Documentation/process/coding-assistants.rst

 drivers/staging/vme_user/vme_user.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/vme_user/vme_user.c b/drivers/staging/vme_user/vme_user.c
index 11e25c2f6..64e95b026 100644
--- a/drivers/staging/vme_user/vme_user.c
+++ b/drivers/staging/vme_user/vme_user.c
@@ -394,6 +394,14 @@ static int vme_user_ioctl(struct inode *inode, struct file *file,
 				return -EFAULT;
 			}
 
+			/*
+			 * Reject window sizes larger than the kernel buffer
+			 * allocated at probe time, otherwise subsequent
+			 * read/write would access memory beyond kern_buf.
+			 */
+			if (slave.size > image[minor].size_buf)
+				return -EINVAL;
+
 			/* XXX	We do not want to push aspace, cycle and width
 			 *	to userspace as they are
 			 */
@@ -401,7 +409,7 @@ static int vme_user_ioctl(struct inode *inode, struct file *file,
 				slave.enable, slave.vme_addr, slave.size,
 				image[minor].pci_buf, slave.aspace,
 				slave.cycle);
-
+				
 			break;
 		}
 		break;
-- 
2.43.0

Re: [PATCH v3] staging: vme_user: validate slave window size against buffer size

[Greg Kroah-Hartman]

On Sat, May 09, 2026 at 06:26:27PM +0900, Rion Kiguchi wrote:
> The VME_SET_SLAVE ioctl in drivers/staging/vme_user/vme_user.c accepts
> a user-controlled slave.size and forwards it to vme_slave_set() without
> comparing it against image[minor].size_buf. The slave-image kernel
> buffer is allocated at probe time with a fixed size of PCI_BUF_SIZE
> (0x20000 / 128 KiB), but the configured VME window size can be made
> much larger via the ioctl.
> 
> The subsequent read() / write() handlers (vme_user_read /
> vme_user_write) clamp the I/O range against vme_get_size(), which
> returns the size the bridge driver has programmed for the window
> (i.e. the attacker-supplied slave.size). vme_get_size() does not
> consult size_buf, so an oversized window passes the existing bounds
> checks, and buffer_to_user() / buffer_from_user() then index
> image[minor].kern_buf with offsets beyond the actual allocation.
> 
> Result: a local user with read/write access to /dev/bus/vme/s* can
> trigger out-of-bounds read and write of the kernel slab adjacent to
> the slave-image buffer.
> 
> Fix: reject slave.size > size_buf in the VME_SET_SLAVE handler.
> With this check in place, the existing bounds checks in
> vme_user_read() / vme_user_write() against vme_get_size() are
> sufficient to prevent OOB access; no additional checks in
> buffer_to_user() / buffer_from_user() are needed.
> 
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4-7
> Signed-off-by: Rion Kiguchi <kiguchi.r.sec@gmail.com>
> ---
> Changes in v3:
>  - Drop redundant checks in buffer_to_user() / buffer_from_user();
>    the existing vme_get_size()-based bounds checks in vme_user_read()
>    / vme_user_write() are sufficient once VME_SET_SLAVE rejects
>    oversized windows (Greg's review feedback)
>  - Reword commit message to explain why vme_get_size() does not
>    already catch this

Please slow down.  Take some time (i.e. a few days) between versions and
read all of the review comments before sending a new one (you ignored my
past review).  Relax and wait a few days, do some testing, and actually
verify that your fix is correct (I don't think it is.)

And of course, actually use checkpatch.pl to verify that you are not
adding a new coding style issue to the file (which this patch does).

There is no rush here.  Take your time, this isn't a real problem that
must be solved immediately.

this also should have been v4, not v3 :(

And finally, no need to cc: security@k.o, that alias has nothing to do
with this issue as it is not a real security problem and is now public.

thanks,

greg k-h

[PATCH v4] staging: vme_user: validate slave window size against buffer size

[Rion Kiguchi]

The VME_SET_SLAVE ioctl in drivers/staging/vme_user/vme_user.c accepts
a user-controlled slave.size and forwards it to vme_slave_set() without
comparing it against image[minor].size_buf. The slave-image kernel
buffer is allocated at probe time with a fixed size of PCI_BUF_SIZE
(0x20000 / 128 KiB), but the configured VME window size can be made
much larger via the ioctl.

Additionally, a slave.size of 0 is permitted, which causes vme_get_size()
to return 0. In vme_user_read() and vme_user_write(), the boundary check
(*ppos > image_size - 1) suffers from an integer underflow because
image_size is size_t. This bypasses the bounds check entirely, allowing
offsets beyond the actual allocation.

Result: a local user with read/write access to /dev/bus/vme/s* can
trigger out-of-bounds read and write of the kernel slab adjacent to
the slave-image buffer.

Fix: reject slave.size == 0 and slave.size > size_buf in the VME_SET_SLAVE
handler. With this check in place, the existing bounds checks in
vme_user_read() / vme_user_write() against vme_get_size() are
sufficient to prevent OOB access.

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Rion Kiguchi <kiguchi.r.sec@gmail.com>
---
Changes in v4:
 - Add check for slave.size == 0 to prevent integer underflow in bounds check.
 - Remove trailing whitespaces inadvertently added in previous versions.
 - Remove Fixes and Cc: stable tags to resolve checkpatch warnings based on maintainer feedback.

Changes in v3:
 - Drop redundant checks in buffer_to_user() / buffer_from_user();
   the existing vme_get_size()-based bounds checks in vme_user_read()
   / vme_user_write() are sufficient once VME_SET_SLAVE rejects
   oversized windows.

Changes in v2:
 - Use git send-email instead of Gmail web compose.
 - Drop redundant Reported-by tag.
 - Add Assisted-by tag.

 drivers/staging/vme_user/vme_user.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/staging/vme_user/vme_user.c b/drivers/staging/vme_user/vme_user.c
index 11e25c2f6..ba4c9a6d0 100644
--- a/drivers/staging/vme_user/vme_user.c
+++ b/drivers/staging/vme_user/vme_user.c
@@ -394,6 +394,16 @@ static int vme_user_ioctl(struct inode *inode, struct file *file,
 				return -EFAULT;
 			}
 
+			/*
+			 * Reject window sizes larger than the kernel buffer
+			 * allocated at probe time, otherwise subsequent
+			 * read/write would access memory beyond kern_buf.
+			 * Also reject size == 0 to prevent integer underflow
+			 * in the boundary check (*ppos > image_size - 1).
+			 */
+			if (slave.size == 0 || slave.size > image[minor].size_buf)
+				return -EINVAL;
+
 			/* XXX	We do not want to push aspace, cycle and width
 			 *	to userspace as they are
 			 */
-- 
2.43.0