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

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/[email protected]/
> Signed-off-by: Som Tripathi <[email protected]>
> 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;
>  	}