lkml.org 
[lkml]   [2018]   [Dec]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v2] media: uvcvideo: Fix 'type' check leading to overflow
Date
Hi Alistair,

Thank you for the patch.

On Wednesday, 19 December 2018 03:32:48 EET Alistair Strachan wrote:
> From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Are you sure you don't want to keep authorship ? I've merely reviewed v1 and
proposed an alternative implementation :-) Let me know what you would prefer
and I'll apply this to my tree.

> When initially testing the Camera Terminal Descriptor wTerminalType
> field (buffer[4]), no mask is used. Later in the function, the MSB is
> overloaded to store the descriptor subtype, and so a mask of 0x7fff
> is used to check the type.
>
> If a descriptor is specially crafted to set this overloaded bit in the
> original wTerminalType field, the initial type check will fail (falling
> through, without adjusting the buffer size), but the later type checks
> will pass, assuming the buffer has been made suitably large, causing an
> overflow.
>
> Avoid this problem by checking for the MSB in the wTerminalType field.
> If the bit is set, assume the descriptor is bad, and abort parsing it.
>
> Originally reported here:
> https://groups.google.com/forum/#!topic/syzkaller/Ot1fOE6v1d8
> A similar (non-compiling) patch was provided at that time.
>
> Reported-by: syzbot <syzkaller@googlegroups.com>
> Signed-off-by: Alistair Strachan <astrachan@google.com>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
> Cc: linux-media@vger.kernel.org
> Cc: kernel-team@android.com
> ---
> v2: Use an alternative fix suggested by Laurent
> drivers/media/usb/uvc/uvc_driver.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_driver.c
> b/drivers/media/usb/uvc/uvc_driver.c index bc369a0934a3..7fde3ce642c4
> 100644
> --- a/drivers/media/usb/uvc/uvc_driver.c
> +++ b/drivers/media/usb/uvc/uvc_driver.c
> @@ -1065,11 +1065,19 @@ static int uvc_parse_standard_control(struct
> uvc_device *dev, return -EINVAL;
> }
>
> - /* Make sure the terminal type MSB is not null, otherwise it
> - * could be confused with a unit.
> + /*
> + * Reject invalid terminal types that would cause issues:
> + *
> + * - The high byte must be non-zero, otherwise it would be
> + * confused with a unit.
> + *
> + * - Bit 15 must be 0, as we use it internally as a terminal
> + * direction flag.
> + *
> + * Other unknown types are accepted.
> */
> type = get_unaligned_le16(&buffer[4]);
> - if ((type & 0xff00) == 0) {
> + if ((type & 0x7f00) == 0 || (type & 0x8000) != 0) {
> uvc_trace(UVC_TRACE_DESCR, "device %d videocontrol "
> "interface %d INPUT_TERMINAL %d has invalid "
> "type 0x%04x, skipping\n", udev->devnum,

--
Regards,

Laurent Pinchart



\
 
 \ /
  Last update: 2018-12-19 09:17    [W:0.422 / U:0.192 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site