Received-SPF: pass (google.com: domain of linux-kernel+bounces-91823-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Mon, 04 Mar 2024 23:41:25 -0800
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Javier Carrasco <javier.carrasco.cruz@gmail.com>,
 Henrik Rydberg <rydberg@bitmath.org>
CC: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
 regressions@lists.linux.dev, Jacopo Radice <jacopo.radice@outlook.com>
Subject: =?US-ASCII?Q?Re=3A_=5BPATCH_v2=5D_Revert_=22Input=3A_bcm5974_-_ch?= =?US-ASCII?Q?eck_endpoint_type_before_starting_traffic=22?=
User-Agent: K-9 Mail for Android
In-Reply-To: <20240305-revert_bcm5974_ep_check-v2-1-925ae9b188d9@gmail.com>
References: <20240305-revert_bcm5974_ep_check-v2-1-925ae9b188d9@gmail.com>
Message-ID: <04E6C7D8-395C-4DB3-B331-B79215DE6B40@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain;
 charset=utf-8
Content-Transfer-Encoding: quoted-printable

On March 4, 2024 10:38:33 PM PST, Javier Carrasco <javier=2Ecarrasco=2Ecruz=
@gmail=2Ecom> 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=
=2E
>
>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=2E
>
>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)=2E The driver should claim the
>interfaces it requires, but that is still not the case=2E
>
>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=2E
>
>Note that reverting this patch will probably trigger the syzkaller bug
>at some point=2E
>
>This reverts commit 2b9c3eb32a699acdd4784d6b93743271b4970899=2E
>
>Link: https://syzkaller=2Eappspot=2Ecom/bug?extid=3D348331f63b034f89b622 =
[1]
>Link: https://lore=2Ekernel=2Eorg/linux-input/87sf161jjc=2Ewl-tiwai@suse=
=2Ede/ [2]
>
>Fixes: b516b1b0dfcc ("Revert "Input: bcm5974 - check endpoint type before=
 starting traffic"")


This "fixes" tag looks incorrect=2E The patch fixes itself?

>Reported-by: Jacopo Radice <jacopo=2Eradice@outlook=2Ecom>
>Closes: https://bugzilla=2Esuse=2Ecom/show_bug=2Ecgi?id=3D1220030
>Signed-off-by: Javier Carrasco <javier=2Ecarrasco=2Ecruz@gmail=2Ecom>
>---
>Changes in v2:
>- Add "Reported-by", "Closes" and "Link" tags=2E
>- Use shorter lore link=2E
>- Link to v1: https://lore=2Ekernel=2Eorg/r/20240305-revert_bcm5974_ep_ch=
eck-v1-1-db4f0422588f@gmail=2Ecom
>---
> drivers/input/mouse/bcm5974=2Ec | 20 --------------------
> 1 file changed, 20 deletions(-)
>
>diff --git a/drivers/input/mouse/bcm5974=2Ec b/drivers/input/mouse/bcm597=
4=2Ec
>index 953992b458e9=2E=2Eca150618d32f 100644
>--- a/drivers/input/mouse/bcm5974=2Ec
>+++ b/drivers/input/mouse/bcm5974=2Ec
>@@ -19,7 +19,6 @@
>  * Copyright (C) 2006	   Nicolas Boichat (nicolas@boichat=2Ech)
>  */
>=20
>-#include "linux/usb=2Eh"
> #include <linux/kernel=2Eh>
> #include <linux/errno=2Eh>
> #include <linux/slab=2Eh>
>@@ -194,8 +193,6 @@ enum tp_type {
>=20
> /* list of device capability bits */
> #define HAS_INTEGRATED_BUTTON	1
>-/* maximum number of supported endpoints (currently trackpad and button)=
 */
>-#define MAX_ENDPOINTS	2
>=20
> /* trackpad finger data block size */
> #define FSIZE_TYPE1		(14 * sizeof(__le16))
>@@ -894,18 +891,6 @@ static int bcm5974_resume(struct usb_interface *ifac=
e)
> 	return error;
> }
>=20
>-static bool bcm5974_check_endpoints(struct usb_interface *iface,
>-				    const struct bcm5974_config *cfg)
>-{
>-	u8 ep_addr[MAX_ENDPOINTS + 1] =3D {0};
>-
>-	ep_addr[0] =3D cfg->tp_ep;
>-	if (cfg->tp_type =3D=3D TYPE1)
>-		ep_addr[1] =3D 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 =3D bcm5974_get_config(udev);
>=20
>-	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 =3D kzalloc(sizeof(struct bcm5974), GFP_KERNEL);
> 	input_dev =3D input_allocate_device();
>
>---
>base-commit: 90d35da658da8cff0d4ecbb5113f5fac9d00eb72
>change-id: 20240305-revert_bcm5974_ep_check-37f2a6ab2714
>
>Best regards,


--=20
Dmitry