diff mbox series

Input: bcm5974 - check endpoint type before starting traffic

Message ID 20231007-topic-bcm5974_bulk-v1-1-355be9f8ad80@gmail.com (mailing list archive)
State Superseded
Headers show
Series Input: bcm5974 - check endpoint type before starting traffic | expand

Commit Message

Javier Carrasco Oct. 8, 2023, 7:16 p.m. UTC 
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@gmail.com>
Reported-and-tested-by: syzbot+348331f63b034f89b622@syzkaller.appspotmail.com
---
 drivers/input/mouse/bcm5974.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)


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

Best regards,

Comments

Dan Carpenter Oct. 12, 2023, 6:29 a.m. UTC | #1 
Hi Javier,

kernel test robot noticed the following build warnings:

url:    https://github.com/intel-lab-lkp/linux/commits/Javier-Carrasco/Input-bcm5974-check-endpoint-type-before-starting-traffic/20231009-031809
base:   b9ddbb0cde2adcedda26045cc58f31316a492215
patch link:    https://lore.kernel.org/r/20231007-topic-bcm5974_bulk-v1-1-355be9f8ad80%40gmail.com
patch subject: [PATCH] Input: bcm5974 - check endpoint type before starting traffic
config: x86_64-randconfig-161-20231011 (https://download.01.org/0day-ci/archive/20231012/202310121107.hYslwzCT-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20231012/202310121107.hYslwzCT-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202310121107.hYslwzCT-lkp@intel.com/

smatch warnings:
drivers/input/mouse/bcm5974.c:978 bcm5974_probe() warn: missing error code 'error'

vim +/error +978 drivers/input/mouse/bcm5974.c

f89bd95c5c9467 Henrik Rydberg     2008-08-08   914  static int bcm5974_probe(struct usb_interface *iface,
f89bd95c5c9467 Henrik Rydberg     2008-08-08   915  			 const struct usb_device_id *id)
f89bd95c5c9467 Henrik Rydberg     2008-08-08   916  {
f89bd95c5c9467 Henrik Rydberg     2008-08-08   917  	struct usb_device *udev = interface_to_usbdev(iface);
f89bd95c5c9467 Henrik Rydberg     2008-08-08   918  	const struct bcm5974_config *cfg;
f89bd95c5c9467 Henrik Rydberg     2008-08-08   919  	struct bcm5974 *dev;
f89bd95c5c9467 Henrik Rydberg     2008-08-08   920  	struct input_dev *input_dev;
49b3150e7755c3 Javier Carrasco    2023-10-08   921  	int error;
f89bd95c5c9467 Henrik Rydberg     2008-08-08   922  
f89bd95c5c9467 Henrik Rydberg     2008-08-08   923  	/* find the product index */
f89bd95c5c9467 Henrik Rydberg     2008-08-08   924  	cfg = bcm5974_get_config(udev);
f89bd95c5c9467 Henrik Rydberg     2008-08-08   925  
49b3150e7755c3 Javier Carrasco    2023-10-08   926  	if (cfg->tp_type == TYPE1) {
49b3150e7755c3 Javier Carrasco    2023-10-08   927  		error = bcm5974_int_in_endpoint(iface->cur_altsetting, cfg->bt_ep);
49b3150e7755c3 Javier Carrasco    2023-10-08   928  		if (error) {
49b3150e7755c3 Javier Carrasco    2023-10-08   929  			dev_err(&iface->dev, "No int-in endpoint for the button\n");
49b3150e7755c3 Javier Carrasco    2023-10-08   930  			return error;
49b3150e7755c3 Javier Carrasco    2023-10-08   931  		}
49b3150e7755c3 Javier Carrasco    2023-10-08   932  	}
49b3150e7755c3 Javier Carrasco    2023-10-08   933  
49b3150e7755c3 Javier Carrasco    2023-10-08   934  	error = bcm5974_int_in_endpoint(iface->cur_altsetting, cfg->tp_ep);
49b3150e7755c3 Javier Carrasco    2023-10-08   935  	if (error) {
49b3150e7755c3 Javier Carrasco    2023-10-08   936  		dev_err(&iface->dev, "No int-in endpoint for the trackpad\n");
49b3150e7755c3 Javier Carrasco    2023-10-08   937  		return error;
49b3150e7755c3 Javier Carrasco    2023-10-08   938  	}
49b3150e7755c3 Javier Carrasco    2023-10-08   939  
f89bd95c5c9467 Henrik Rydberg     2008-08-08   940  	/* allocate memory for our device state and initialize it */
f89bd95c5c9467 Henrik Rydberg     2008-08-08   941  	dev = kzalloc(sizeof(struct bcm5974), GFP_KERNEL);
f89bd95c5c9467 Henrik Rydberg     2008-08-08   942  	input_dev = input_allocate_device();
f89bd95c5c9467 Henrik Rydberg     2008-08-08   943  	if (!dev || !input_dev) {
6c1d1b246199c7 Greg Kroah-Hartman 2012-04-25   944  		dev_err(&iface->dev, "out of memory\n");
49b3150e7755c3 Javier Carrasco    2023-10-08   945  		error = -ENOMEM;
f89bd95c5c9467 Henrik Rydberg     2008-08-08   946  		goto err_free_devs;
f89bd95c5c9467 Henrik Rydberg     2008-08-08   947  	}
f89bd95c5c9467 Henrik Rydberg     2008-08-08   948  
f89bd95c5c9467 Henrik Rydberg     2008-08-08   949  	dev->udev = udev;
88da765f4d5f59 Dmitry Torokhov    2008-08-08   950  	dev->intf = iface;
f89bd95c5c9467 Henrik Rydberg     2008-08-08   951  	dev->input = input_dev;
f89bd95c5c9467 Henrik Rydberg     2008-08-08   952  	dev->cfg = *cfg;
f89bd95c5c9467 Henrik Rydberg     2008-08-08   953  	mutex_init(&dev->pm_mutex);
f89bd95c5c9467 Henrik Rydberg     2008-08-08   954  
f89bd95c5c9467 Henrik Rydberg     2008-08-08   955  	/* setup urbs */
43f482b48d0322 Henrik Rydberg     2012-08-13   956  	if (cfg->tp_type == TYPE1) {
f89bd95c5c9467 Henrik Rydberg     2008-08-08   957  		dev->bt_urb = usb_alloc_urb(0, GFP_KERNEL);
f89bd95c5c9467 Henrik Rydberg     2008-08-08   958  		if (!dev->bt_urb)
f89bd95c5c9467 Henrik Rydberg     2008-08-08   959  			goto err_free_devs;
43f482b48d0322 Henrik Rydberg     2012-08-13   960  	}
f89bd95c5c9467 Henrik Rydberg     2008-08-08   961  
f89bd95c5c9467 Henrik Rydberg     2008-08-08   962  	dev->tp_urb = usb_alloc_urb(0, GFP_KERNEL);
f89bd95c5c9467 Henrik Rydberg     2008-08-08   963  	if (!dev->tp_urb)
f89bd95c5c9467 Henrik Rydberg     2008-08-08   964  		goto err_free_bt_urb;
f89bd95c5c9467 Henrik Rydberg     2008-08-08   965  
43f482b48d0322 Henrik Rydberg     2012-08-13   966  	if (dev->bt_urb) {
997ea58eb92f99 Daniel Mack        2010-04-12   967  		dev->bt_data = usb_alloc_coherent(dev->udev,
f89bd95c5c9467 Henrik Rydberg     2008-08-08   968  					  dev->cfg.bt_datalen, GFP_KERNEL,
f89bd95c5c9467 Henrik Rydberg     2008-08-08   969  					  &dev->bt_urb->transfer_dma);
f89bd95c5c9467 Henrik Rydberg     2008-08-08   970  		if (!dev->bt_data)
f89bd95c5c9467 Henrik Rydberg     2008-08-08   971  			goto err_free_urb;
43f482b48d0322 Henrik Rydberg     2012-08-13   972  	}
f89bd95c5c9467 Henrik Rydberg     2008-08-08   973  
997ea58eb92f99 Daniel Mack        2010-04-12   974  	dev->tp_data = usb_alloc_coherent(dev->udev,
f89bd95c5c9467 Henrik Rydberg     2008-08-08   975  					  dev->cfg.tp_datalen, GFP_KERNEL,
f89bd95c5c9467 Henrik Rydberg     2008-08-08   976  					  &dev->tp_urb->transfer_dma);
f89bd95c5c9467 Henrik Rydberg     2008-08-08   977  	if (!dev->tp_data)
f89bd95c5c9467 Henrik Rydberg     2008-08-08  @978  		goto err_free_bt_buffer;

error = -ENOMEM;

f89bd95c5c9467 Henrik Rydberg     2008-08-08   979  
c42e65664390be Mathias Nyman      2022-06-07   980  	if (dev->bt_urb) {
f89bd95c5c9467 Henrik Rydberg     2008-08-08   981  		usb_fill_int_urb(dev->bt_urb, udev,
f89bd95c5c9467 Henrik Rydberg     2008-08-08   982  				 usb_rcvintpipe(udev, cfg->bt_ep),
f89bd95c5c9467 Henrik Rydberg     2008-08-08   983  				 dev->bt_data, dev->cfg.bt_datalen,
f89bd95c5c9467 Henrik Rydberg     2008-08-08   984  				 bcm5974_irq_button, dev, 1);
f89bd95c5c9467 Henrik Rydberg     2008-08-08   985  
c42e65664390be Mathias Nyman      2022-06-07   986  		dev->bt_urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
c42e65664390be Mathias Nyman      2022-06-07   987  	}
c42e65664390be Mathias Nyman      2022-06-07   988  
f89bd95c5c9467 Henrik Rydberg     2008-08-08   989  	usb_fill_int_urb(dev->tp_urb, udev,
f89bd95c5c9467 Henrik Rydberg     2008-08-08   990  			 usb_rcvintpipe(udev, cfg->tp_ep),
f89bd95c5c9467 Henrik Rydberg     2008-08-08   991  			 dev->tp_data, dev->cfg.tp_datalen,
f89bd95c5c9467 Henrik Rydberg     2008-08-08   992  			 bcm5974_irq_trackpad, dev, 1);
f89bd95c5c9467 Henrik Rydberg     2008-08-08   993  
c42e65664390be Mathias Nyman      2022-06-07   994  	dev->tp_urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
c42e65664390be Mathias Nyman      2022-06-07   995  
f89bd95c5c9467 Henrik Rydberg     2008-08-08   996  	/* create bcm5974 device */
f89bd95c5c9467 Henrik Rydberg     2008-08-08   997  	usb_make_path(udev, dev->phys, sizeof(dev->phys));
f89bd95c5c9467 Henrik Rydberg     2008-08-08   998  	strlcat(dev->phys, "/input0", sizeof(dev->phys));
f89bd95c5c9467 Henrik Rydberg     2008-08-08   999  
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1000  	input_dev->name = "bcm5974";
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1001  	input_dev->phys = dev->phys;
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1002  	usb_to_input_id(dev->udev, &input_dev->id);
158e928741e58e Henrik Rydberg     2009-04-28  1003  	/* report driver capabilities via the version field */
158e928741e58e Henrik Rydberg     2009-04-28  1004  	input_dev->id.version = cfg->caps;
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1005  	input_dev->dev.parent = &iface->dev;
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1006  
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1007  	input_set_drvdata(input_dev, dev);
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1008  
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1009  	input_dev->open = bcm5974_open;
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1010  	input_dev->close = bcm5974_close;
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1011  
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1012  	setup_events_to_report(input_dev, cfg);
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1013  
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1014  	error = input_register_device(dev->input);
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1015  	if (error)
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1016  		goto err_free_buffer;
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1017  
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1018  	/* save our data pointer in this interface device */
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1019  	usb_set_intfdata(iface, dev);
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1020  
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1021  	return 0;
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1022  
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1023  err_free_buffer:
997ea58eb92f99 Daniel Mack        2010-04-12  1024  	usb_free_coherent(dev->udev, dev->cfg.tp_datalen,
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1025  		dev->tp_data, dev->tp_urb->transfer_dma);
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1026  err_free_bt_buffer:
43f482b48d0322 Henrik Rydberg     2012-08-13  1027  	if (dev->bt_urb)
997ea58eb92f99 Daniel Mack        2010-04-12  1028  		usb_free_coherent(dev->udev, dev->cfg.bt_datalen,
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1029  				  dev->bt_data, dev->bt_urb->transfer_dma);
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1030  err_free_urb:
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1031  	usb_free_urb(dev->tp_urb);
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1032  err_free_bt_urb:
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1033  	usb_free_urb(dev->bt_urb);
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1034  err_free_devs:
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1035  	usb_set_intfdata(iface, NULL);
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1036  	input_free_device(input_dev);
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1037  	kfree(dev);
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1038  	return error;
f89bd95c5c9467 Henrik Rydberg     2008-08-08  1039  }
diff mbox series

Patch

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;
 	}