linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] Input: fix USB alsetting bugs
@ 2019-12-10 11:37 Johan Hovold
  2019-12-10 11:37 ` [PATCH 1/7] Input: pegasus_notetaker: fix endpoint sanity check Johan Hovold
                   ` (7 more replies)
  0 siblings, 8 replies; 18+ messages in thread
From: Johan Hovold @ 2019-12-10 11:37 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input, linux-usb, linux-kernel, Johan Hovold

We had quite a few driver using the first alternate setting instead of
the current one when doing descriptor sanity checks. This is mostly an
issue on kernels with panic_on_warn set due to a WARN() in
usb_submit_urn(). Since we've started backporting such fixes (e.g. as
reported by syzbot), I've marked these for stable as well.

Included are also a couple of related clean ups to prevent future
issues.

Johan


Johan Hovold (7):
  Input: pegasus_notetaker: fix endpoint sanity check
  Input: aiptek: fix endpoint sanity check
  Input: aiptek: use descriptors of current altsetting
  Input: gtco: fix endpoint sanity check
  Input: gtco: fix extra-descriptor debug message
  Input: gtco: drop redundant variable reinit
  Input: sur40: fix interface sanity checks

 drivers/input/tablet/aiptek.c            |  8 ++++----
 drivers/input/tablet/gtco.c              | 13 ++++---------
 drivers/input/tablet/pegasus_notetaker.c |  2 +-
 drivers/input/touchscreen/sur40.c        |  2 +-
 4 files changed, 10 insertions(+), 15 deletions(-)

-- 
2.24.0


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/7] Input: pegasus_notetaker: fix endpoint sanity check
  2019-12-10 11:37 [PATCH 0/7] Input: fix USB alsetting bugs Johan Hovold
@ 2019-12-10 11:37 ` Johan Hovold
  2019-12-10 11:51   ` Martin Kepplinger
  2020-02-04  8:24   ` Johan Hovold
  2019-12-10 11:37 ` [PATCH 2/7] Input: aiptek: " Johan Hovold
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 18+ messages in thread
From: Johan Hovold @ 2019-12-10 11:37 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-input, linux-usb, linux-kernel, Johan Hovold, stable,
	Martin Kepplinger

The driver was checking the number of endpoints of the first alternate
setting instead of the current one, something which could be used by a
malicious device (or USB descriptor fuzzer) to trigger a NULL-pointer
dereference.

Fixes: 1afca2b66aac ("Input: add Pegasus Notetaker tablet driver")
Cc: stable <stable@vger.kernel.org>     # 4.8
Cc: Martin Kepplinger <martink@posteo.de>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/input/tablet/pegasus_notetaker.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/tablet/pegasus_notetaker.c b/drivers/input/tablet/pegasus_notetaker.c
index a1f3a0cb197e..38f087404f7a 100644
--- a/drivers/input/tablet/pegasus_notetaker.c
+++ b/drivers/input/tablet/pegasus_notetaker.c
@@ -275,7 +275,7 @@ static int pegasus_probe(struct usb_interface *intf,
 		return -ENODEV;
 
 	/* Sanity check that the device has an endpoint */
-	if (intf->altsetting[0].desc.bNumEndpoints < 1) {
+	if (intf->cur_altsetting->desc.bNumEndpoints < 1) {
 		dev_err(&intf->dev, "Invalid number of endpoints\n");
 		return -EINVAL;
 	}
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/7] Input: aiptek: fix endpoint sanity check
  2019-12-10 11:37 [PATCH 0/7] Input: fix USB alsetting bugs Johan Hovold
  2019-12-10 11:37 ` [PATCH 1/7] Input: pegasus_notetaker: fix endpoint sanity check Johan Hovold
@ 2019-12-10 11:37 ` Johan Hovold
  2019-12-11 11:55   ` Vladis Dronov
  2019-12-10 11:37 ` [PATCH 3/7] Input: aiptek: use descriptors of current altsetting Johan Hovold
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Johan Hovold @ 2019-12-10 11:37 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-input, linux-usb, linux-kernel, Johan Hovold, stable,
	Vladis Dronov

The driver was checking the number of endpoints of the first alternate
setting instead of the current one, something which could lead to the
driver binding to an invalid interface.

This in turn could cause the driver to misbehave or trigger a WARN() in
usb_submit_urb() that kernels with panic_on_warn set would choke on.

Fixes: 8e20cf2bce12 ("Input: aiptek - fix crash on detecting device without endpoints")
Cc: stable <stable@vger.kernel.org>     # 4.4
Cc: Vladis Dronov <vdronov@redhat.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/input/tablet/aiptek.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/input/tablet/aiptek.c b/drivers/input/tablet/aiptek.c
index 2ca586fb914f..06d0ffef4a17 100644
--- a/drivers/input/tablet/aiptek.c
+++ b/drivers/input/tablet/aiptek.c
@@ -1802,14 +1802,14 @@ aiptek_probe(struct usb_interface *intf, const struct usb_device_id *id)
 	input_set_abs_params(inputdev, ABS_WHEEL, AIPTEK_WHEEL_MIN, AIPTEK_WHEEL_MAX - 1, 0, 0);
 
 	/* Verify that a device really has an endpoint */
-	if (intf->altsetting[0].desc.bNumEndpoints < 1) {
+	if (intf->cur_altsetting->desc.bNumEndpoints < 1) {
 		dev_err(&intf->dev,
 			"interface has %d endpoints, but must have minimum 1\n",
-			intf->altsetting[0].desc.bNumEndpoints);
+			intf->cur_altsetting->desc.bNumEndpoints);
 		err = -EINVAL;
 		goto fail3;
 	}
-	endpoint = &intf->altsetting[0].endpoint[0].desc;
+	endpoint = &intf->cur_altsetting->endpoint[0].desc;
 
 	/* Go set up our URB, which is called when the tablet receives
 	 * input.
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/7] Input: aiptek: use descriptors of current altsetting
  2019-12-10 11:37 [PATCH 0/7] Input: fix USB alsetting bugs Johan Hovold
  2019-12-10 11:37 ` [PATCH 1/7] Input: pegasus_notetaker: fix endpoint sanity check Johan Hovold
  2019-12-10 11:37 ` [PATCH 2/7] Input: aiptek: " Johan Hovold
@ 2019-12-10 11:37 ` Johan Hovold
  2019-12-10 11:37 ` [PATCH 4/7] Input: gtco: fix endpoint sanity check Johan Hovold
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Johan Hovold @ 2019-12-10 11:37 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input, linux-usb, linux-kernel, Johan Hovold

Make sure to always use the descriptors of the current alternate setting
to avoid future issues when accessing fields that may differ between
settings.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/input/tablet/aiptek.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/tablet/aiptek.c b/drivers/input/tablet/aiptek.c
index 06d0ffef4a17..e08b0ef078e8 100644
--- a/drivers/input/tablet/aiptek.c
+++ b/drivers/input/tablet/aiptek.c
@@ -1713,7 +1713,7 @@ aiptek_probe(struct usb_interface *intf, const struct usb_device_id *id)
 
 	aiptek->inputdev = inputdev;
 	aiptek->intf = intf;
-	aiptek->ifnum = intf->altsetting[0].desc.bInterfaceNumber;
+	aiptek->ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
 	aiptek->inDelay = 0;
 	aiptek->endDelay = 0;
 	aiptek->previousJitterable = 0;
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/7] Input: gtco: fix endpoint sanity check
  2019-12-10 11:37 [PATCH 0/7] Input: fix USB alsetting bugs Johan Hovold
                   ` (2 preceding siblings ...)
  2019-12-10 11:37 ` [PATCH 3/7] Input: aiptek: use descriptors of current altsetting Johan Hovold
@ 2019-12-10 11:37 ` Johan Hovold
  2019-12-11 11:59   ` Vladis Dronov
  2019-12-10 11:37 ` [PATCH 5/7] Input: gtco: fix extra-descriptor debug message Johan Hovold
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Johan Hovold @ 2019-12-10 11:37 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-input, linux-usb, linux-kernel, Johan Hovold, stable,
	Vladis Dronov

The driver was checking the number of endpoints of the first alternate
setting instead of the current one, something which could lead to the
driver binding to an invalid interface.

This in turn could cause the driver to misbehave or trigger a WARN() in
usb_submit_urb() that kernels with panic_on_warn set would choke on.

Fixes: 162f98dea487 ("Input: gtco - fix crash on detecting device without endpoints")
Cc: stable <stable@vger.kernel.org>     # 4.6
Cc: Vladis Dronov <vdronov@redhat.com>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/input/tablet/gtco.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/input/tablet/gtco.c b/drivers/input/tablet/gtco.c
index 35031228a6d0..799c94dda651 100644
--- a/drivers/input/tablet/gtco.c
+++ b/drivers/input/tablet/gtco.c
@@ -875,18 +875,14 @@ static int gtco_probe(struct usb_interface *usbinterface,
 	}
 
 	/* Sanity check that a device has an endpoint */
-	if (usbinterface->altsetting[0].desc.bNumEndpoints < 1) {
+	if (usbinterface->cur_altsetting->desc.bNumEndpoints < 1) {
 		dev_err(&usbinterface->dev,
 			"Invalid number of endpoints\n");
 		error = -EINVAL;
 		goto err_free_urb;
 	}
 
-	/*
-	 * The endpoint is always altsetting 0, we know this since we know
-	 * this device only has one interrupt endpoint
-	 */
-	endpoint = &usbinterface->altsetting[0].endpoint[0].desc;
+	endpoint = &usbinterface->cur_altsetting->endpoint[0].desc;
 
 	/* Some debug */
 	dev_dbg(&usbinterface->dev, "gtco # interfaces: %d\n", usbinterface->num_altsetting);
@@ -973,7 +969,7 @@ static int gtco_probe(struct usb_interface *usbinterface,
 	input_dev->dev.parent = &usbinterface->dev;
 
 	/* Setup the URB, it will be posted later on open of input device */
-	endpoint = &usbinterface->altsetting[0].endpoint[0].desc;
+	endpoint = &usbinterface->cur_altsetting->endpoint[0].desc;
 
 	usb_fill_int_urb(gtco->urbinfo,
 			 udev,
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 5/7] Input: gtco: fix extra-descriptor debug message
  2019-12-10 11:37 [PATCH 0/7] Input: fix USB alsetting bugs Johan Hovold
                   ` (3 preceding siblings ...)
  2019-12-10 11:37 ` [PATCH 4/7] Input: gtco: fix endpoint sanity check Johan Hovold
@ 2019-12-10 11:37 ` Johan Hovold
  2019-12-10 11:37 ` [PATCH 6/7] Input: gtco: drop redundant variable reinit Johan Hovold
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Johan Hovold @ 2019-12-10 11:37 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input, linux-usb, linux-kernel, Johan Hovold

Make sure to use the current altsetting when printing size of any extra
descriptors of the interface.

Also fix the s/endpoint/interface/ mixup in the message itself.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/input/tablet/gtco.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/input/tablet/gtco.c b/drivers/input/tablet/gtco.c
index 799c94dda651..eef5946a6ba4 100644
--- a/drivers/input/tablet/gtco.c
+++ b/drivers/input/tablet/gtco.c
@@ -892,7 +892,8 @@ static int gtco_probe(struct usb_interface *usbinterface,
 	if (usb_endpoint_xfer_int(endpoint))
 		dev_dbg(&usbinterface->dev, "endpoint: we have interrupt endpoint\n");
 
-	dev_dbg(&usbinterface->dev, "endpoint extra len:%d\n", usbinterface->altsetting[0].extralen);
+	dev_dbg(&usbinterface->dev, "interface extra len:%d\n",
+		usbinterface->cur_altsetting->extralen);
 
 	/*
 	 * Find the HID descriptor so we can find out the size of the
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 6/7] Input: gtco: drop redundant variable reinit
  2019-12-10 11:37 [PATCH 0/7] Input: fix USB alsetting bugs Johan Hovold
                   ` (4 preceding siblings ...)
  2019-12-10 11:37 ` [PATCH 5/7] Input: gtco: fix extra-descriptor debug message Johan Hovold
@ 2019-12-10 11:37 ` Johan Hovold
  2019-12-10 11:37 ` [PATCH 7/7] Input: sur40: fix interface sanity checks Johan Hovold
  2019-12-12 15:25 ` [PATCH 0/7] Input: fix USB alsetting bugs Vladis Dronov
  7 siblings, 0 replies; 18+ messages in thread
From: Johan Hovold @ 2019-12-10 11:37 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input, linux-usb, linux-kernel, Johan Hovold

Drop the second, redundant reinitialisation of the endpoint-descriptor
pointer from probe.

Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/input/tablet/gtco.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/input/tablet/gtco.c b/drivers/input/tablet/gtco.c
index eef5946a6ba4..96d65575f75a 100644
--- a/drivers/input/tablet/gtco.c
+++ b/drivers/input/tablet/gtco.c
@@ -970,8 +970,6 @@ static int gtco_probe(struct usb_interface *usbinterface,
 	input_dev->dev.parent = &usbinterface->dev;
 
 	/* Setup the URB, it will be posted later on open of input device */
-	endpoint = &usbinterface->cur_altsetting->endpoint[0].desc;
-
 	usb_fill_int_urb(gtco->urbinfo,
 			 udev,
 			 usb_rcvintpipe(udev,
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 7/7] Input: sur40: fix interface sanity checks
  2019-12-10 11:37 [PATCH 0/7] Input: fix USB alsetting bugs Johan Hovold
                   ` (5 preceding siblings ...)
  2019-12-10 11:37 ` [PATCH 6/7] Input: gtco: drop redundant variable reinit Johan Hovold
@ 2019-12-10 11:37 ` Johan Hovold
  2019-12-12 15:25 ` [PATCH 0/7] Input: fix USB alsetting bugs Vladis Dronov
  7 siblings, 0 replies; 18+ messages in thread
From: Johan Hovold @ 2019-12-10 11:37 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-input, linux-usb, linux-kernel, Johan Hovold, stable,
	Florian Echtler

Make sure to use the current alternate setting when verifying the
interface descriptors to avoid binding to an invalid interface.

This in turn could cause the driver to misbehave or trigger a WARN() in
usb_submit_urb() that kernels with panic_on_warn set would choke on.

Fixes: bdb5c57f209c ("Input: add sur40 driver for Samsung SUR40 (aka MS Surface 2.0/Pixelsense)")
Cc: stable <stable@vger.kernel.org>     # 3.13
Cc: Florian Echtler <floe@butterbrot.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/input/touchscreen/sur40.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index 1dd47dda71cd..34d31c7ec8ba 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -661,7 +661,7 @@ static int sur40_probe(struct usb_interface *interface,
 	int error;
 
 	/* Check if we really have the right interface. */
-	iface_desc = &interface->altsetting[0];
+	iface_desc = interface->cur_altsetting;
 	if (iface_desc->desc.bInterfaceClass != 0xFF)
 		return -ENODEV;
 
-- 
2.24.0


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/7] Input: pegasus_notetaker: fix endpoint sanity check
  2019-12-10 11:37 ` [PATCH 1/7] Input: pegasus_notetaker: fix endpoint sanity check Johan Hovold
@ 2019-12-10 11:51   ` Martin Kepplinger
  2020-02-04  8:24   ` Johan Hovold
  1 sibling, 0 replies; 18+ messages in thread
From: Martin Kepplinger @ 2019-12-10 11:51 UTC (permalink / raw)
  To: Johan Hovold, Dmitry Torokhov
  Cc: linux-input, linux-usb, linux-kernel, stable

On 10.12.19 12:37, Johan Hovold wrote:
> The driver was checking the number of endpoints of the first alternate
> setting instead of the current one, something which could be used by a
> malicious device (or USB descriptor fuzzer) to trigger a NULL-pointer
> dereference.
> 
> Fixes: 1afca2b66aac ("Input: add Pegasus Notetaker tablet driver")
> Cc: stable <stable@vger.kernel.org>     # 4.8
> Cc: Martin Kepplinger <martink@posteo.de>
> Signed-off-by: Johan Hovold <johan@kernel.org>
> ---
>  drivers/input/tablet/pegasus_notetaker.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/input/tablet/pegasus_notetaker.c b/drivers/input/tablet/pegasus_notetaker.c
> index a1f3a0cb197e..38f087404f7a 100644
> --- a/drivers/input/tablet/pegasus_notetaker.c
> +++ b/drivers/input/tablet/pegasus_notetaker.c
> @@ -275,7 +275,7 @@ static int pegasus_probe(struct usb_interface *intf,
>  		return -ENODEV;
>  
>  	/* Sanity check that the device has an endpoint */
> -	if (intf->altsetting[0].desc.bNumEndpoints < 1) {
> +	if (intf->cur_altsetting->desc.bNumEndpoints < 1) {
>  		dev_err(&intf->dev, "Invalid number of endpoints\n");
>  		return -EINVAL;
>  	}
> 

Acked-by: Martin Kepplinger <martink@posteo.de>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/7] Input: aiptek: fix endpoint sanity check
  2019-12-10 11:37 ` [PATCH 2/7] Input: aiptek: " Johan Hovold
@ 2019-12-11 11:55   ` Vladis Dronov
  0 siblings, 0 replies; 18+ messages in thread
From: Vladis Dronov @ 2019-12-11 11:55 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Dmitry Torokhov, linux-input, linux-usb, linux-kernel, stable

A fix for a bug indeed, thank you Johan.

Best regards,
Vladis Dronov | Red Hat, Inc. | The Core Kernel | Senior Software Engineer

----- Original Message -----
> From: "Johan Hovold" <johan@kernel.org>
> To: "Dmitry Torokhov" <dmitry.torokhov@gmail.com>
> Cc: linux-input@vger.kernel.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, "Johan Hovold"
> <johan@kernel.org>, "stable" <stable@vger.kernel.org>, "Vladis Dronov" <vdronov@redhat.com>
> Sent: Tuesday, December 10, 2019 12:37:32 PM
> Subject: [PATCH 2/7] Input: aiptek: fix endpoint sanity check
> 
> The driver was checking the number of endpoints of the first alternate
> setting instead of the current one, something which could lead to the
> driver binding to an invalid interface.
> 
> This in turn could cause the driver to misbehave or trigger a WARN() in
> usb_submit_urb() that kernels with panic_on_warn set would choke on.
> 
> Fixes: 8e20cf2bce12 ("Input: aiptek - fix crash on detecting device without
> endpoints")
> Cc: stable <stable@vger.kernel.org>     # 4.4
> Cc: Vladis Dronov <vdronov@redhat.com>
> Signed-off-by: Johan Hovold <johan@kernel.org>


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/7] Input: gtco: fix endpoint sanity check
  2019-12-10 11:37 ` [PATCH 4/7] Input: gtco: fix endpoint sanity check Johan Hovold
@ 2019-12-11 11:59   ` Vladis Dronov
  0 siblings, 0 replies; 18+ messages in thread
From: Vladis Dronov @ 2019-12-11 11:59 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Dmitry Torokhov, linux-input, linux-usb, linux-kernel, stable

Thanks, Johan! A fix for a bug indeed.

Best regards,
Vladis Dronov | Red Hat, Inc. | The Core Kernel | Senior Software Engineer

----- Original Message -----
> From: "Johan Hovold" <johan@kernel.org>
> To: "Dmitry Torokhov" <dmitry.torokhov@gmail.com>
> Cc: linux-input@vger.kernel.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, "Johan Hovold"
> <johan@kernel.org>, "stable" <stable@vger.kernel.org>, "Vladis Dronov" <vdronov@redhat.com>
> Sent: Tuesday, December 10, 2019 12:37:34 PM
> Subject: [PATCH 4/7] Input: gtco: fix endpoint sanity check
> 
> The driver was checking the number of endpoints of the first alternate
> setting instead of the current one, something which could lead to the
> driver binding to an invalid interface.
> 
> This in turn could cause the driver to misbehave or trigger a WARN() in
> usb_submit_urb() that kernels with panic_on_warn set would choke on.
> 
> Fixes: 162f98dea487 ("Input: gtco - fix crash on detecting device without
> endpoints")
> Cc: stable <stable@vger.kernel.org>     # 4.6
> Cc: Vladis Dronov <vdronov@redhat.com>
> Signed-off-by: Johan Hovold <johan@kernel.org>


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/7] Input: fix USB alsetting bugs
  2019-12-10 11:37 [PATCH 0/7] Input: fix USB alsetting bugs Johan Hovold
                   ` (6 preceding siblings ...)
  2019-12-10 11:37 ` [PATCH 7/7] Input: sur40: fix interface sanity checks Johan Hovold
@ 2019-12-12 15:25 ` Vladis Dronov
  2020-01-10 22:43   ` Dmitry Torokhov
  7 siblings, 1 reply; 18+ messages in thread
From: Vladis Dronov @ 2019-12-12 15:25 UTC (permalink / raw)
  To: Johan Hovold; +Cc: Dmitry Torokhov, linux-input, linux-usb, linux-kernel

Hello,

On 10.12.19 12:37, Johan Hovold wrote:

> We had quite a few driver using the first alternate setting instead of
> the current one when doing descriptor sanity checks. This is mostly an
> issue on kernels with panic_on_warn set due to a WARN() in
> usb_submit_urn(). Since we've started backporting such fixes (e.g. as
> reported by syzbot), I've marked these for stable as well.
> 
> Included are also a couple of related clean ups to prevent future
> issues.

For the series:

Acked-by: Vladis Dronov <vdronov@redhat.com>

Best regards,
Vladis Dronov | Red Hat, Inc. | The Core Kernel | Senior Software Engineer


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/7] Input: fix USB alsetting bugs
  2019-12-12 15:25 ` [PATCH 0/7] Input: fix USB alsetting bugs Vladis Dronov
@ 2020-01-10 22:43   ` Dmitry Torokhov
  0 siblings, 0 replies; 18+ messages in thread
From: Dmitry Torokhov @ 2020-01-10 22:43 UTC (permalink / raw)
  To: Vladis Dronov; +Cc: Johan Hovold, linux-input, linux-usb, linux-kernel

On Thu, Dec 12, 2019 at 04:25:18PM +0100, Vladis Dronov wrote:
> Hello,
> 
> On 10.12.19 12:37, Johan Hovold wrote:
> 
> > We had quite a few driver using the first alternate setting instead of
> > the current one when doing descriptor sanity checks. This is mostly an
> > issue on kernels with panic_on_warn set due to a WARN() in
> > usb_submit_urn(). Since we've started backporting such fixes (e.g. as
> > reported by syzbot), I've marked these for stable as well.
> > 
> > Included are also a couple of related clean ups to prevent future
> > issues.
> 
> For the series:
> 
> Acked-by: Vladis Dronov <vdronov@redhat.com>

Applied the lot, thank you.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/7] Input: pegasus_notetaker: fix endpoint sanity check
  2019-12-10 11:37 ` [PATCH 1/7] Input: pegasus_notetaker: fix endpoint sanity check Johan Hovold
  2019-12-10 11:51   ` Martin Kepplinger
@ 2020-02-04  8:24   ` Johan Hovold
  2020-02-04 10:02     ` Greg KH
  1 sibling, 1 reply; 18+ messages in thread
From: Johan Hovold @ 2020-02-04  8:24 UTC (permalink / raw)
  To: Dmitry Torokhov, stable
  Cc: linux-input, linux-usb, linux-kernel, Johan Hovold, stable,
	Martin Kepplinger

On Tue, Dec 10, 2019 at 12:37:31PM +0100, Johan Hovold wrote:
> The driver was checking the number of endpoints of the first alternate
> setting instead of the current one, something which could be used by a
> malicious device (or USB descriptor fuzzer) to trigger a NULL-pointer
> dereference.
> 
> Fixes: 1afca2b66aac ("Input: add Pegasus Notetaker tablet driver")
> Cc: stable <stable@vger.kernel.org>     # 4.8
> Cc: Martin Kepplinger <martink@posteo.de>
> Signed-off-by: Johan Hovold <johan@kernel.org>

Looks like the stable tag was removed when this one was applied, and
similar for patches 2, 4 and 7 of this series (commits 3111491fca4f,
a8eeb74df5a6, 6b32391ed675 upstream).

While the last three are mostly an issue for the syzbot fuzzer, we have
started backporting those as well.

This one (bcfcb7f9b480) is more clear cut as it can be used to trigger a
NULL-deref.

I only noticed because Sasha picked up one of the other patches in the
series which was never intended for stable.

> ---
>  drivers/input/tablet/pegasus_notetaker.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/input/tablet/pegasus_notetaker.c b/drivers/input/tablet/pegasus_notetaker.c
> index a1f3a0cb197e..38f087404f7a 100644
> --- a/drivers/input/tablet/pegasus_notetaker.c
> +++ b/drivers/input/tablet/pegasus_notetaker.c
> @@ -275,7 +275,7 @@ static int pegasus_probe(struct usb_interface *intf,
>  		return -ENODEV;
>  
>  	/* Sanity check that the device has an endpoint */
> -	if (intf->altsetting[0].desc.bNumEndpoints < 1) {
> +	if (intf->cur_altsetting->desc.bNumEndpoints < 1) {
>  		dev_err(&intf->dev, "Invalid number of endpoints\n");
>  		return -EINVAL;
>  	}

Johan

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/7] Input: pegasus_notetaker: fix endpoint sanity check
  2020-02-04  8:24   ` Johan Hovold
@ 2020-02-04 10:02     ` Greg KH
  2020-02-04 10:14       ` Johan Hovold
  0 siblings, 1 reply; 18+ messages in thread
From: Greg KH @ 2020-02-04 10:02 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Dmitry Torokhov, stable, linux-input, linux-usb, linux-kernel,
	Martin Kepplinger

On Tue, Feb 04, 2020 at 09:24:41AM +0100, Johan Hovold wrote:
> On Tue, Dec 10, 2019 at 12:37:31PM +0100, Johan Hovold wrote:
> > The driver was checking the number of endpoints of the first alternate
> > setting instead of the current one, something which could be used by a
> > malicious device (or USB descriptor fuzzer) to trigger a NULL-pointer
> > dereference.
> > 
> > Fixes: 1afca2b66aac ("Input: add Pegasus Notetaker tablet driver")
> > Cc: stable <stable@vger.kernel.org>     # 4.8
> > Cc: Martin Kepplinger <martink@posteo.de>
> > Signed-off-by: Johan Hovold <johan@kernel.org>
> 
> Looks like the stable tag was removed when this one was applied, and
> similar for patches 2, 4 and 7 of this series (commits 3111491fca4f,
> a8eeb74df5a6, 6b32391ed675 upstream).
> 
> While the last three are mostly an issue for the syzbot fuzzer, we have
> started backporting those as well.
> 
> This one (bcfcb7f9b480) is more clear cut as it can be used to trigger a
> NULL-deref.
> 
> I only noticed because Sasha picked up one of the other patches in the
> series which was never intended for stable.

Did I end up catching all of these properly?  I've had to expand my
search for some patches like this that do not explicitly have the cc:
stable mark on them as not all subsystems do this well (if at all.)

And there's also Sasha's work in digging up patches based on patterns of
fixes, which also is needed because of this "problem".

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/7] Input: pegasus_notetaker: fix endpoint sanity check
  2020-02-04 10:02     ` Greg KH
@ 2020-02-04 10:14       ` Johan Hovold
  2020-02-04 19:24         ` Dmitry Torokhov
  0 siblings, 1 reply; 18+ messages in thread
From: Johan Hovold @ 2020-02-04 10:14 UTC (permalink / raw)
  To: Greg KH
  Cc: Johan Hovold, Dmitry Torokhov, stable, linux-input, linux-usb,
	linux-kernel, Martin Kepplinger

On Tue, Feb 04, 2020 at 10:02:32AM +0000, Greg Kroah-Hartman wrote:
> On Tue, Feb 04, 2020 at 09:24:41AM +0100, Johan Hovold wrote:
> > On Tue, Dec 10, 2019 at 12:37:31PM +0100, Johan Hovold wrote:
> > > The driver was checking the number of endpoints of the first alternate
> > > setting instead of the current one, something which could be used by a
> > > malicious device (or USB descriptor fuzzer) to trigger a NULL-pointer
> > > dereference.
> > > 
> > > Fixes: 1afca2b66aac ("Input: add Pegasus Notetaker tablet driver")
> > > Cc: stable <stable@vger.kernel.org>     # 4.8
> > > Cc: Martin Kepplinger <martink@posteo.de>
> > > Signed-off-by: Johan Hovold <johan@kernel.org>
> > 
> > Looks like the stable tag was removed when this one was applied, and
> > similar for patches 2, 4 and 7 of this series (commits 3111491fca4f,
> > a8eeb74df5a6, 6b32391ed675 upstream).
> > 
> > While the last three are mostly an issue for the syzbot fuzzer, we have
> > started backporting those as well.
> > 
> > This one (bcfcb7f9b480) is more clear cut as it can be used to trigger a
> > NULL-deref.
> > 
> > I only noticed because Sasha picked up one of the other patches in the
> > series which was never intended for stable.
> 
> Did I end up catching all of these properly?  I've had to expand my
> search for some patches like this that do not explicitly have the cc:
> stable mark on them as not all subsystems do this well (if at all.)

No, sorry, should have been more clear on that point; these four were
never picked up for stable it seems.

I was a bit surprised to see the stable-tags be removed from the
original submissions here, even if I know the net-maintainers do this
routinely, and any maintainer can of course override a submitters
judgement.

> And there's also Sasha's work in digging up patches based on patterns of
> fixes, which also is needed because of this "problem".

Yeah, seems likely that autosel would have caught these eventually.

Johan

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/7] Input: pegasus_notetaker: fix endpoint sanity check
  2020-02-04 10:14       ` Johan Hovold
@ 2020-02-04 19:24         ` Dmitry Torokhov
  2020-02-05 12:54           ` Johan Hovold
  0 siblings, 1 reply; 18+ messages in thread
From: Dmitry Torokhov @ 2020-02-04 19:24 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Greg KH, stable, linux-input, linux-usb, linux-kernel, Martin Kepplinger

On Tue, Feb 04, 2020 at 11:14:35AM +0100, Johan Hovold wrote:
> On Tue, Feb 04, 2020 at 10:02:32AM +0000, Greg Kroah-Hartman wrote:
> > On Tue, Feb 04, 2020 at 09:24:41AM +0100, Johan Hovold wrote:
> > > On Tue, Dec 10, 2019 at 12:37:31PM +0100, Johan Hovold wrote:
> > > > The driver was checking the number of endpoints of the first alternate
> > > > setting instead of the current one, something which could be used by a
> > > > malicious device (or USB descriptor fuzzer) to trigger a NULL-pointer
> > > > dereference.
> > > > 
> > > > Fixes: 1afca2b66aac ("Input: add Pegasus Notetaker tablet driver")
> > > > Cc: stable <stable@vger.kernel.org>     # 4.8
> > > > Cc: Martin Kepplinger <martink@posteo.de>
> > > > Signed-off-by: Johan Hovold <johan@kernel.org>
> > > 
> > > Looks like the stable tag was removed when this one was applied, and
> > > similar for patches 2, 4 and 7 of this series (commits 3111491fca4f,
> > > a8eeb74df5a6, 6b32391ed675 upstream).
> > > 
> > > While the last three are mostly an issue for the syzbot fuzzer, we have
> > > started backporting those as well.
> > > 
> > > This one (bcfcb7f9b480) is more clear cut as it can be used to trigger a
> > > NULL-deref.
> > > 
> > > I only noticed because Sasha picked up one of the other patches in the
> > > series which was never intended for stable.
> > 
> > Did I end up catching all of these properly?  I've had to expand my
> > search for some patches like this that do not explicitly have the cc:
> > stable mark on them as not all subsystems do this well (if at all.)
> 
> No, sorry, should have been more clear on that point; these four were
> never picked up for stable it seems.
> 
> I was a bit surprised to see the stable-tags be removed from the
> original submissions here, even if I know the net-maintainers do this
> routinely, and any maintainer can of course override a submitters
> judgement.

Sorry, dropping stable tags was not intentional. I was trying to improve
my tooling and drop the CC noise from the changelogs, but my script got
too eager and removed "Cc: stable" as well. I believe my tool should be
fixed now and it should not happen again.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/7] Input: pegasus_notetaker: fix endpoint sanity check
  2020-02-04 19:24         ` Dmitry Torokhov
@ 2020-02-05 12:54           ` Johan Hovold
  0 siblings, 0 replies; 18+ messages in thread
From: Johan Hovold @ 2020-02-05 12:54 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Johan Hovold, Greg KH, stable, linux-input, linux-usb,
	linux-kernel, Martin Kepplinger

On Tue, Feb 04, 2020 at 11:24:34AM -0800, Dmitry Torokhov wrote:
> On Tue, Feb 04, 2020 at 11:14:35AM +0100, Johan Hovold wrote:
> > On Tue, Feb 04, 2020 at 10:02:32AM +0000, Greg Kroah-Hartman wrote:
> > > On Tue, Feb 04, 2020 at 09:24:41AM +0100, Johan Hovold wrote:
> > > > On Tue, Dec 10, 2019 at 12:37:31PM +0100, Johan Hovold wrote:
> > > > > The driver was checking the number of endpoints of the first alternate
> > > > > setting instead of the current one, something which could be used by a
> > > > > malicious device (or USB descriptor fuzzer) to trigger a NULL-pointer
> > > > > dereference.
> > > > > 
> > > > > Fixes: 1afca2b66aac ("Input: add Pegasus Notetaker tablet driver")
> > > > > Cc: stable <stable@vger.kernel.org>     # 4.8
> > > > > Cc: Martin Kepplinger <martink@posteo.de>
> > > > > Signed-off-by: Johan Hovold <johan@kernel.org>
> > > > 
> > > > Looks like the stable tag was removed when this one was applied, and
> > > > similar for patches 2, 4 and 7 of this series (commits 3111491fca4f,
> > > > a8eeb74df5a6, 6b32391ed675 upstream).
> > > > 
> > > > While the last three are mostly an issue for the syzbot fuzzer, we have
> > > > started backporting those as well.
> > > > 
> > > > This one (bcfcb7f9b480) is more clear cut as it can be used to trigger a
> > > > NULL-deref.
> > > > 
> > > > I only noticed because Sasha picked up one of the other patches in the
> > > > series which was never intended for stable.
> > > 
> > > Did I end up catching all of these properly?  I've had to expand my
> > > search for some patches like this that do not explicitly have the cc:
> > > stable mark on them as not all subsystems do this well (if at all.)
> > 
> > No, sorry, should have been more clear on that point; these four were
> > never picked up for stable it seems.
> > 
> > I was a bit surprised to see the stable-tags be removed from the
> > original submissions here, even if I know the net-maintainers do this
> > routinely, and any maintainer can of course override a submitters
> > judgement.
> 
> Sorry, dropping stable tags was not intentional. I was trying to improve
> my tooling and drop the CC noise from the changelogs, but my script got
> too eager and removed "Cc: stable" as well. I believe my tool should be
> fixed now and it should not happen again.

Ah, ok, thanks for confirming.

Johan

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2020-02-05 12:54 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-10 11:37 [PATCH 0/7] Input: fix USB alsetting bugs Johan Hovold
2019-12-10 11:37 ` [PATCH 1/7] Input: pegasus_notetaker: fix endpoint sanity check Johan Hovold
2019-12-10 11:51   ` Martin Kepplinger
2020-02-04  8:24   ` Johan Hovold
2020-02-04 10:02     ` Greg KH
2020-02-04 10:14       ` Johan Hovold
2020-02-04 19:24         ` Dmitry Torokhov
2020-02-05 12:54           ` Johan Hovold
2019-12-10 11:37 ` [PATCH 2/7] Input: aiptek: " Johan Hovold
2019-12-11 11:55   ` Vladis Dronov
2019-12-10 11:37 ` [PATCH 3/7] Input: aiptek: use descriptors of current altsetting Johan Hovold
2019-12-10 11:37 ` [PATCH 4/7] Input: gtco: fix endpoint sanity check Johan Hovold
2019-12-11 11:59   ` Vladis Dronov
2019-12-10 11:37 ` [PATCH 5/7] Input: gtco: fix extra-descriptor debug message Johan Hovold
2019-12-10 11:37 ` [PATCH 6/7] Input: gtco: drop redundant variable reinit Johan Hovold
2019-12-10 11:37 ` [PATCH 7/7] Input: sur40: fix interface sanity checks Johan Hovold
2019-12-12 15:25 ` [PATCH 0/7] Input: fix USB alsetting bugs Vladis Dronov
2020-01-10 22:43   ` Dmitry Torokhov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).