* [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).