| Message ID | 1563442027-12388-2-git-send-email-paolo.pisati@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | CVE-2019-2101: USB Video Class info | expand |
On 18.07.19 11:27, Paolo Pisati wrote: > From: Alistair Strachan <astrachan@google.com> > > 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> > > CVE-2019-2101 > > (cherry picked from commit 47bb117911b051bbc90764a8bff96543cbd2005f) > Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com> Acked-by: Stefan Bader <stefan.bader@canonical.com> > --- > 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 28b91b7d756f..1778fdeded94 100644 > --- a/drivers/media/usb/uvc/uvc_driver.c > +++ b/drivers/media/usb/uvc/uvc_driver.c > @@ -1054,11 +1054,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, >
On 7/18/19 11:27 AM, Paolo Pisati wrote: > From: Alistair Strachan <astrachan@google.com> > > 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> > > CVE-2019-2101 > > (cherry picked from commit 47bb117911b051bbc90764a8bff96543cbd2005f) > Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com> Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> > --- > 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 28b91b7d756f..1778fdeded94 100644 > --- a/drivers/media/usb/uvc/uvc_driver.c > +++ b/drivers/media/usb/uvc/uvc_driver.c > @@ -1054,11 +1054,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, >
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index 28b91b7d756f..1778fdeded94 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -1054,11 +1054,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,