Re: [PATCH] staging: most: video: improve arguments to copy_to_user()

[Greg KH]

On Wed, Aug 13, 2025 at 05:12:48PM +0530, Akhilesh Patil wrote:
> Define cnt constant as unsigned long as expected by copy_to_user()
> to avoid implicit type conversion. Define rem constant as unsigned long
> to compare it with the same type size_t of count variable.
> Use standard helper min() to carry out careful comparison to achive
> same functionality.
> 
> Signed-off-by: Akhilesh Patil <akhilesh@xxxxxxxxxxxxx>
> ---
> This patch is motivated from coccinelle report which suggested to use
> kernel standard helper min(). During build check, I found that
> comparison max()  showing error while comparing variables of
> different types. Hence this patch also fixes that to make comparison of
> save types.
> 
> Compile tested only.
> ---
>  drivers/staging/most/video/video.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/most/video/video.c b/drivers/staging/most/video/video.c
> index 2b3cdb1ce140..4b15c390c32d 100644
> --- a/drivers/staging/most/video/video.c
> +++ b/drivers/staging/most/video/video.c
> @@ -172,8 +172,8 @@ static ssize_t comp_vdev_read(struct file *filp, char __user *buf,
>  
>  	while (count > 0 && data_ready(mdev)) {
>  		struct mbo *const mbo = get_top_mbo(mdev);
> -		int const rem = mbo->processed_length - fh->offs;
> -		int const cnt = rem < count ? rem : count;
> +		unsigned long const rem = mbo->processed_length - fh->offs;

Why is this unsigned long now?  processed_length is a u16 and offs is a
u32.  So really this could have gone negative :(


> +		unsigned long const cnt = min(rem, count);

But count is size_t, so again, why unsigned long?

I understand the goal of using min(), but it feels like you just might
have broken something with the loss of the overflow check :(

thanks,

greg k-h