linux-kernel - [PATCH] Input: bcm5974 - check endpoint type before starting traffic

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 08 Oct 2023 21:16:30 +0200
From:   Javier Carrasco <javier.carrasco.cruz@...il.com>
To:     John Horan <knasher@...il.com>,
        Henrik Rydberg <rydberg@...math.org>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc:     linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
        Javier Carrasco <javier.carrasco.cruz@...il.com>,
        syzbot+348331f63b034f89b622@...kaller.appspotmail.com
Subject: [PATCH] Input: bcm5974 - check endpoint type before starting
 traffic

syzbot has found a type mismatch between a USB pipe and the transfer
endpoint, which is triggered by the bcm5974 driver[1].

This driver expects the device to provide input interrupt endpoints and
if that is not the case, the driver registration should terminate.

Repros are available to reproduce this issue with a certain setup for
the dummy_hcd, leading to an interrupt/bulk mismatch which is caught in
the USB core after calling usb_submit_urb() with the following message:
"BOGUS urb xfer, pipe 1 != type 3"

Some other device drivers (like the appletouch driver bcm5974 is mainly
based on) provide some checking mechanism to make sure that an IN
interrupt endpoint is available. In this particular case the endpoint
addresses are provided by a config table, so the checking can be
targeted to the provided endpoints.

Add some basic checking to guarantee that the endpoints available match
the expected type for both the trackpad and button endpoints.

This issue was only found for the trackpad endpoint, but the checking
has been added to the button endpoint as well for the same reasons.

[1] https://syzkaller.appspot.com/bug?extid=348331f63b034f89b622

Signed-off-by: Javier Carrasco <javier.carrasco.cruz@...il.com>
Reported-and-tested-by: syzbot+348331f63b034f89b622@...kaller.appspotmail.com
---
 drivers/input/mouse/bcm5974.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/input/mouse/bcm5974.c b/drivers/input/mouse/bcm5974.c
index ca150618d32f..eb552bb4751e 100644
--- a/drivers/input/mouse/bcm5974.c
+++ b/drivers/input/mouse/bcm5974.c
@@ -891,6 +891,26 @@ static int bcm5974_resume(struct usb_interface *iface)
 	return error;
 }
 
+static int bcm5974_int_in_endpoint(struct usb_host_interface *iface, int addr)
+{
+	struct usb_endpoint_descriptor *endpoint;
+	bool int_in_endpoint = false;
+	int i;
+
+	for (i = 0; i < iface->desc.bNumEndpoints; i++) {
+		endpoint = &iface->endpoint[i].desc;
+		if (endpoint->bEndpointAddress == addr) {
+			if (usb_endpoint_is_int_in(endpoint))
+				int_in_endpoint = true;
+			break;
+		}
+	}
+	if (!int_in_endpoint)
+		return -ENODEV;
+
+	return 0;
+}
+
 static int bcm5974_probe(struct usb_interface *iface,
 			 const struct usb_device_id *id)
 {
@@ -898,16 +918,31 @@ static int bcm5974_probe(struct usb_interface *iface,
 	const struct bcm5974_config *cfg;
 	struct bcm5974 *dev;
 	struct input_dev *input_dev;
-	int error = -ENOMEM;
+	int error;
 
 	/* find the product index */
 	cfg = bcm5974_get_config(udev);
 
+	if (cfg->tp_type == TYPE1) {
+		error = bcm5974_int_in_endpoint(iface->cur_altsetting, cfg->bt_ep);
+		if (error) {
+			dev_err(&iface->dev, "No int-in endpoint for the button\n");
+			return error;
+		}
+	}
+
+	error = bcm5974_int_in_endpoint(iface->cur_altsetting, cfg->tp_ep);
+	if (error) {
+		dev_err(&iface->dev, "No int-in endpoint for the trackpad\n");
+		return error;
+	}
+
 	/* allocate memory for our device state and initialize it */
 	dev = kzalloc(sizeof(struct bcm5974), GFP_KERNEL);
 	input_dev = input_allocate_device();
 	if (!dev || !input_dev) {
 		dev_err(&iface->dev, "out of memory\n");
+		error = -ENOMEM;
 		goto err_free_devs;
 	}
 

---
base-commit: b9ddbb0cde2adcedda26045cc58f31316a492215
change-id: 20231007-topic-bcm5974_bulk-c66b743ba7ba

Best regards,
-- 
Javier Carrasco <javier.carrasco.cruz@...il.com>