On March 4, 2024 10:38:33 PM PST, Javier Carrasco <javier.carrasco.cruz@xxxxxxxxx> wrote: >This patch intended to fix an well-knonw issue in old drivers where the >endpoint type is taken for granted, which is often triggered by fuzzers. > >That was the case for this driver [1], and although the fix seems to be >correct, it uncovered another issue that leads to a regression [2], if >the endpoints of the current interface are checked. > >The driver makes use of endpoints that belong to a different interface >rather than the one it binds (it binds to the third interface, but also >accesses an endpoint from a different one). The driver should claim the >interfaces it requires, but that is still not the case. > >Given that the regression is more severe than the issue found by >syzkaller, the best approach is reverting the patch that causes the >regression, and trying to fix the underlying problem before checking >the endpoint types again. > >Note that reverting this patch will probably trigger the syzkaller bug >at some point. > >This reverts commit 2b9c3eb32a699acdd4784d6b93743271b4970899. > >Link: https://syzkaller.appspot.com/bug?extid=348331f63b034f89b622 [1] >Link: https://lore.kernel.org/linux-input/87sf161jjc.wl-tiwai@xxxxxxx/ [2] > >Fixes: b516b1b0dfcc ("Revert "Input: bcm5974 - check endpoint type before starting traffic"") This "fixes" tag looks incorrect. The patch fixes itself? >Reported-by: Jacopo Radice <jacopo.radice@xxxxxxxxxxx> >Closes: https://bugzilla.suse.com/show_bug.cgi?id=1220030 >Signed-off-by: Javier Carrasco <javier.carrasco.cruz@xxxxxxxxx> >--- >Changes in v2: >- Add "Reported-by", "Closes" and "Link" tags. >- Use shorter lore link. >- Link to v1: https://lore.kernel.org/r/20240305-revert_bcm5974_ep_check-v1-1-db4f0422588f@xxxxxxxxx >--- > drivers/input/mouse/bcm5974.c | 20 -------------------- > 1 file changed, 20 deletions(-) > >diff --git a/drivers/input/mouse/bcm5974.c b/drivers/input/mouse/bcm5974.c >index 953992b458e9..ca150618d32f 100644 >--- a/drivers/input/mouse/bcm5974.c >+++ b/drivers/input/mouse/bcm5974.c >@@ -19,7 +19,6 @@ > * Copyright (C) 2006 Nicolas Boichat (nicolas@xxxxxxxxxx) > */ > >-#include "linux/usb.h" > #include <linux/kernel.h> > #include <linux/errno.h> > #include <linux/slab.h> >@@ -194,8 +193,6 @@ enum tp_type { > > /* list of device capability bits */ > #define HAS_INTEGRATED_BUTTON 1 >-/* maximum number of supported endpoints (currently trackpad and button) */ >-#define MAX_ENDPOINTS 2 > > /* trackpad finger data block size */ > #define FSIZE_TYPE1 (14 * sizeof(__le16)) >@@ -894,18 +891,6 @@ static int bcm5974_resume(struct usb_interface *iface) > return error; > } > >-static bool bcm5974_check_endpoints(struct usb_interface *iface, >- const struct bcm5974_config *cfg) >-{ >- u8 ep_addr[MAX_ENDPOINTS + 1] = {0}; >- >- ep_addr[0] = cfg->tp_ep; >- if (cfg->tp_type == TYPE1) >- ep_addr[1] = cfg->bt_ep; >- >- return usb_check_int_endpoints(iface, ep_addr); >-} >- > static int bcm5974_probe(struct usb_interface *iface, > const struct usb_device_id *id) > { >@@ -918,11 +903,6 @@ static int bcm5974_probe(struct usb_interface *iface, > /* find the product index */ > cfg = bcm5974_get_config(udev); > >- if (!bcm5974_check_endpoints(iface, cfg)) { >- dev_err(&iface->dev, "Unexpected non-int endpoint\n"); >- return -ENODEV; >- } >- > /* allocate memory for our device state and initialize it */ > dev = kzalloc(sizeof(struct bcm5974), GFP_KERNEL); > input_dev = input_allocate_device(); > >--- >base-commit: 90d35da658da8cff0d4ecbb5113f5fac9d00eb72 >change-id: 20240305-revert_bcm5974_ep_check-37f2a6ab2714 > >Best regards, -- Dmitry
