[PATCH] staging: most: video: fix read() length underflow

[PATCH] staging: most: video: fix read() length underflow

[Alexandru Hossu]

Avoid unsigned underflow when fh->offs exceeds mbo->processed_length.
Use size_t for length calculations and clamp invalid offsets.

Signed-off-by: Alexandru Hossu <hossu.alexandru@gmail.com>
---
 drivers/staging/most/video/video.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/most/video/video.c b/drivers/staging/most/video/video.c
index 04351f8ccccf..8c4800be875e 100644
--- a/drivers/staging/most/video/video.c
+++ b/drivers/staging/most/video/video.c
@@ -158,7 +158,7 @@ static ssize_t comp_vdev_read(struct file *filp, char __user *buf,
 {
 	struct comp_fh *fh = to_comp_fh(filp);
 	struct most_video_dev *mdev = fh->mdev;
-	int ret = 0;
+	ssize_t ret = 0;
 
 	if (*pos)
 		return -ESPIPE;
@@ -177,8 +177,19 @@ 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;
+		size_t rem, cnt;
+
+		if (fh->offs >= mbo->processed_length) {
+			fh->offs = 0;
+			spin_lock_irq(&mdev->list_lock);
+			list_del(&mbo->list);
+			spin_unlock_irq(&mdev->list_lock);
+			most_put_mbo(mbo);
+			continue;
+		}
+
+		rem = mbo->processed_length - fh->offs;
+		cnt = min_t(size_t, rem, count);
 
 		if (copy_to_user(buf, mbo->virt_address + fh->offs, cnt)) {
 			v4l2_err(&mdev->v4l2_dev, "read: copy_to_user failed\n");
-- 
2.43.0

Re: [PATCH] staging: most: video: fix read() length underflow

[Dan Carpenter]

On Thu, Mar 05, 2026 at 02:57:03AM +0100, Alexandru Hossu wrote:
> Avoid unsigned underflow when fh->offs exceeds mbo->processed_length.
> Use size_t for length calculations and clamp invalid offsets.
> 
> Signed-off-by: Alexandru Hossu <hossu.alexandru@gmail.com>
> ---

No, this patch isn't correct or required.  Look at how fh->offs is set.
It can't be more than processed_length.  I was worried there might be a
race condition but that is prevented by the:

	if (!atomic_inc_and_test(&mdev->access_ref)) {

which prevents multiple concurrent readers.

The other thing is that "count" can't be more than MAX_RW_COUNT so
ret is fine as an int.  (Also it can't be more than
processed_length which is at most U16_MAX.)

With this kind of change I would want the commit message to have
an explanation of all the variables and the list of functions
where they are set.  That shows you have done the analysis and it
speeds up my analysis as well as a reviewer.

regards,
dan carpenter

Re: [PATCH] staging: most: video: fix read() length underflow

[Alexandru Hossu]

Hi Dan,

Thanks for the detailed review.

You are right: given how fh->offs is set and the single-reader guard via
atomic_inc_and_test(&mdev->access_ref), fh->offs should not exceed
mbo->processed_length, so my underflow concern is unfounded. I will drop
this patch.

I also appreciate the guidance about documenting variable invariants and
where they are set. I will include that analysis up front for similar
changes in the future.

Regards,
Alexandru