[PATCH] staging: vme_user: fix bounds check when vme_get_size() returns zero

[PATCH] staging: vme_user: fix bounds check when vme_get_size() returns zero

[Som Tripathi]

vme_get_size() returns zero on failure, as its kerneldoc in vme.c
states. vme_user_read() and vme_user_write() assign it to a size_t and
check the file position with:

	if ((*ppos < 0) || (*ppos > (image_size - 1)))

When image_size is zero, image_size - 1 wraps to SIZE_MAX. The test is
then never true, so the check does nothing. The following statement,

	count = image_size - *ppos;

wraps the same way whenever *ppos is greater than zero.

This is not an out-of-bounds access. resource_to_user() and
resource_from_user() clamp count to size_buf, buffer_to_user() and
buffer_from_user() clamp it to size_buf - *ppos, and vme_master_read()
and vme_master_write() reject an offset greater than the window
length. What happens instead is that read() and write() operate on a
window whose size the driver failed to read, rather than returning at
the check.

Compare *ppos against image_size directly. The two forms agree for a
non-zero size, the new one is also correct for zero, and both wraps go
away.

Found by reading the code after Dan Carpenter listed this as one of
three outstanding bugs in this driver; see the Link below. Compile
tested only. I have no VME hardware.

Fixes: f00a86d98a1e ("Staging: vme: add VME userspace driver")
Link: https://lore.kernel.org/all/aj0WWwiOzjLGbY5z@stanley.mountain/
Signed-off-by: Som Tripathi <tripathisom142004@gmail.com>
Assisted-by: Claude:claude-opus-5
---
 drivers/staging/vme_user/vme_user.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/vme_user/vme_user.c b/drivers/staging/vme_user/vme_user.c
index a472a38ef..0df30a3c3 100644
--- a/drivers/staging/vme_user/vme_user.c
+++ b/drivers/staging/vme_user/vme_user.c
@@ -213,7 +213,7 @@ static ssize_t vme_user_read(struct file *file, char __user *buf, size_t count,
 	image_size = vme_get_size(image[minor].resource);
 
 	/* Ensure we are starting at a valid location */
-	if ((*ppos < 0) || (*ppos > (image_size - 1))) {
+	if ((*ppos < 0) || (*ppos >= image_size)) {
 		mutex_unlock(&image[minor].mutex);
 		return 0;
 	}
@@ -255,7 +255,7 @@ static ssize_t vme_user_write(struct file *file, const char __user *buf,
 	image_size = vme_get_size(image[minor].resource);
 
 	/* Ensure we are starting at a valid location */
-	if ((*ppos < 0) || (*ppos > (image_size - 1))) {
+	if ((*ppos < 0) || (*ppos >= image_size)) {
 		mutex_unlock(&image[minor].mutex);
 		return 0;
 	}
-- 
2.55.0.windows.1

Re: [PATCH] staging: vme_user: fix bounds check when vme_get_size() returns zero

[Dan Carpenter]

On Thu, Aug 06, 2026 at 08:25:06PM -0500, Som Tripathi wrote:
> vme_get_size() returns zero on failure, as its kerneldoc in vme.c
> states. vme_user_read() and vme_user_write() assign it to a size_t and
> check the file position with:
> 
> 	if ((*ppos < 0) || (*ppos > (image_size - 1)))
> 
> When image_size is zero, image_size - 1 wraps to SIZE_MAX. The test is
> then never true, so the check does nothing. The following statement,
> 
> 	count = image_size - *ppos;
> 
> wraps the same way whenever *ppos is greater than zero.
> 
> This is not an out-of-bounds access. resource_to_user() and
> resource_from_user() clamp count to size_buf, buffer_to_user() and
> buffer_from_user() clamp it to size_buf - *ppos, and vme_master_read()
> and vme_master_write() reject an offset greater than the window
> length. What happens instead is that read() and write() operate on a
> window whose size the driver failed to read, rather than returning at
> the check.
> 
> Compare *ppos against image_size directly. The two forms agree for a
> non-zero size, the new one is also correct for zero, and both wraps go
> away.
> 
> Found by reading the code after Dan Carpenter listed this as one of
> three outstanding bugs in this driver;

There are probably more than three.  :P

> see the Link below. Compile
> tested only. I have no VME hardware.
> 
> Fixes: f00a86d98a1e ("Staging: vme: add VME userspace driver")
> Link: https://lore.kernel.org/all/aj0WWwiOzjLGbY5z@stanley.mountain/
> Signed-off-by: Som Tripathi <tripathisom142004@gmail.com>
> Assisted-by: Claude:claude-opus-5
> ---
>  drivers/staging/vme_user/vme_user.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/vme_user/vme_user.c b/drivers/staging/vme_user/vme_user.c
> index a472a38ef..0df30a3c3 100644
> --- a/drivers/staging/vme_user/vme_user.c
> +++ b/drivers/staging/vme_user/vme_user.c
> @@ -213,7 +213,7 @@ static ssize_t vme_user_read(struct file *file, char __user *buf, size_t count,
>  	image_size = vme_get_size(image[minor].resource);

It would be better to just add a check here.

	if (!image_size)
		return 0;

Same for the other.

regards,
dan carpenter

>  
>  	/* Ensure we are starting at a valid location */
> -	if ((*ppos < 0) || (*ppos > (image_size - 1))) {
> +	if ((*ppos < 0) || (*ppos >= image_size)) {
>  		mutex_unlock(&image[minor].mutex);
>  		return 0;
>  	}

Re: [PATCH] staging: vme_user: fix bounds check when vme_get_size() returns zero

[Greg KH]

On Thu, Aug 06, 2026 at 08:25:06PM -0500, Som Tripathi wrote:
> vme_get_size() returns zero on failure, as its kerneldoc in vme.c
> states. vme_user_read() and vme_user_write() assign it to a size_t and
> check the file position with:
> 
> 	if ((*ppos < 0) || (*ppos > (image_size - 1)))
> 
> When image_size is zero, image_size - 1 wraps to SIZE_MAX. The test is
> then never true, so the check does nothing. The following statement,
> 
> 	count = image_size - *ppos;
> 
> wraps the same way whenever *ppos is greater than zero.
> 
> This is not an out-of-bounds access. resource_to_user() and
> resource_from_user() clamp count to size_buf, buffer_to_user() and
> buffer_from_user() clamp it to size_buf - *ppos, and vme_master_read()
> and vme_master_write() reject an offset greater than the window
> length. What happens instead is that read() and write() operate on a
> window whose size the driver failed to read, rather than returning at
> the check.
> 
> Compare *ppos against image_size directly. The two forms agree for a
> non-zero size, the new one is also correct for zero, and both wraps go
> away.
> 
> Found by reading the code after Dan Carpenter listed this as one of
> three outstanding bugs in this driver; see the Link below. Compile
> tested only. I have no VME hardware.

Please see:
	https://lore.kernel.org/all/2026080354-skater-urgent-31b2@gregkh/T/#u
for why I can't take this.

thanks,

greg k-h