[PATCH] media: stk1160: fix some bounds checking in stk1160_copy_video()

From: Dan Carpenter
Date: Wed Apr 17 2024 - 13:52:14 EST


The subtract in this condition is reversed. The ->length is the length
of the buffer. The ->bytesused is how many bytes we have copied thus
far. When the condition is reversed that means the result of the
subtraction is always negative but since it's unsigned then the result
is a very high positive value. That means the overflow check is never
true.

Fixes: 9cb2173e6ea8 ("[media] media: Add stk1160 new driver (easycap replacement)")
Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
---
This patch is untested, I just spotted it in review.

When this bug is fixed, the two checks for negative values of "lencopy"
could be removed. I wrote a version of this patch which removed the
checks, but in the end I decided to leave the checks. They're harmless.

drivers/media/usb/stk1160/stk1160-video.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/usb/stk1160/stk1160-video.c b/drivers/media/usb/stk1160/stk1160-video.c
index 366f0e4a5dc0..bfb97ea352e7 100644
--- a/drivers/media/usb/stk1160/stk1160-video.c
+++ b/drivers/media/usb/stk1160/stk1160-video.c
@@ -139,8 +139,8 @@ void stk1160_copy_video(struct stk1160 *dev, u8 *src, int len)
* Check if we have enough space left in the buffer.
* In that case, we force loop exit after copy.
*/
- if (lencopy > buf->bytesused - buf->length) {
- lencopy = buf->bytesused - buf->length;
+ if (lencopy > buf->length - buf->bytesused) {
+ lencopy = buf->length - buf->bytesused;
remain = lencopy;
}

@@ -182,8 +182,8 @@ void stk1160_copy_video(struct stk1160 *dev, u8 *src, int len)
* Check if we have enough space left in the buffer.
* In that case, we force loop exit after copy.
*/
- if (lencopy > buf->bytesused - buf->length) {
- lencopy = buf->bytesused - buf->length;
+ if (lencopy > buf->length - buf->bytesused) {
+ lencopy = buf->length - buf->bytesused;
remain = lencopy;
}

--
2.43.0