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: Re: [PATCH v2] Revert "Input: bcm5974 - check endpoint type before starting traffic"
Date: Mon, 04 Mar 2024 23:41:25 -0800 [thread overview]
Message-ID: <04E6C7D8-395C-4DB3-B331-B79215DE6B40@gmail.com> (raw)
In-Reply-To: <20240305-revert_bcm5974_ep_check-v2-1-925ae9b188d9@gmail.com>
On March 4, 2024 10:38:33 PM PST, Javier Carrasco <javier.carrasco.cruz@gmail.com> 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@suse.de/ [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@outlook.com>
>Closes: https://bugzilla.suse.com/show_bug.cgi?id=1220030
>Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
>---
>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@gmail.com
>---
> 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@boichat.ch)
> */
>
>-#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
prev parent reply other threads:[~2024-03-05 7:41 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-05 6:38 [PATCH v2] Revert "Input: bcm5974 - check endpoint type before starting traffic" Javier Carrasco
2024-03-05 7:41 ` Dmitry Torokhov [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=04E6C7D8-395C-4DB3-B331-B79215DE6B40@gmail.com \
--to=dmitry.torokhov@gmail.com \
--cc=jacopo.radice@outlook.com \
--cc=javier.carrasco.cruz@gmail.com \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=regressions@lists.linux.dev \
--cc=rydberg@bitmath.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).