* [PATCH] staging: vme_user: validate slave window size against buffer size
@ 2026-05-09 7:53 Rion Kiguchi
2026-05-09 8:04 ` Greg KH
0 siblings, 1 reply; 7+ messages in thread
From: Rion Kiguchi @ 2026-05-09 7:53 UTC (permalink / raw)
To: kiguchi.r.sec; +Cc: stable
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() (the
configured window size, attacker-controlled) but never consult
size_buf. The slave I/O paths buffer_to_user() and buffer_from_user()
then index image[minor].kern_buf with *ppos values up to
image_size - 1, well 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. Also
add defensive bounds checks against size_buf in buffer_to_user() and
buffer_from_user() so that the I/O paths cannot exceed the
allocation even if a future ioctl path forgets to validate.
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Rion Kiguchi <kiguchi.r.sec@gmail.com>
---
drivers/staging/vme_user/vme_user.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/vme_user/vme_user.c b/drivers/staging/vme_user/vme_user.c
index 11e25c2f6..41b8d5b51 100644
--- a/drivers/staging/vme_user/vme_user.c
+++ b/drivers/staging/vme_user/vme_user.c
@@ -156,6 +156,11 @@ static ssize_t buffer_to_user(unsigned int minor, char __user *buf,
{
void *image_ptr;
+ if (*ppos < 0 || (u64)*ppos >= image[minor].size_buf ||
+ count > image[minor].size_buf - (u64)*ppos) {
+ pr_warn_ratelimited("%s: out-of-bounds access\n", __func__);
+ return -EINVAL;
+ }
image_ptr = image[minor].kern_buf + *ppos;
if (copy_to_user(buf, image_ptr, (unsigned long)count))
return -EFAULT;
@@ -168,6 +173,11 @@ static ssize_t buffer_from_user(unsigned int minor, const char __user *buf,
{
void *image_ptr;
+ if (*ppos < 0 || (u64)*ppos >= image[minor].size_buf ||
+ count > image[minor].size_buf - (u64)*ppos) {
+ pr_warn_ratelimited("%s: out-of-bounds access\n", __func__);
+ return -EINVAL;
+ }
image_ptr = image[minor].kern_buf + *ppos;
if (copy_from_user(image_ptr, buf, (unsigned long)count))
return -EFAULT;
@@ -394,6 +404,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 +419,6 @@ 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
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] staging: vme_user: validate slave window size against buffer size
2026-05-09 7:53 [PATCH] staging: vme_user: validate slave window size against buffer size Rion Kiguchi
@ 2026-05-09 8:04 ` Greg KH
2026-05-09 9:07 ` [PATCH v3] " Rion Kiguchi
2026-05-09 9:26 ` Rion Kiguchi
0 siblings, 2 replies; 7+ messages in thread
From: Greg KH @ 2026-05-09 8:04 UTC (permalink / raw)
To: Rion Kiguchi; +Cc: stable
On Sat, May 09, 2026 at 04:53:18PM +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() (the
> configured window size, attacker-controlled) but never consult
> size_buf. The slave I/O paths buffer_to_user() and buffer_from_user()
> then index image[minor].kern_buf with *ppos values up to
> image_size - 1, well 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. Also
> add defensive bounds checks against size_buf in buffer_to_user() and
> buffer_from_user() so that the I/O paths cannot exceed the
> allocation even if a future ioctl path forgets to validate.
>
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4-7
> Signed-off-by: Rion Kiguchi <kiguchi.r.sec@gmail.com>
> ---
> drivers/staging/vme_user/vme_user.c | 19 ++++++++++++++++++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/vme_user/vme_user.c b/drivers/staging/vme_user/vme_user.c
> index 11e25c2f6..41b8d5b51 100644
> --- a/drivers/staging/vme_user/vme_user.c
> +++ b/drivers/staging/vme_user/vme_user.c
> @@ -156,6 +156,11 @@ static ssize_t buffer_to_user(unsigned int minor, char __user *buf,
> {
> void *image_ptr;
>
> + if (*ppos < 0 || (u64)*ppos >= image[minor].size_buf ||
> + count > image[minor].size_buf - (u64)*ppos) {
> + pr_warn_ratelimited("%s: out-of-bounds access\n", __func__);
> + return -EINVAL;
> + }
Why doesn't the check in vme_user_read() already catch this? You are
duplicating much of the same logic again, are you _SURE_ the
LLM-generated report here is actually correct?
And don't spam the kernel log for when a user sends invalid data, that
would just be a mess. But if you do want to, use the proper device
information, not just a static function name, which is very generic and
impossible to determine what went wrong (i.e. use the correct logging
functions like dev_err() and the like).
And you need an extra blank line after the check here, your LLM should
know better :)
> image_ptr = image[minor].kern_buf + *ppos;
> if (copy_to_user(buf, image_ptr, (unsigned long)count))
> return -EFAULT;
> @@ -168,6 +173,11 @@ static ssize_t buffer_from_user(unsigned int minor, const char __user *buf,
> {
> void *image_ptr;
>
> + if (*ppos < 0 || (u64)*ppos >= image[minor].size_buf ||
> + count > image[minor].size_buf - (u64)*ppos) {
> + pr_warn_ratelimited("%s: out-of-bounds access\n", __func__);
> + return -EINVAL;
> + }
Same as above.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v3] staging: vme_user: validate slave window size against buffer size
2026-05-09 8:04 ` Greg KH
@ 2026-05-09 9:07 ` Rion Kiguchi
2026-05-09 9:15 ` Greg KH
2026-05-09 9:16 ` Greg KH
2026-05-09 9:26 ` Rion Kiguchi
1 sibling, 2 replies; 7+ messages in thread
From: Rion Kiguchi @ 2026-05-09 9:07 UTC (permalink / raw)
To: kiguchi.r.sec; +Cc: stable
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>
---
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..6fd051f49 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,6 @@ 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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3] staging: vme_user: validate slave window size against buffer size
2026-05-09 9:07 ` [PATCH v3] " Rion Kiguchi
@ 2026-05-09 9:15 ` Greg KH
2026-05-09 9:16 ` Greg KH
1 sibling, 0 replies; 7+ messages in thread
From: Greg KH @ 2026-05-09 9:15 UTC (permalink / raw)
To: Rion Kiguchi; +Cc: stable
On Sat, May 09, 2026 at 06:07:21PM +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>
> ---
> 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..6fd051f49 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,6 @@ 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
>
>
Hi,
This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him
a patch that has triggered this response. He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created. Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.
You are receiving this message because of the following common error(s)
as indicated below:
- This looks like a new version of a previously submitted patch, but you
did not list below the --- line any changes from the previous version.
Please read the section entitled "The canonical patch format" in the
kernel file, Documentation/process/submitting-patches.rst for what
needs to be done here to properly describe this.
If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.
thanks,
greg k-h's patch email bot
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3] staging: vme_user: validate slave window size against buffer size
2026-05-09 9:07 ` [PATCH v3] " Rion Kiguchi
2026-05-09 9:15 ` Greg KH
@ 2026-05-09 9:16 ` Greg KH
1 sibling, 0 replies; 7+ messages in thread
From: Greg KH @ 2026-05-09 9:16 UTC (permalink / raw)
To: Rion Kiguchi; +Cc: stable
On Sat, May 09, 2026 at 06:07:21PM +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.
<snip>
For some reason you are not using scripts/get_maintainer.pl on your
patch to know who to send this to (hint, it's not the stable email
address...)
> @@ -401,7 +409,6 @@ 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;
Why was this change made?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3] staging: vme_user: validate slave window size against buffer size
2026-05-09 8:04 ` Greg KH
2026-05-09 9:07 ` [PATCH v3] " Rion Kiguchi
@ 2026-05-09 9:26 ` Rion Kiguchi
2026-05-09 9:58 ` Greg Kroah-Hartman
1 sibling, 1 reply; 7+ messages in thread
From: Rion Kiguchi @ 2026-05-09 9:26 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-staging, linux-kernel, security, Rion Kiguchi, stable
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3] staging: vme_user: validate slave window size against buffer size
2026-05-09 9:26 ` Rion Kiguchi
@ 2026-05-09 9:58 ` Greg Kroah-Hartman
0 siblings, 0 replies; 7+ messages in thread
From: Greg Kroah-Hartman @ 2026-05-09 9:58 UTC (permalink / raw)
To: Rion Kiguchi; +Cc: linux-staging, linux-kernel, security, stable
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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-05-09 9:58 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-09 7:53 [PATCH] staging: vme_user: validate slave window size against buffer size Rion Kiguchi
2026-05-09 8:04 ` Greg KH
2026-05-09 9:07 ` [PATCH v3] " Rion Kiguchi
2026-05-09 9:15 ` Greg KH
2026-05-09 9:16 ` Greg KH
2026-05-09 9:26 ` Rion Kiguchi
2026-05-09 9:58 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox