diff mbox series

USB: core: prevent malicious bNumInterfaces overflow

Message ID 1532338840-14416-2-git-send-email-paolo.pisati@canonical.com
State New
Headers show
Series USB: core: prevent malicious bNumInterfaces overflow | expand

Commit Message

Paolo Pisati July 23, 2018, 9:40 a.m. UTC
From: Alan Stern <stern@rowland.harvard.edu>

A malicious USB device with crafted descriptors can cause the kernel
to access unallocated memory by setting the bNumInterfaces value too
high in a configuration descriptor.  Although the value is adjusted
during parsing, this adjustment is skipped in one of the error return
paths.

This patch prevents the problem by setting bNumInterfaces to 0
initially.  The existing code already sets it to the proper value
after parsing is complete.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Reported-by: Andrey Konovalov <andreyknvl@google.com>
CC: <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
(cherry picked from commit 48a4ff1c7bb5a32d2e396b03132d20d552c0eca7)
Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com>
---
 drivers/usb/core/config.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Stefan Bader July 23, 2018, 12:45 p.m. UTC | #1
On 23.07.2018 11:40, Paolo Pisati wrote:
> From: Alan Stern <stern@rowland.harvard.edu>
> 
> A malicious USB device with crafted descriptors can cause the kernel
> to access unallocated memory by setting the bNumInterfaces value too
> high in a configuration descriptor.  Although the value is adjusted
> during parsing, this adjustment is skipped in one of the error return
> paths.
> 
> This patch prevents the problem by setting bNumInterfaces to 0
> initially.  The existing code already sets it to the proper value
> after parsing is complete.
> 
> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> CC: <stable@vger.kernel.org>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

CVE-2017-17558

> (cherry picked from commit 48a4ff1c7bb5a32d2e396b03132d20d552c0eca7)
> Signed-off-by: Paolo Pisati <paolo.pisati@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>
> ---

As Sam already pointed out: CVE number missing.

>  drivers/usb/core/config.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
> index 4cf67fc..b58452f 100644
> --- a/drivers/usb/core/config.c
> +++ b/drivers/usb/core/config.c
> @@ -425,6 +425,9 @@ static int usb_parse_configuration(struct usb_device *dev, int cfgidx,
>  	unsigned iad_num = 0;
>  
>  	memcpy(&config->desc, buffer, USB_DT_CONFIG_SIZE);
> +	nintf = nintf_orig = config->desc.bNumInterfaces;
> +	config->desc.bNumInterfaces = 0;	// Adjusted later
> +
>  	if (config->desc.bDescriptorType != USB_DT_CONFIG ||
>  	    config->desc.bLength < USB_DT_CONFIG_SIZE ||
>  	    config->desc.bLength > size) {
> @@ -438,7 +441,6 @@ static int usb_parse_configuration(struct usb_device *dev, int cfgidx,
>  	buffer += config->desc.bLength;
>  	size -= config->desc.bLength;
>  
> -	nintf = nintf_orig = config->desc.bNumInterfaces;
>  	if (nintf > USB_MAXINTERFACES) {
>  		dev_warn(ddev, "config %d has too many interfaces: %d, "
>  		    "using maximum allowed: %d\n",
>
diff mbox series

Patch

diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
index 4cf67fc..b58452f 100644
--- a/drivers/usb/core/config.c
+++ b/drivers/usb/core/config.c
@@ -425,6 +425,9 @@  static int usb_parse_configuration(struct usb_device *dev, int cfgidx,
 	unsigned iad_num = 0;
 
 	memcpy(&config->desc, buffer, USB_DT_CONFIG_SIZE);
+	nintf = nintf_orig = config->desc.bNumInterfaces;
+	config->desc.bNumInterfaces = 0;	// Adjusted later
+
 	if (config->desc.bDescriptorType != USB_DT_CONFIG ||
 	    config->desc.bLength < USB_DT_CONFIG_SIZE ||
 	    config->desc.bLength > size) {
@@ -438,7 +441,6 @@  static int usb_parse_configuration(struct usb_device *dev, int cfgidx,
 	buffer += config->desc.bLength;
 	size -= config->desc.bLength;
 
-	nintf = nintf_orig = config->desc.bNumInterfaces;
 	if (nintf > USB_MAXINTERFACES) {
 		dev_warn(ddev, "config %d has too many interfaces: %d, "
 		    "using maximum allowed: %d\n",