Commit faa4364b authored by Dan Carpenter's avatar Dan Carpenter Committed by Hans Verkuil

media: stk1160: fix bounds checking in stk1160_copy_video()

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.

Additionally, the ->bytesused doesn't actually work for this purpose
because we're not writing to "buf->mem + buf->bytesused".  Instead, the
math to calculate the destination where we are writing is a bit
involved.  You calculate the number of full lines already written,
multiply by two, skip a line if necessary so that we start on an odd
numbered line, and add the offset into the line.

To fix this buffer overflow, just take the actual destination where we
are writing, if the offset is already out of bounds print an error and
return.  Otherwise, write up to buf->length bytes.

Fixes: 9cb2173e ("[media] media: Add stk1160 new driver (easycap replacement)")
Signed-off-by: default avatarDan Carpenter <dan.carpenter@linaro.org>
Reviewed-by: default avatarRicardo Ribalda <ribalda@chromium.org>
Signed-off-by: default avatarHans Verkuil <hverkuil-cisco@xs4all.nl>
parent e32b4567
......@@ -99,7 +99,7 @@ void stk1160_buffer_done(struct stk1160 *dev)
static inline
void stk1160_copy_video(struct stk1160 *dev, u8 *src, int len)
{
int linesdone, lineoff, lencopy;
int linesdone, lineoff, lencopy, offset;
int bytesperline = dev->width * 2;
struct stk1160_buffer *buf = dev->isoc_ctl.buf;
u8 *dst = buf->mem;
......@@ -139,8 +139,13 @@ 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;
offset = dst - (u8 *)buf->mem;
if (offset > buf->length) {
dev_warn_ratelimited(dev->dev, "out of bounds offset\n");
return;
}
if (lencopy > buf->length - offset) {
lencopy = buf->length - offset;
remain = lencopy;
}
......@@ -182,8 +187,13 @@ 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;
offset = dst - (u8 *)buf->mem;
if (offset > buf->length) {
dev_warn_ratelimited(dev->dev, "offset out of bounds\n");
return;
}
if (lencopy > buf->length - offset) {
lencopy = buf->length - offset;
remain = lencopy;
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment