Linux-USB Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/4] USB: fix NULL-deref and alternate settings bugs
@ 2019-12-10 11:25 Johan Hovold
  2019-12-10 11:25 ` [PATCH 1/4] USB: atm: ueagle-atm: add missing endpoint check Johan Hovold
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Johan Hovold @ 2019-12-10 11:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-usb, Johan Hovold

I stumbled over a couple potential NULL-pointer dereferences due to
drivers using the first altsetting instead of the current one when doing
descriptor sanity checks.

Turns out we have a quite a few drivers getting this wrong even if this
would mostly be an issue on kernels with panic_on_warn set due to the
WARN() in usb_submit_urb().

Since we've started backporting fixes for such warnings (e.g. as
reported by syzbot), I've marked these for stable as well.

Johan


Johan Hovold (4):
  USB: atm: ueagle-atm: add missing endpoint check
  USB: adutux: fix interface sanity check
  USB: idmouse: fix interface sanity checks
  USB: serial: io_edgeport: fix epic endpoint lookup

 drivers/usb/atm/ueagle-atm.c     | 18 ++++++++++++------
 drivers/usb/misc/adutux.c        |  2 +-
 drivers/usb/misc/idmouse.c       |  2 +-
 drivers/usb/serial/io_edgeport.c | 10 ++++++----
 4 files changed, 20 insertions(+), 12 deletions(-)

-- 
2.24.0


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

* [PATCH 1/4] USB: atm: ueagle-atm: add missing endpoint check
  2019-12-10 11:25 [PATCH 0/4] USB: fix NULL-deref and alternate settings bugs Johan Hovold
@ 2019-12-10 11:25 ` Johan Hovold
  2019-12-10 11:25 ` [PATCH 2/4] USB: adutux: fix interface sanity check Johan Hovold
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Johan Hovold @ 2019-12-10 11:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-usb, Johan Hovold, stable

Make sure that the interrupt interface has an endpoint before trying to
access its endpoint descriptors to avoid dereferencing a NULL pointer.

The driver binds to the interrupt interface with interface number 0, but
must not assume that this interface or its current alternate setting are
the first entries in the corresponding configuration arrays.

Fixes: b72458a80c75 ("[PATCH] USB: Eagle and ADI 930 usb adsl modem driver")
Cc: stable <stable@vger.kernel.org>     # 2.6.16
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/atm/ueagle-atm.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/atm/ueagle-atm.c b/drivers/usb/atm/ueagle-atm.c
index 8b0ea8c70d73..635cf0466b59 100644
--- a/drivers/usb/atm/ueagle-atm.c
+++ b/drivers/usb/atm/ueagle-atm.c
@@ -2124,10 +2124,11 @@ static void uea_intr(struct urb *urb)
 /*
  * Start the modem : init the data and start kernel thread
  */
-static int uea_boot(struct uea_softc *sc)
+static int uea_boot(struct uea_softc *sc, struct usb_interface *intf)
 {
-	int ret, size;
 	struct intr_pkt *intr;
+	int ret = -ENOMEM;
+	int size;
 
 	uea_enters(INS_TO_USBDEV(sc));
 
@@ -2152,6 +2153,11 @@ static int uea_boot(struct uea_softc *sc)
 	if (UEA_CHIP_VERSION(sc) == ADI930)
 		load_XILINX_firmware(sc);
 
+	if (intf->cur_altsetting->desc.bNumEndpoints < 1) {
+		ret = -ENODEV;
+		goto err0;
+	}
+
 	intr = kmalloc(size, GFP_KERNEL);
 	if (!intr)
 		goto err0;
@@ -2163,8 +2169,7 @@ static int uea_boot(struct uea_softc *sc)
 	usb_fill_int_urb(sc->urb_int, sc->usb_dev,
 			 usb_rcvintpipe(sc->usb_dev, UEA_INTR_PIPE),
 			 intr, size, uea_intr, sc,
-			 sc->usb_dev->actconfig->interface[0]->altsetting[0].
-			 endpoint[0].desc.bInterval);
+			 intf->cur_altsetting->endpoint[0].desc.bInterval);
 
 	ret = usb_submit_urb(sc->urb_int, GFP_KERNEL);
 	if (ret < 0) {
@@ -2179,6 +2184,7 @@ static int uea_boot(struct uea_softc *sc)
 	sc->kthread = kthread_create(uea_kthread, sc, "ueagle-atm");
 	if (IS_ERR(sc->kthread)) {
 		uea_err(INS_TO_USBDEV(sc), "failed to create thread\n");
+		ret = PTR_ERR(sc->kthread);
 		goto err2;
 	}
 
@@ -2193,7 +2199,7 @@ static int uea_boot(struct uea_softc *sc)
 	kfree(intr);
 err0:
 	uea_leaves(INS_TO_USBDEV(sc));
-	return -ENOMEM;
+	return ret;
 }
 
 /*
@@ -2548,7 +2554,7 @@ static int uea_bind(struct usbatm_data *usbatm, struct usb_interface *intf,
 		}
 	}
 
-	ret = uea_boot(sc);
+	ret = uea_boot(sc, intf);
 	if (ret < 0)
 		goto error;
 
-- 
2.24.0


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

* [PATCH 2/4] USB: adutux: fix interface sanity check
  2019-12-10 11:25 [PATCH 0/4] USB: fix NULL-deref and alternate settings bugs Johan Hovold
  2019-12-10 11:25 ` [PATCH 1/4] USB: atm: ueagle-atm: add missing endpoint check Johan Hovold
@ 2019-12-10 11:25 ` Johan Hovold
  2019-12-10 11:26 ` [PATCH 3/4] USB: idmouse: fix interface sanity checks Johan Hovold
  2019-12-10 11:26 ` [PATCH 4/4] USB: serial: io_edgeport: fix epic endpoint lookup Johan Hovold
  3 siblings, 0 replies; 5+ messages in thread
From: Johan Hovold @ 2019-12-10 11:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-usb, Johan Hovold, stable

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

Failing to do so 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: 03270634e242 ("USB: Add ADU support for Ontrak ADU devices")
Cc: stable <stable@vger.kernel.org>     # 2.6.19
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/misc/adutux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/misc/adutux.c b/drivers/usb/misc/adutux.c
index 6f5edb9fc61e..d8d157c4c271 100644
--- a/drivers/usb/misc/adutux.c
+++ b/drivers/usb/misc/adutux.c
@@ -669,7 +669,7 @@ static int adu_probe(struct usb_interface *interface,
 	init_waitqueue_head(&dev->read_wait);
 	init_waitqueue_head(&dev->write_wait);
 
-	res = usb_find_common_endpoints_reverse(&interface->altsetting[0],
+	res = usb_find_common_endpoints_reverse(interface->cur_altsetting,
 			NULL, NULL,
 			&dev->interrupt_in_endpoint,
 			&dev->interrupt_out_endpoint);
-- 
2.24.0


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

* [PATCH 3/4] USB: idmouse: fix interface sanity checks
  2019-12-10 11:25 [PATCH 0/4] USB: fix NULL-deref and alternate settings bugs Johan Hovold
  2019-12-10 11:25 ` [PATCH 1/4] USB: atm: ueagle-atm: add missing endpoint check Johan Hovold
  2019-12-10 11:25 ` [PATCH 2/4] USB: adutux: fix interface sanity check Johan Hovold
@ 2019-12-10 11:26 ` Johan Hovold
  2019-12-10 11:26 ` [PATCH 4/4] USB: serial: io_edgeport: fix epic endpoint lookup Johan Hovold
  3 siblings, 0 replies; 5+ messages in thread
From: Johan Hovold @ 2019-12-10 11:26 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-usb, Johan Hovold, stable

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

Failing to do so 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: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/misc/idmouse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/misc/idmouse.c b/drivers/usb/misc/idmouse.c
index 4afb5ddfd361..e9437a176518 100644
--- a/drivers/usb/misc/idmouse.c
+++ b/drivers/usb/misc/idmouse.c
@@ -322,7 +322,7 @@ static int idmouse_probe(struct usb_interface *interface,
 	int result;
 
 	/* check if we have gotten the data or the hid interface */
-	iface_desc = &interface->altsetting[0];
+	iface_desc = interface->cur_altsetting;
 	if (iface_desc->desc.bInterfaceClass != 0x0A)
 		return -ENODEV;
 
-- 
2.24.0


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

* [PATCH 4/4] USB: serial: io_edgeport: fix epic endpoint lookup
  2019-12-10 11:25 [PATCH 0/4] USB: fix NULL-deref and alternate settings bugs Johan Hovold
                   ` (2 preceding siblings ...)
  2019-12-10 11:26 ` [PATCH 3/4] USB: idmouse: fix interface sanity checks Johan Hovold
@ 2019-12-10 11:26 ` Johan Hovold
  3 siblings, 0 replies; 5+ messages in thread
From: Johan Hovold @ 2019-12-10 11:26 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-usb, Johan Hovold, stable

Make sure to use the current alternate setting when looking up the
endpoints on epic devices to avoid binding to an invalid interface.

Failing to do so 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: 6e8cf7751f9f ("USB: add EPIC support to the io_edgeport driver")
Cc: stable <stable@vger.kernel.org>     # 2.6.21
Signed-off-by: Johan Hovold <johan@kernel.org>
---
 drivers/usb/serial/io_edgeport.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/serial/io_edgeport.c b/drivers/usb/serial/io_edgeport.c
index 48a439298a68..9690a5f4b9d6 100644
--- a/drivers/usb/serial/io_edgeport.c
+++ b/drivers/usb/serial/io_edgeport.c
@@ -2901,16 +2901,18 @@ static int edge_startup(struct usb_serial *serial)
 	response = 0;
 
 	if (edge_serial->is_epic) {
+		struct usb_host_interface *alt;
+
+		alt = serial->interface->cur_altsetting;
+
 		/* EPIC thing, set up our interrupt polling now and our read
 		 * urb, so that the device knows it really is connected. */
 		interrupt_in_found = bulk_in_found = bulk_out_found = false;
-		for (i = 0; i < serial->interface->altsetting[0]
-						.desc.bNumEndpoints; ++i) {
+		for (i = 0; i < alt->desc.bNumEndpoints; ++i) {
 			struct usb_endpoint_descriptor *endpoint;
 			int buffer_size;
 
-			endpoint = &serial->interface->altsetting[0].
-							endpoint[i].desc;
+			endpoint = &alt->endpoint[i].desc;
 			buffer_size = usb_endpoint_maxp(endpoint);
 			if (!interrupt_in_found &&
 			    (usb_endpoint_is_int_in(endpoint))) {
-- 
2.24.0


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

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-10 11:25 [PATCH 0/4] USB: fix NULL-deref and alternate settings bugs Johan Hovold
2019-12-10 11:25 ` [PATCH 1/4] USB: atm: ueagle-atm: add missing endpoint check Johan Hovold
2019-12-10 11:25 ` [PATCH 2/4] USB: adutux: fix interface sanity check Johan Hovold
2019-12-10 11:26 ` [PATCH 3/4] USB: idmouse: fix interface sanity checks Johan Hovold
2019-12-10 11:26 ` [PATCH 4/4] USB: serial: io_edgeport: fix epic endpoint lookup Johan Hovold

Linux-USB Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-usb/0 linux-usb/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-usb linux-usb/ https://lore.kernel.org/linux-usb \
		linux-usb@vger.kernel.org
	public-inbox-index linux-usb

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-usb


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git