Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1599293 > unrolled thread

[PATCH 0/7] Input: fix NULL-derefs at probe

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/7] Input: fix NULL-derefs at probe Johan Hovold <johan@kernel.org> - 2017-03-13 13:40 +0100
    [PATCH 7/7] Input: sur40 - fix NULL-deref at probe Johan Hovold <johan@kernel.org> - 2017-03-13 13:40 +0100
    Re: [PATCH 0/7] Input: fix NULL-derefs at probe Oliver Neukum <oneukum@suse.com> - 2017-03-13 16:20 +0100
      Re: [PATCH 0/7] Input: fix NULL-derefs at probe Johan Hovold <johan@kernel.org> - 2017-03-13 16:50 +0100

#1599293 — [PATCH 0/7] Input: fix NULL-derefs at probe

This series fixes a number of NULL-pointer dereferences due to missing
endpoint sanity checks that can be triggered by a malicious USB device.

Johan


Johan Hovold (7):
  Input: iforce - fix NULL-deref at probe
  Input: cm109 - fix NULL-deref at probe
  Input: ims-pcu - fix NULL-deref at probe
  Input: yealink - fix NULL-deref at probe
  Input: hanwang - fix NULL-deref at probe
  Input: kbtab - fix NULL-deref at probe
  Input: sur40 - fix NULL-deref at probe

 drivers/input/joystick/iforce/iforce-usb.c | 3 +++
 drivers/input/misc/cm109.c                 | 4 ++++
 drivers/input/misc/ims-pcu.c               | 4 ++++
 drivers/input/misc/yealink.c               | 4 ++++
 drivers/input/tablet/hanwang.c             | 3 +++
 drivers/input/tablet/kbtab.c               | 3 +++
 drivers/input/touchscreen/sur40.c          | 3 +++
 7 files changed, 24 insertions(+)

-- 
2.12.0

[toc] | [next] | [standalone]

#1599294 — [PATCH 7/7] Input: sur40 - fix NULL-deref at probe

Make sure to check the number of endpoints to avoid dereferencing a
NULL-pointer or accessing memory that lie beyond the end of the endpoint
array should a malicious device lack the expected endpoints.

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>
Cc: David Herrmann <dh.herrmann@gmail.com>
Cc: Henrik Rydberg <rydberg@euromail.se>

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

diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
index aefb6e11f88a..4c0eecae065c 100644
--- a/drivers/input/touchscreen/sur40.c
+++ b/drivers/input/touchscreen/sur40.c
@@ -527,6 +527,9 @@ static int sur40_probe(struct usb_interface *interface,
 	if (iface_desc->desc.bInterfaceClass != 0xFF)
 		return -ENODEV;
 
+	if (iface_desc->desc.bNumEndpoints < 5)
+		return -ENODEV;
+
 	/* Use endpoint #4 (0x86). */
 	endpoint = &iface_desc->endpoint[4].desc;
 	if (endpoint->bEndpointAddress != TOUCH_ENDPOINT)
-- 
2.12.0

[toc] | [prev] | [next] | [standalone]

#1599501

Am Montag, den 13.03.2017, 13:35 +0100 schrieb Johan Hovold:
> This series fixes a number of NULL-pointer dereferences due to
> missing
> endpoint sanity checks that can be triggered by a malicious USB
> device.
> 
At the risk of repeating myself, doesn't the sheer number of fixes
demonstrate the need for a more centralized check?

	Regards
		Oliver

[toc] | [prev] | [next] | [standalone]

#1599540

On Mon, Mar 13, 2017 at 04:15:18PM +0100, Oliver Neukum wrote:
> Am Montag, den 13.03.2017, 13:35 +0100 schrieb Johan Hovold:
> > This series fixes a number of NULL-pointer dereferences due to
> > missing
> > endpoint sanity checks that can be triggered by a malicious USB
> > device.
>
> At the risk of repeating myself, doesn't the sheer number of fixes
> demonstrate the need for a more centralized check?

No, I don't think that follows. These are plain bugs that needs to be
fixed (cf. not checking for allocation failures or whatever) and
backported to the stable trees.

I think I may have surveyed just about every USB driver this last week,
and there is no single pattern for how endpoints are verified and
retrieved that could easily be refactored into USB core.

Now there are certain patterns that could benefit from a few helpers,
and some obvious bugs could then be caught by declaring those helpers as
__must_check. But specifically, you'd still be checking the return
value from the helpers.

Then verifying the endpoint counts before calling driver probe,
typically only saves a bit of time while probing *malicious* devices
(and the occasional odd interface which cannot be matched on other
attributes).

That being said, we could still add a centralised sanity check for a
large class of drivers (e.g. that do not use altsettings and only need
minimum constraints) but it's not going to obviate the need for careful
driver implementations.

I'll be posting some more patches related to this shortly.

Thanks,
Johan

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web