| From 47bb117911b051bbc90764a8bff96543cbd2005f Mon Sep 17 00:00:00 2001 |
| From: Alistair Strachan <astrachan@google.com> |
| Date: Tue, 18 Dec 2018 20:32:48 -0500 |
| Subject: media: uvcvideo: Fix 'type' check leading to overflow |
| |
| From: Alistair Strachan <astrachan@google.com> |
| |
| commit 47bb117911b051bbc90764a8bff96543cbd2005f upstream. |
| |
| 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> |
| Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> |
| Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| drivers/media/usb/uvc/uvc_driver.c | 14 +++++++++++--- |
| 1 file changed, 11 insertions(+), 3 deletions(-) |
| |
| --- 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(st |
| 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, |