* [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data
@ 2018-12-09 16:32 ` Greg Kroah-Hartman
0 siblings, 0 replies; 28+ messages in thread
From: Greg KH @ 2018-12-09 16:32 UTC (permalink / raw)
To: David S. Miller, netdev
Cc: linux-usb, Sebastian Andrzej Siewior, Hui Peng, Mathias Payer
From: Hui Peng <benquike@gmail.com>
The function hso_probe reads if_num from the USB device (as an u8) and uses
it without a length check to index an array, resulting in an OOB memory read
in hso_probe or hso_get_config_data. Added a length check for both locations
and updated hso_probe to bail on error.
Reported-by: Hui Peng <benquike@gmail.com>
Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
Signed-off-by: Hui Peng <benquike@gmail.com>
Signed-off-by: Mathias Payer <mathias.payer@nebelwelt.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/net/usb/hso.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 184c24baca15..168f9081d4ea 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2807,6 +2807,12 @@ static int hso_get_config_data(struct usb_interface *interface)
return -EIO;
}
+ /* check if we have a valid interface */
+ if (if_num > 16) {
+ kfree(config_data);
+ return -EINVAL;
+ }
+
switch (config_data[if_num]) {
case 0x0:
result = 0;
@@ -2877,10 +2883,18 @@ static int hso_probe(struct usb_interface *interface,
/* Get the interface/port specification from either driver_info or from
* the device itself */
- if (id->driver_info)
+ if (id->driver_info) {
+ /* if_num is controlled by the device, driver_info is a 0 terminated
+ * array. Make sure, the access is in bounds! */
+ for (i = 0; i <= if_num; ++i)
+ if (((u32 *)(id->driver_info))[i] == 0)
+ goto exit;
port_spec = ((u32 *)(id->driver_info))[if_num];
- else
+ } else {
port_spec = hso_get_config_data(interface);
+ if (IS_ERR_VALUE((long)port_spec))
+ goto exit;
+ }
/* Check if we need to switch to alt interfaces prior to port
* configuration */
--
2.19.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data
@ 2018-12-09 16:32 ` Greg Kroah-Hartman
0 siblings, 0 replies; 28+ messages in thread
From: Greg Kroah-Hartman @ 2018-12-09 16:32 UTC (permalink / raw)
To: David S. Miller, netdev
Cc: linux-usb, Sebastian Andrzej Siewior, Hui Peng, Mathias Payer
From: Hui Peng <benquike@gmail.com>
The function hso_probe reads if_num from the USB device (as an u8) and uses
it without a length check to index an array, resulting in an OOB memory read
in hso_probe or hso_get_config_data. Added a length check for both locations
and updated hso_probe to bail on error.
Reported-by: Hui Peng <benquike@gmail.com>
Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
Signed-off-by: Hui Peng <benquike@gmail.com>
Signed-off-by: Mathias Payer <mathias.payer@nebelwelt.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/net/usb/hso.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 184c24baca15..168f9081d4ea 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2807,6 +2807,12 @@ static int hso_get_config_data(struct usb_interface *interface)
return -EIO;
}
+ /* check if we have a valid interface */
+ if (if_num > 16) {
+ kfree(config_data);
+ return -EINVAL;
+ }
+
switch (config_data[if_num]) {
case 0x0:
result = 0;
@@ -2877,10 +2883,18 @@ static int hso_probe(struct usb_interface *interface,
/* Get the interface/port specification from either driver_info or from
* the device itself */
- if (id->driver_info)
+ if (id->driver_info) {
+ /* if_num is controlled by the device, driver_info is a 0 terminated
+ * array. Make sure, the access is in bounds! */
+ for (i = 0; i <= if_num; ++i)
+ if (((u32 *)(id->driver_info))[i] == 0)
+ goto exit;
port_spec = ((u32 *)(id->driver_info))[if_num];
- else
+ } else {
port_spec = hso_get_config_data(interface);
+ if (IS_ERR_VALUE((long)port_spec))
+ goto exit;
+ }
/* Check if we need to switch to alt interfaces prior to port
* configuration */
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data
@ 2018-12-09 19:54 ` David Miller
0 siblings, 0 replies; 28+ messages in thread
From: David Miller @ 2018-12-09 19:54 UTC (permalink / raw)
To: gregkh; +Cc: netdev, linux-usb, bigeasy, benquike, mathias.payer
From: Greg KH <gregkh@linuxfoundation.org>
Date: Sun, 9 Dec 2018 17:32:45 +0100
> + } else {
> port_spec = hso_get_config_data(interface);
> + if (IS_ERR_VALUE((long)port_spec))
> + goto exit;
'port_spec' is an 'int', it makes no sense to cast it 3 times all the
way back to 'int' to figure out if it is a negative error value or
not. (--> long --> void * --> int)
^ permalink raw reply [flat|nested] 28+ messages in thread
* USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data
@ 2018-12-09 19:54 ` David Miller
0 siblings, 0 replies; 28+ messages in thread
From: David Miller @ 2018-12-09 19:54 UTC (permalink / raw)
To: gregkh; +Cc: netdev, linux-usb, bigeasy, benquike, mathias.payer
From: Greg KH <gregkh@linuxfoundation.org>
Date: Sun, 9 Dec 2018 17:32:45 +0100
> + } else {
> port_spec = hso_get_config_data(interface);
> + if (IS_ERR_VALUE((long)port_spec))
> + goto exit;
'port_spec' is an 'int', it makes no sense to cast it 3 times all the
way back to 'int' to figure out if it is a negative error value or
not. (--> long --> void * --> int)
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data
@ 2018-12-09 20:02 ` Mathias Payer
0 siblings, 0 replies; 28+ messages in thread
From: Mathias Payer @ 2018-12-09 20:02 UTC (permalink / raw)
To: David Miller, gregkh; +Cc: netdev, linux-usb, bigeasy, benquike
[-- Attachment #1.1: Type: text/plain, Size: 1032 bytes --]
Hi David,
>> + } else {
>> port_spec = hso_get_config_data(interface);
>> + if (IS_ERR_VALUE((long)port_spec))
>> + goto exit;
>
> 'port_spec' is an 'int', it makes no sense to cast it 3 times all the
> way back to 'int' to figure out if it is a negative error value or
> not. (--> long --> void * --> int)
>
Passing an int to the macro results in a compiler warning. One option would be
to test for the individual errors (instead of using the macro) with the drawback
that future extensions that return different errors may be missed. Another
alternative is to test for negative values with the drawback that this bypasses
the existing test macros.
Also, the macro does not cast back to int but unsigned long:
https://elixir.bootlin.com/linux/v4.20-rc5/source/include/linux/err.h#L22
#define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= (unsigned
long)-MAX_ERRNO)
The cast chain is int -> long -> void * -> unsigned long.
What other check would you propose?
Thanks,
Mathias
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data
@ 2018-12-09 20:02 ` Mathias Payer
0 siblings, 0 replies; 28+ messages in thread
From: Mathias Payer @ 2018-12-09 20:02 UTC (permalink / raw)
To: David Miller, gregkh; +Cc: netdev, linux-usb, bigeasy, benquike
Hi David,
>> + } else {
>> port_spec = hso_get_config_data(interface);
>> + if (IS_ERR_VALUE((long)port_spec))
>> + goto exit;
>
> 'port_spec' is an 'int', it makes no sense to cast it 3 times all the
> way back to 'int' to figure out if it is a negative error value or
> not. (--> long --> void * --> int)
>
Passing an int to the macro results in a compiler warning. One option would be
to test for the individual errors (instead of using the macro) with the drawback
that future extensions that return different errors may be missed. Another
alternative is to test for negative values with the drawback that this bypasses
the existing test macros.
Also, the macro does not cast back to int but unsigned long:
https://elixir.bootlin.com/linux/v4.20-rc5/source/include/linux/err.h#L22
#define IS_ERR_VALUE(x) unlikely((unsigned long)(void *)(x) >= (unsigned
long)-MAX_ERRNO)
The cast chain is int -> long -> void * -> unsigned long.
What other check would you propose?
Thanks,
Mathias
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data
@ 2018-12-09 20:04 ` Mathias Payer
0 siblings, 0 replies; 28+ messages in thread
From: Mathias Payer @ 2018-12-09 20:04 UTC (permalink / raw)
To: Greg KH, David S. Miller, netdev
Cc: linux-usb, Sebastian Andrzej Siewior, Hui Peng
[-- Attachment #1.1: Type: text/plain, Size: 2096 bytes --]
FYI, this issue has been assigned CVE-2018-19985.
Thanks,
Mathias
On 12/9/18 5:32 PM, Greg KH wrote:
> From: Hui Peng <benquike@gmail.com>
>
> The function hso_probe reads if_num from the USB device (as an u8) and uses
> it without a length check to index an array, resulting in an OOB memory read
> in hso_probe or hso_get_config_data. Added a length check for both locations
> and updated hso_probe to bail on error.
>
> Reported-by: Hui Peng <benquike@gmail.com>
> Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
> Signed-off-by: Hui Peng <benquike@gmail.com>
> Signed-off-by: Mathias Payer <mathias.payer@nebelwelt.net>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> drivers/net/usb/hso.c | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
> index 184c24baca15..168f9081d4ea 100644
> --- a/drivers/net/usb/hso.c
> +++ b/drivers/net/usb/hso.c
> @@ -2807,6 +2807,12 @@ static int hso_get_config_data(struct usb_interface *interface)
> return -EIO;
> }
>
> + /* check if we have a valid interface */
> + if (if_num > 16) {
> + kfree(config_data);
> + return -EINVAL;
> + }
> +
> switch (config_data[if_num]) {
> case 0x0:
> result = 0;
> @@ -2877,10 +2883,18 @@ static int hso_probe(struct usb_interface *interface,
>
> /* Get the interface/port specification from either driver_info or from
> * the device itself */
> - if (id->driver_info)
> + if (id->driver_info) {
> + /* if_num is controlled by the device, driver_info is a 0 terminated
> + * array. Make sure, the access is in bounds! */
> + for (i = 0; i <= if_num; ++i)
> + if (((u32 *)(id->driver_info))[i] == 0)
> + goto exit;
> port_spec = ((u32 *)(id->driver_info))[if_num];
> - else
> + } else {
> port_spec = hso_get_config_data(interface);
> + if (IS_ERR_VALUE((long)port_spec))
> + goto exit;
> + }
>
> /* Check if we need to switch to alt interfaces prior to port
> * configuration */
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data
@ 2018-12-09 20:04 ` Mathias Payer
0 siblings, 0 replies; 28+ messages in thread
From: Mathias Payer @ 2018-12-09 20:04 UTC (permalink / raw)
To: Greg KH, David S. Miller, netdev
Cc: linux-usb, Sebastian Andrzej Siewior, Hui Peng
FYI, this issue has been assigned CVE-2018-19985.
Thanks,
Mathias
On 12/9/18 5:32 PM, Greg KH wrote:
> From: Hui Peng <benquike@gmail.com>
>
> The function hso_probe reads if_num from the USB device (as an u8) and uses
> it without a length check to index an array, resulting in an OOB memory read
> in hso_probe or hso_get_config_data. Added a length check for both locations
> and updated hso_probe to bail on error.
>
> Reported-by: Hui Peng <benquike@gmail.com>
> Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
> Signed-off-by: Hui Peng <benquike@gmail.com>
> Signed-off-by: Mathias Payer <mathias.payer@nebelwelt.net>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> drivers/net/usb/hso.c | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
> index 184c24baca15..168f9081d4ea 100644
> --- a/drivers/net/usb/hso.c
> +++ b/drivers/net/usb/hso.c
> @@ -2807,6 +2807,12 @@ static int hso_get_config_data(struct usb_interface *interface)
> return -EIO;
> }
>
> + /* check if we have a valid interface */
> + if (if_num > 16) {
> + kfree(config_data);
> + return -EINVAL;
> + }
> +
> switch (config_data[if_num]) {
> case 0x0:
> result = 0;
> @@ -2877,10 +2883,18 @@ static int hso_probe(struct usb_interface *interface,
>
> /* Get the interface/port specification from either driver_info or from
> * the device itself */
> - if (id->driver_info)
> + if (id->driver_info) {
> + /* if_num is controlled by the device, driver_info is a 0 terminated
> + * array. Make sure, the access is in bounds! */
> + for (i = 0; i <= if_num; ++i)
> + if (((u32 *)(id->driver_info))[i] == 0)
> + goto exit;
> port_spec = ((u32 *)(id->driver_info))[if_num];
> - else
> + } else {
> port_spec = hso_get_config_data(interface);
> + if (IS_ERR_VALUE((long)port_spec))
> + goto exit;
> + }
>
> /* Check if we need to switch to alt interfaces prior to port
> * configuration */
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data,Re: [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data
@ 2018-12-09 20:06 ` David Miller
0 siblings, 0 replies; 28+ messages in thread
From: David Miller @ 2018-12-09 20:06 UTC (permalink / raw)
To: mathias.payer; +Cc: gregkh, netdev, linux-usb, bigeasy, benquike
From: Mathias Payer <mathias.payer@nebelwelt.net>
Date: Sun, 9 Dec 2018 21:02:25 +0100
> Passing an int to the macro results in a compiler warning. One option would be
> to test for the individual errors (instead of using the macro) with the drawback
> that future extensions that return different errors may be missed. Another
> alternative is to test for negative values with the drawback that this bypasses
> the existing test macros.
The whole kernel is full of situations where an int is returned and if it's
negative it's an error. Why is this location so different?
Just check < 0 and be done with it.
^ permalink raw reply [flat|nested] 28+ messages in thread
* USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data
@ 2018-12-09 20:06 ` David Miller
0 siblings, 0 replies; 28+ messages in thread
From: David Miller @ 2018-12-09 20:06 UTC (permalink / raw)
To: mathias.payer; +Cc: gregkh, netdev, linux-usb, bigeasy, benquike
From: Mathias Payer <mathias.payer@nebelwelt.net>
Date: Sun, 9 Dec 2018 21:02:25 +0100
> Passing an int to the macro results in a compiler warning. One option would be
> to test for the individual errors (instead of using the macro) with the drawback
> that future extensions that return different errors may be missed. Another
> alternative is to test for negative values with the drawback that this bypasses
> the existing test macros.
The whole kernel is full of situations where an int is returned and if it's
negative it's an error. Why is this location so different?
Just check < 0 and be done with it.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data,Re: [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data
@ 2018-12-09 20:17 ` Mathias Payer
0 siblings, 0 replies; 28+ messages in thread
From: Mathias Payer @ 2018-12-09 20:17 UTC (permalink / raw)
To: David Miller; +Cc: gregkh, netdev, linux-usb, bigeasy, benquike
[-- Attachment #1.1.1: Type: text/plain, Size: 306 bytes --]
> The whole kernel is full of situations where an int is returned and if it's
> negative it's an error. Why is this location so different?
>
> Just check < 0 and be done with it.
OK, whatever you prefer.
I've attached the updated patch. (Greg, please add your Signed-off-by).
Best,
Mathias
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.1.2: bug3-usb-hso.patch --]
[-- Type: text/x-patch; name="bug3-usb-hso.patch", Size: 2017 bytes --]
commit 78e4a03cd3ae7c8f5957a170bc054fd89d7600f1
Author: Mathias Payer <mathias.payer@nebelwelt.net>
Date: Sun Dec 9 16:45:09 2018 +0100
NET: hso: Fix OOB memory access in hso_probe/hso_get_config_data in hso.c
From: Hui Peng <benquike@gmail.com>
The function hso_probe reads if_num from the USB device (as an u8) and uses
it without a length check to index an array, resulting in an OOB memory read
in hso_probe or hso_get_config_data. Added a length check for both locations
and updated hso_probe to bail on error.
This issue has been assigned CVE-2018-19985.
Reported-by: Hui Peng <benquike@gmail.com>
Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
Signed-off-by: Hui Peng <benquike@gmail.com>
Signed-off-by: Mathias Payer <mathias.payer@nebelwelt.net>
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 184c24baca15..d6916f787fce 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2807,6 +2807,12 @@ static int hso_get_config_data(struct usb_interface *interface)
return -EIO;
}
+ /* check if we have a valid interface */
+ if (if_num > 16) {
+ kfree(config_data);
+ return -EINVAL;
+ }
+
switch (config_data[if_num]) {
case 0x0:
result = 0;
@@ -2877,10 +2883,18 @@ static int hso_probe(struct usb_interface *interface,
/* Get the interface/port specification from either driver_info or from
* the device itself */
- if (id->driver_info)
+ if (id->driver_info) {
+ /* if_num is controlled by the device, driver_info is a 0 terminated
+ * array. Make sure, the access is in bounds! */
+ for (i = 0; i <= if_num; ++i)
+ if (((u32 *)(id->driver_info))[i] == 0)
+ goto exit;
port_spec = ((u32 *)(id->driver_info))[if_num];
- else
+ } else {
port_spec = hso_get_config_data(interface);
+ if (port_spec < 0)
+ goto exit;
+ }
/* Check if we need to switch to alt interfaces prior to port
* configuration */
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 28+ messages in thread
* USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data,Re: [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data
@ 2018-12-09 20:17 ` Mathias Payer
0 siblings, 0 replies; 28+ messages in thread
From: Mathias Payer @ 2018-12-09 20:17 UTC (permalink / raw)
To: David Miller; +Cc: gregkh, netdev, linux-usb, bigeasy, benquike
> The whole kernel is full of situations where an int is returned and if it's
> negative it's an error. Why is this location so different?
>
> Just check < 0 and be done with it.
OK, whatever you prefer.
I've attached the updated patch. (Greg, please add your Signed-off-by).
Best,
Mathias
commit 78e4a03cd3ae7c8f5957a170bc054fd89d7600f1
Author: Mathias Payer <mathias.payer@nebelwelt.net>
Date: Sun Dec 9 16:45:09 2018 +0100
NET: hso: Fix OOB memory access in hso_probe/hso_get_config_data in hso.c
From: Hui Peng <benquike@gmail.com>
The function hso_probe reads if_num from the USB device (as an u8) and uses
it without a length check to index an array, resulting in an OOB memory read
in hso_probe or hso_get_config_data. Added a length check for both locations
and updated hso_probe to bail on error.
This issue has been assigned CVE-2018-19985.
Reported-by: Hui Peng <benquike@gmail.com>
Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
Signed-off-by: Hui Peng <benquike@gmail.com>
Signed-off-by: Mathias Payer <mathias.payer@nebelwelt.net>
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 184c24baca15..d6916f787fce 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2807,6 +2807,12 @@ static int hso_get_config_data(struct usb_interface *interface)
return -EIO;
}
+ /* check if we have a valid interface */
+ if (if_num > 16) {
+ kfree(config_data);
+ return -EINVAL;
+ }
+
switch (config_data[if_num]) {
case 0x0:
result = 0;
@@ -2877,10 +2883,18 @@ static int hso_probe(struct usb_interface *interface,
/* Get the interface/port specification from either driver_info or from
* the device itself */
- if (id->driver_info)
+ if (id->driver_info) {
+ /* if_num is controlled by the device, driver_info is a 0 terminated
+ * array. Make sure, the access is in bounds! */
+ for (i = 0; i <= if_num; ++i)
+ if (((u32 *)(id->driver_info))[i] == 0)
+ goto exit;
port_spec = ((u32 *)(id->driver_info))[if_num];
- else
+ } else {
port_spec = hso_get_config_data(interface);
+ if (port_spec < 0)
+ goto exit;
+ }
/* Check if we need to switch to alt interfaces prior to port
* configuration */
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data,Re: [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data,Re: [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data,Re: [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data
@ 2018-12-09 20:40 ` David Miller
0 siblings, 0 replies; 28+ messages in thread
From: David Miller @ 2018-12-09 20:40 UTC (permalink / raw)
To: mathias.payer; +Cc: gregkh, netdev, linux-usb, bigeasy, benquike
From: Mathias Payer <mathias.payer@nebelwelt.net>
Date: Sun, 9 Dec 2018 21:17:58 +0100
> I've attached the updated patch. (Greg, please add your Signed-off-by).
Patches should be posted inline, not as attachments as per
process/submitting-patches.rst
^ permalink raw reply [flat|nested] 28+ messages in thread
* USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data,Re: [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data
@ 2018-12-09 20:40 ` David Miller
0 siblings, 0 replies; 28+ messages in thread
From: David Miller @ 2018-12-09 20:40 UTC (permalink / raw)
To: mathias.payer; +Cc: gregkh, netdev, linux-usb, bigeasy, benquike
From: Mathias Payer <mathias.payer@nebelwelt.net>
Date: Sun, 9 Dec 2018 21:17:58 +0100
> I've attached the updated patch. (Greg, please add your Signed-off-by).
Patches should be posted inline, not as attachments as per
process/submitting-patches.rst
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data,Re: [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data,Re: [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data,Re: [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data
@ 2018-12-09 20:59 ` Mathias Payer
0 siblings, 0 replies; 28+ messages in thread
From: Mathias Payer @ 2018-12-09 20:59 UTC (permalink / raw)
To: David Miller; +Cc: gregkh, netdev, linux-usb, bigeasy, benquike
[-- Attachment #1.1: Type: text/plain, Size: 2455 bytes --]
On 12/9/18 9:40 PM, David Miller wrote:
> From: Mathias Payer <mathias.payer@nebelwelt.net>
> Date: Sun, 9 Dec 2018 21:17:58 +0100
>
>> I've attached the updated patch. (Greg, please add your Signed-off-by).
>
> Patches should be posted inline, not as attachments as per
> process/submitting-patches.rst
Sorry! And thanks for your patience, we're going through a learning experience.
Inlined updated patch with the discussed changes:
commit 78e4a03cd3ae7c8f5957a170bc054fd89d7600f1
Author: Mathias Payer <mathias.payer@nebelwelt.net>
Date: Sun Dec 9 16:45:09 2018 +0100
NET: hso: Fix OOB memory access in hso_probe/hso_get_config_data in hso.c
From: Hui Peng <benquike@gmail.com>
The function hso_probe reads if_num from the USB device (as an u8) and uses
it without a length check to index an array, resulting in an OOB memory read
in hso_probe or hso_get_config_data. Added a length check for both locations
and updated hso_probe to bail on error.
This issue has been assigned CVE-2018-19985.
Reported-by: Hui Peng <benquike@gmail.com>
Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
Signed-off-by: Hui Peng <benquike@gmail.com>
Signed-off-by: Mathias Payer <mathias.payer@nebelwelt.net>
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 184c24baca15..d6916f787fce 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2807,6 +2807,12 @@ static int hso_get_config_data(struct usb_interface
*interface)
return -EIO;
}
+ /* check if we have a valid interface */
+ if (if_num > 16) {
+ kfree(config_data);
+ return -EINVAL;
+ }
+
switch (config_data[if_num]) {
case 0x0:
result = 0;
@@ -2877,10 +2883,18 @@ static int hso_probe(struct usb_interface *interface,
/* Get the interface/port specification from either driver_info or from
* the device itself */
- if (id->driver_info)
+ if (id->driver_info) {
+ /* if_num is controlled by the device, driver_info is a 0 terminated
+ * array. Make sure, the access is in bounds! */
+ for (i = 0; i <= if_num; ++i)
+ if (((u32 *)(id->driver_info))[i] == 0)
+ goto exit;
port_spec = ((u32 *)(id->driver_info))[if_num];
- else
+ } else {
port_spec = hso_get_config_data(interface);
+ if (port_spec < 0)
+ goto exit;
+ }
/* Check if we need to switch to alt interfaces prior to port
* configuration */
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related [flat|nested] 28+ messages in thread
* USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data,Re: [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data,Re: [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data,Re: [PATCH] USB: hso: Fix OOB memory
@ 2018-12-09 20:59 ` Mathias Payer
0 siblings, 0 replies; 28+ messages in thread
From: Mathias Payer @ 2018-12-09 20:59 UTC (permalink / raw)
To: David Miller; +Cc: gregkh, netdev, linux-usb, bigeasy, benquike
On 12/9/18 9:40 PM, David Miller wrote:
> From: Mathias Payer <mathias.payer@nebelwelt.net>
> Date: Sun, 9 Dec 2018 21:17:58 +0100
>
>> I've attached the updated patch. (Greg, please add your Signed-off-by).
>
> Patches should be posted inline, not as attachments as per
> process/submitting-patches.rst
Sorry! And thanks for your patience, we're going through a learning experience.
Inlined updated patch with the discussed changes:
commit 78e4a03cd3ae7c8f5957a170bc054fd89d7600f1
Author: Mathias Payer <mathias.payer@nebelwelt.net>
Date: Sun Dec 9 16:45:09 2018 +0100
NET: hso: Fix OOB memory access in hso_probe/hso_get_config_data in hso.c
From: Hui Peng <benquike@gmail.com>
The function hso_probe reads if_num from the USB device (as an u8) and uses
it without a length check to index an array, resulting in an OOB memory read
in hso_probe or hso_get_config_data. Added a length check for both locations
and updated hso_probe to bail on error.
This issue has been assigned CVE-2018-19985.
Reported-by: Hui Peng <benquike@gmail.com>
Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
Signed-off-by: Hui Peng <benquike@gmail.com>
Signed-off-by: Mathias Payer <mathias.payer@nebelwelt.net>
result = 0;
@@ -2877,10 +2883,18 @@ static int hso_probe(struct usb_interface *interface,
/* Get the interface/port specification from either driver_info or from
* the device itself */
- if (id->driver_info)
+ if (id->driver_info) {
+ /* if_num is controlled by the device, driver_info is a 0 terminated
+ * array. Make sure, the access is in bounds! */
+ for (i = 0; i <= if_num; ++i)
+ if (((u32 *)(id->driver_info))[i] == 0)
+ goto exit;
port_spec = ((u32 *)(id->driver_info))[if_num];
- else
+ } else {
port_spec = hso_get_config_data(interface);
+ if (port_spec < 0)
+ goto exit;
+ }
/* Check if we need to switch to alt interfaces prior to port
* configuration */
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 184c24baca15..d6916f787fce 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2807,6 +2807,12 @@ static int hso_get_config_data(struct usb_interface
*interface)
return -EIO;
}
+ /* check if we have a valid interface */
+ if (if_num > 16) {
+ kfree(config_data);
+ return -EINVAL;
+ }
+
switch (config_data[if_num]) {
case 0x0:
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data,Re: [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data
@ 2018-12-10 7:18 ` Greg Kroah-Hartman
0 siblings, 0 replies; 28+ messages in thread
From: Greg KH @ 2018-12-10 7:18 UTC (permalink / raw)
To: Mathias Payer; +Cc: David Miller, netdev, linux-usb, bigeasy, benquike
On Sun, Dec 09, 2018 at 09:17:58PM +0100, Mathias Payer wrote:
> > The whole kernel is full of situations where an int is returned and if it's
> > negative it's an error. Why is this location so different?
> >
> > Just check < 0 and be done with it.
>
> OK, whatever you prefer.
My fault, I was tired and suggested the other way...
> I've attached the updated patch. (Greg, please add your Signed-off-by).
Patches can't be attached, I'll fix this up and resend, thanks.
greg k-h
^ permalink raw reply [flat|nested] 28+ messages in thread
* USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data,Re: [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data
@ 2018-12-10 7:18 ` Greg Kroah-Hartman
0 siblings, 0 replies; 28+ messages in thread
From: Greg Kroah-Hartman @ 2018-12-10 7:18 UTC (permalink / raw)
To: Mathias Payer; +Cc: David Miller, netdev, linux-usb, bigeasy, benquike
On Sun, Dec 09, 2018 at 09:17:58PM +0100, Mathias Payer wrote:
> > The whole kernel is full of situations where an int is returned and if it's
> > negative it's an error. Why is this location so different?
> >
> > Just check < 0 and be done with it.
>
> OK, whatever you prefer.
My fault, I was tired and suggested the other way...
> I've attached the updated patch. (Greg, please add your Signed-off-by).
Patches can't be attached, I'll fix this up and resend, thanks.
greg k-h
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v2] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data
@ 2018-12-10 8:04 ` Greg Kroah-Hartman
0 siblings, 0 replies; 28+ messages in thread
From: Greg KH @ 2018-12-10 8:04 UTC (permalink / raw)
To: David S. Miller, netdev
Cc: linux-usb, Sebastian Andrzej Siewior, Hui Peng, Mathias Payer
From: Hui Peng <benquike@gmail.com>
The function hso_probe reads if_num from the USB device (as an u8) and uses
it without a length check to index an array, resulting in an OOB memory read
in hso_probe or hso_get_config_data. Added a length check for both locations
and updated hso_probe to bail on error.
This issue has been assigned CVE-2018-19985.
Reported-by: Hui Peng <benquike@gmail.com>
Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
Signed-off-by: Hui Peng <benquike@gmail.com>
Signed-off-by: Mathias Payer <mathias.payer@nebelwelt.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
v2: fixed error check to just be < 0
Added CVE to changelog text
drivers/net/usb/hso.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 184c24baca15..d6916f787fce 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2807,6 +2807,12 @@ static int hso_get_config_data(struct usb_interface *interface)
return -EIO;
}
+ /* check if we have a valid interface */
+ if (if_num > 16) {
+ kfree(config_data);
+ return -EINVAL;
+ }
+
switch (config_data[if_num]) {
case 0x0:
result = 0;
@@ -2877,10 +2883,18 @@ static int hso_probe(struct usb_interface *interface,
/* Get the interface/port specification from either driver_info or from
* the device itself */
- if (id->driver_info)
+ if (id->driver_info) {
+ /* if_num is controlled by the device, driver_info is a 0 terminated
+ * array. Make sure, the access is in bounds! */
+ for (i = 0; i <= if_num; ++i)
+ if (((u32 *)(id->driver_info))[i] == 0)
+ goto exit;
port_spec = ((u32 *)(id->driver_info))[if_num];
- else
+ } else {
port_spec = hso_get_config_data(interface);
+ if (port_spec < 0)
+ goto exit;
+ }
/* Check if we need to switch to alt interfaces prior to port
* configuration */
--
2.19.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [v2] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data
@ 2018-12-10 8:04 ` Greg Kroah-Hartman
0 siblings, 0 replies; 28+ messages in thread
From: Greg Kroah-Hartman @ 2018-12-10 8:04 UTC (permalink / raw)
To: David S. Miller, netdev
Cc: linux-usb, Sebastian Andrzej Siewior, Hui Peng, Mathias Payer
From: Hui Peng <benquike@gmail.com>
The function hso_probe reads if_num from the USB device (as an u8) and uses
it without a length check to index an array, resulting in an OOB memory read
in hso_probe or hso_get_config_data. Added a length check for both locations
and updated hso_probe to bail on error.
This issue has been assigned CVE-2018-19985.
Reported-by: Hui Peng <benquike@gmail.com>
Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
Signed-off-by: Hui Peng <benquike@gmail.com>
Signed-off-by: Mathias Payer <mathias.payer@nebelwelt.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
v2: fixed error check to just be < 0
Added CVE to changelog text
drivers/net/usb/hso.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 184c24baca15..d6916f787fce 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2807,6 +2807,12 @@ static int hso_get_config_data(struct usb_interface *interface)
return -EIO;
}
+ /* check if we have a valid interface */
+ if (if_num > 16) {
+ kfree(config_data);
+ return -EINVAL;
+ }
+
switch (config_data[if_num]) {
case 0x0:
result = 0;
@@ -2877,10 +2883,18 @@ static int hso_probe(struct usb_interface *interface,
/* Get the interface/port specification from either driver_info or from
* the device itself */
- if (id->driver_info)
+ if (id->driver_info) {
+ /* if_num is controlled by the device, driver_info is a 0 terminated
+ * array. Make sure, the access is in bounds! */
+ for (i = 0; i <= if_num; ++i)
+ if (((u32 *)(id->driver_info))[i] == 0)
+ goto exit;
port_spec = ((u32 *)(id->driver_info))[if_num];
- else
+ } else {
port_spec = hso_get_config_data(interface);
+ if (port_spec < 0)
+ goto exit;
+ }
/* Check if we need to switch to alt interfaces prior to port
* configuration */
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v2] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data
@ 2018-12-10 12:50 ` Sebastian Andrzej Siewior
0 siblings, 0 replies; 28+ messages in thread
From: Sebastian Andrzej Siewior @ 2018-12-10 12:50 UTC (permalink / raw)
To: Greg KH; +Cc: David S. Miller, netdev, linux-usb, Hui Peng, Mathias Payer
On 2018-12-10 09:04:43 [+0100], Greg KH wrote:
> From: Hui Peng <benquike@gmail.com>
>
> The function hso_probe reads if_num from the USB device (as an u8) and uses
> it without a length check to index an array, resulting in an OOB memory read
> in hso_probe or hso_get_config_data. Added a length check for both locations
> and updated hso_probe to bail on error.
The function hso_probe reads if_num from the USB device (as an u8) and uses
it without a length check to index an array, resulting in an OOB memory read
in hso_probe or hso_get_config_data.
Add a length check for both locations and update hso_probe to bail on
error.
> This issue has been assigned CVE-2018-19985.
>
> Reported-by: Hui Peng <benquike@gmail.com>
> Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
> Signed-off-by: Hui Peng <benquike@gmail.com>
> Signed-off-by: Mathias Payer <mathias.payer@nebelwelt.net>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Sebastian
^ permalink raw reply [flat|nested] 28+ messages in thread
* [v2] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data
@ 2018-12-10 12:50 ` Sebastian Andrzej Siewior
0 siblings, 0 replies; 28+ messages in thread
From: Sebastian Andrzej Siewior @ 2018-12-10 12:50 UTC (permalink / raw)
To: Greg KH; +Cc: David S. Miller, netdev, linux-usb, Hui Peng, Mathias Payer
On 2018-12-10 09:04:43 [+0100], Greg KH wrote:
> From: Hui Peng <benquike@gmail.com>
>
> The function hso_probe reads if_num from the USB device (as an u8) and uses
> it without a length check to index an array, resulting in an OOB memory read
> in hso_probe or hso_get_config_data. Added a length check for both locations
> and updated hso_probe to bail on error.
The function hso_probe reads if_num from the USB device (as an u8) and uses
it without a length check to index an array, resulting in an OOB memory read
in hso_probe or hso_get_config_data.
Add a length check for both locations and update hso_probe to bail on
error.
> This issue has been assigned CVE-2018-19985.
>
> Reported-by: Hui Peng <benquike@gmail.com>
> Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
> Signed-off-by: Hui Peng <benquike@gmail.com>
> Signed-off-by: Mathias Payer <mathias.payer@nebelwelt.net>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Sebastian
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH v2] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data
@ 2018-12-12 11:40 ` Greg Kroah-Hartman
0 siblings, 0 replies; 28+ messages in thread
From: Greg KH @ 2018-12-12 11:40 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: David S. Miller, netdev, linux-usb, Hui Peng, Mathias Payer
On Mon, Dec 10, 2018 at 01:50:20PM +0100, Sebastian Andrzej Siewior wrote:
> On 2018-12-10 09:04:43 [+0100], Greg KH wrote:
> > From: Hui Peng <benquike@gmail.com>
> >
> > The function hso_probe reads if_num from the USB device (as an u8) and uses
> > it without a length check to index an array, resulting in an OOB memory read
> > in hso_probe or hso_get_config_data. Added a length check for both locations
> > and updated hso_probe to bail on error.
>
> The function hso_probe reads if_num from the USB device (as an u8) and uses
> it without a length check to index an array, resulting in an OOB memory read
> in hso_probe or hso_get_config_data.
>
> Add a length check for both locations and update hso_probe to bail on
> error.
>
> > This issue has been assigned CVE-2018-19985.
> >
> > Reported-by: Hui Peng <benquike@gmail.com>
> > Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
> > Signed-off-by: Hui Peng <benquike@gmail.com>
> > Signed-off-by: Mathias Payer <mathias.payer@nebelwelt.net>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
> Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Thanks, will update the changelog and add resend.
greg k-h
^ permalink raw reply [flat|nested] 28+ messages in thread
* [v2] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data
@ 2018-12-12 11:40 ` Greg Kroah-Hartman
0 siblings, 0 replies; 28+ messages in thread
From: Greg Kroah-Hartman @ 2018-12-12 11:40 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: David S. Miller, netdev, linux-usb, Hui Peng, Mathias Payer
On Mon, Dec 10, 2018 at 01:50:20PM +0100, Sebastian Andrzej Siewior wrote:
> On 2018-12-10 09:04:43 [+0100], Greg KH wrote:
> > From: Hui Peng <benquike@gmail.com>
> >
> > The function hso_probe reads if_num from the USB device (as an u8) and uses
> > it without a length check to index an array, resulting in an OOB memory read
> > in hso_probe or hso_get_config_data. Added a length check for both locations
> > and updated hso_probe to bail on error.
>
> The function hso_probe reads if_num from the USB device (as an u8) and uses
> it without a length check to index an array, resulting in an OOB memory read
> in hso_probe or hso_get_config_data.
>
> Add a length check for both locations and update hso_probe to bail on
> error.
>
> > This issue has been assigned CVE-2018-19985.
> >
> > Reported-by: Hui Peng <benquike@gmail.com>
> > Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
> > Signed-off-by: Hui Peng <benquike@gmail.com>
> > Signed-off-by: Mathias Payer <mathias.payer@nebelwelt.net>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
> Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Thanks, will update the changelog and add resend.
greg k-h
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH v3] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data
@ 2018-12-12 11:42 ` Greg Kroah-Hartman
0 siblings, 0 replies; 28+ messages in thread
From: Greg KH @ 2018-12-12 11:42 UTC (permalink / raw)
To: David S. Miller, netdev
Cc: linux-usb, Sebastian Andrzej Siewior, Hui Peng, Mathias Payer
From: Hui Peng <benquike@gmail.com>
The function hso_probe reads if_num from the USB device (as an u8) and uses
it without a length check to index an array, resulting in an OOB memory read
in hso_probe or hso_get_config_data.
Add a length check for both locations and updated hso_probe to bail on
error.
This issue has been assigned CVE-2018-19985.
Reported-by: Hui Peng <benquike@gmail.com>
Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
Signed-off-by: Hui Peng <benquike@gmail.com>
Signed-off-by: Mathias Payer <mathias.payer@nebelwelt.net>
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
v3: redid the changelog text based on review comments from Sebastian
v2: fixed error check to just be < 0
Added CVE to changelog text
drivers/net/usb/hso.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 184c24baca15..d6916f787fce 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2807,6 +2807,12 @@ static int hso_get_config_data(struct usb_interface *interface)
return -EIO;
}
+ /* check if we have a valid interface */
+ if (if_num > 16) {
+ kfree(config_data);
+ return -EINVAL;
+ }
+
switch (config_data[if_num]) {
case 0x0:
result = 0;
@@ -2877,10 +2883,18 @@ static int hso_probe(struct usb_interface *interface,
/* Get the interface/port specification from either driver_info or from
* the device itself */
- if (id->driver_info)
+ if (id->driver_info) {
+ /* if_num is controlled by the device, driver_info is a 0 terminated
+ * array. Make sure, the access is in bounds! */
+ for (i = 0; i <= if_num; ++i)
+ if (((u32 *)(id->driver_info))[i] == 0)
+ goto exit;
port_spec = ((u32 *)(id->driver_info))[if_num];
- else
+ } else {
port_spec = hso_get_config_data(interface);
+ if (port_spec < 0)
+ goto exit;
+ }
/* Check if we need to switch to alt interfaces prior to port
* configuration */
--
2.20.0
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [v3] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data
@ 2018-12-12 11:42 ` Greg Kroah-Hartman
0 siblings, 0 replies; 28+ messages in thread
From: Greg Kroah-Hartman @ 2018-12-12 11:42 UTC (permalink / raw)
To: David S. Miller, netdev
Cc: linux-usb, Sebastian Andrzej Siewior, Hui Peng, Mathias Payer
From: Hui Peng <benquike@gmail.com>
The function hso_probe reads if_num from the USB device (as an u8) and uses
it without a length check to index an array, resulting in an OOB memory read
in hso_probe or hso_get_config_data.
Add a length check for both locations and updated hso_probe to bail on
error.
This issue has been assigned CVE-2018-19985.
Reported-by: Hui Peng <benquike@gmail.com>
Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
Signed-off-by: Hui Peng <benquike@gmail.com>
Signed-off-by: Mathias Payer <mathias.payer@nebelwelt.net>
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
v3: redid the changelog text based on review comments from Sebastian
v2: fixed error check to just be < 0
Added CVE to changelog text
drivers/net/usb/hso.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index 184c24baca15..d6916f787fce 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -2807,6 +2807,12 @@ static int hso_get_config_data(struct usb_interface *interface)
return -EIO;
}
+ /* check if we have a valid interface */
+ if (if_num > 16) {
+ kfree(config_data);
+ return -EINVAL;
+ }
+
switch (config_data[if_num]) {
case 0x0:
result = 0;
@@ -2877,10 +2883,18 @@ static int hso_probe(struct usb_interface *interface,
/* Get the interface/port specification from either driver_info or from
* the device itself */
- if (id->driver_info)
+ if (id->driver_info) {
+ /* if_num is controlled by the device, driver_info is a 0 terminated
+ * array. Make sure, the access is in bounds! */
+ for (i = 0; i <= if_num; ++i)
+ if (((u32 *)(id->driver_info))[i] == 0)
+ goto exit;
port_spec = ((u32 *)(id->driver_info))[if_num];
- else
+ } else {
port_spec = hso_get_config_data(interface);
+ if (port_spec < 0)
+ goto exit;
+ }
/* Check if we need to switch to alt interfaces prior to port
* configuration */
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH v3] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data
@ 2018-12-12 23:41 ` David Miller
0 siblings, 0 replies; 28+ messages in thread
From: David Miller @ 2018-12-12 23:41 UTC (permalink / raw)
To: gregkh; +Cc: netdev, linux-usb, bigeasy, benquike, mathias.payer
From: Greg KH <gregkh@linuxfoundation.org>
Date: Wed, 12 Dec 2018 12:42:24 +0100
> From: Hui Peng <benquike@gmail.com>
>
> The function hso_probe reads if_num from the USB device (as an u8) and uses
> it without a length check to index an array, resulting in an OOB memory read
> in hso_probe or hso_get_config_data.
>
> Add a length check for both locations and updated hso_probe to bail on
> error.
>
> This issue has been assigned CVE-2018-19985.
>
> Reported-by: Hui Peng <benquike@gmail.com>
> Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
> Signed-off-by: Hui Peng <benquike@gmail.com>
> Signed-off-by: Mathias Payer <mathias.payer@nebelwelt.net>
> Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Applied and queued up for -stable, thanks Greg.
^ permalink raw reply [flat|nested] 28+ messages in thread
* [v3] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data
@ 2018-12-12 23:41 ` David Miller
0 siblings, 0 replies; 28+ messages in thread
From: David Miller @ 2018-12-12 23:41 UTC (permalink / raw)
To: gregkh; +Cc: netdev, linux-usb, bigeasy, benquike, mathias.payer
From: Greg KH <gregkh@linuxfoundation.org>
Date: Wed, 12 Dec 2018 12:42:24 +0100
> From: Hui Peng <benquike@gmail.com>
>
> The function hso_probe reads if_num from the USB device (as an u8) and uses
> it without a length check to index an array, resulting in an OOB memory read
> in hso_probe or hso_get_config_data.
>
> Add a length check for both locations and updated hso_probe to bail on
> error.
>
> This issue has been assigned CVE-2018-19985.
>
> Reported-by: Hui Peng <benquike@gmail.com>
> Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
> Signed-off-by: Hui Peng <benquike@gmail.com>
> Signed-off-by: Mathias Payer <mathias.payer@nebelwelt.net>
> Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Applied and queued up for -stable, thanks Greg.
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2018-12-12 23:41 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-09 16:32 [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data Greg KH
2018-12-09 16:32 ` Greg Kroah-Hartman
2018-12-09 19:54 ` [PATCH] " David Miller
2018-12-09 19:54 ` David Miller
2018-12-09 20:02 ` [PATCH] " Mathias Payer
2018-12-09 20:02 ` Mathias Payer
2018-12-09 20:06 ` [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data,Re: [PATCH] " David Miller
2018-12-09 20:06 ` David Miller
2018-12-09 20:17 ` [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data,Re: [PATCH] " Mathias Payer
2018-12-09 20:17 ` Mathias Payer
2018-12-09 20:40 ` [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data,Re: [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data,Re: [PATCH] " David Miller
2018-12-09 20:40 ` David Miller
2018-12-09 20:59 ` [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data,Re: [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data,Re: [PATCH] " Mathias Payer
2018-12-09 20:59 ` USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data,Re: [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data,Re: [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data,Re: [PATCH] USB: hso: Fix OOB memory Mathias Payer
2018-12-10 7:18 ` [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data,Re: [PATCH] USB: hso: Fix OOB memory access in hso_probe/hso_get_config_data Greg KH
2018-12-10 7:18 ` Greg Kroah-Hartman
2018-12-09 20:04 ` Mathias Payer
2018-12-09 20:04 ` Mathias Payer
2018-12-10 8:04 ` [PATCH v2] " Greg KH
2018-12-10 8:04 ` [v2] " Greg Kroah-Hartman
2018-12-10 12:50 ` [PATCH v2] " Sebastian Andrzej Siewior
2018-12-10 12:50 ` [v2] " Sebastian Andrzej Siewior
2018-12-12 11:40 ` [PATCH v2] " Greg KH
2018-12-12 11:40 ` [v2] " Greg Kroah-Hartman
2018-12-12 11:42 ` [PATCH v3] " Greg KH
2018-12-12 11:42 ` [v3] " Greg Kroah-Hartman
2018-12-12 23:41 ` [PATCH v3] " David Miller
2018-12-12 23:41 ` [v3] " David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.