All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 1/3] HID: add hid_is_usb() function to make it simpler for USB detection
@ 2021-12-01 18:35 Greg Kroah-Hartman
  2021-12-01 18:35 ` [PATCH v3 2/3] HID: wacom: fix problems when device is not a valid USB device Greg Kroah-Hartman
  2021-12-01 18:35 ` [PATCH v3 3/3] HID: check for valid USB device for many HID drivers Greg Kroah-Hartman
  0 siblings, 2 replies; 8+ messages in thread
From: Greg Kroah-Hartman @ 2021-12-01 18:35 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires; +Cc: linux-input, Greg Kroah-Hartman

A number of HID drivers already call hid_is_using_ll_driver() but only
for the detection of if this is a USB device or not.  Make this more
obvious by creating hid_is_usb() and calling the function that way.

Also converts the existing hid_is_using_ll_driver() functions to use the
new call.

Cc: Jiri Kosina <jikos@kernel.org>
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: linux-input@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
v3: no change
v2: no change

 drivers/hid/hid-asus.c           | 6 ++----
 drivers/hid/hid-logitech-dj.c    | 2 +-
 drivers/hid/hid-u2fzero.c        | 2 +-
 drivers/hid/hid-uclogic-params.c | 3 +--
 drivers/hid/wacom_sys.c          | 2 +-
 include/linux/hid.h              | 5 +++++
 6 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 5d57214d8dee..c5968fd21d09 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -1028,8 +1028,7 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	if (drvdata->quirks & QUIRK_IS_MULTITOUCH)
 		drvdata->tp = &asus_i2c_tp;
 
-	if ((drvdata->quirks & QUIRK_T100_KEYBOARD) &&
-	    hid_is_using_ll_driver(hdev, &usb_hid_driver)) {
+	if ((drvdata->quirks & QUIRK_T100_KEYBOARD) && hid_is_usb(hdev)) {
 		struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
 
 		if (intf->altsetting->desc.bInterfaceNumber == T100_TPAD_INTF) {
@@ -1057,8 +1056,7 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		drvdata->tp = &asus_t100chi_tp;
 	}
 
-	if ((drvdata->quirks & QUIRK_MEDION_E1239T) &&
-	    hid_is_using_ll_driver(hdev, &usb_hid_driver)) {
+	if ((drvdata->quirks & QUIRK_MEDION_E1239T) && hid_is_usb(hdev)) {
 		struct usb_host_interface *alt =
 			to_usb_interface(hdev->dev.parent)->altsetting;
 
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index a0017b010c34..7106b921b53c 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -1777,7 +1777,7 @@ static int logi_dj_probe(struct hid_device *hdev,
 	case recvr_type_bluetooth:	no_dj_interfaces = 2; break;
 	case recvr_type_dinovo:		no_dj_interfaces = 2; break;
 	}
-	if (hid_is_using_ll_driver(hdev, &usb_hid_driver)) {
+	if (hid_is_usb(hdev)) {
 		intf = to_usb_interface(hdev->dev.parent);
 		if (intf && intf->altsetting->desc.bInterfaceNumber >=
 							no_dj_interfaces) {
diff --git a/drivers/hid/hid-u2fzero.c b/drivers/hid/hid-u2fzero.c
index 31ea7fc69916..ad489caf53ad 100644
--- a/drivers/hid/hid-u2fzero.c
+++ b/drivers/hid/hid-u2fzero.c
@@ -311,7 +311,7 @@ static int u2fzero_probe(struct hid_device *hdev,
 	unsigned int minor;
 	int ret;
 
-	if (!hid_is_using_ll_driver(hdev, &usb_hid_driver))
+	if (!hid_is_usb(hdev))
 		return -EINVAL;
 
 	dev = devm_kzalloc(&hdev->dev, sizeof(*dev), GFP_KERNEL);
diff --git a/drivers/hid/hid-uclogic-params.c b/drivers/hid/hid-uclogic-params.c
index 3d67b748a3b9..adff1bd68d9f 100644
--- a/drivers/hid/hid-uclogic-params.c
+++ b/drivers/hid/hid-uclogic-params.c
@@ -843,8 +843,7 @@ int uclogic_params_init(struct uclogic_params *params,
 	struct uclogic_params p = {0, };
 
 	/* Check arguments */
-	if (params == NULL || hdev == NULL ||
-	    !hid_is_using_ll_driver(hdev, &usb_hid_driver)) {
+	if (params == NULL || hdev == NULL || !hid_is_usb(hdev)) {
 		rc = -EINVAL;
 		goto cleanup;
 	}
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 2717d39600b4..22d73772fbc5 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -2214,7 +2214,7 @@ static void wacom_update_name(struct wacom *wacom, const char *suffix)
 	if ((features->type == HID_GENERIC) && !strcmp("Wacom HID", features->name)) {
 		char *product_name = wacom->hdev->name;
 
-		if (hid_is_using_ll_driver(wacom->hdev, &usb_hid_driver)) {
+		if (hid_is_usb(wacom->hdev)) {
 			struct usb_interface *intf = to_usb_interface(wacom->hdev->dev.parent);
 			struct usb_device *dev = interface_to_usbdev(intf);
 			product_name = dev->product;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 9e067f937dbc..f453be385bd4 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -840,6 +840,11 @@ static inline bool hid_is_using_ll_driver(struct hid_device *hdev,
 	return hdev->ll_driver == driver;
 }
 
+static inline bool hid_is_usb(struct hid_device *hdev)
+{
+	return hid_is_using_ll_driver(hdev, &usb_hid_driver);
+}
+
 #define	PM_HINT_FULLON	1<<5
 #define PM_HINT_NORMAL	1<<1
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 2/3] HID: wacom: fix problems when device is not a valid USB device
  2021-12-01 18:35 [PATCH v3 1/3] HID: add hid_is_usb() function to make it simpler for USB detection Greg Kroah-Hartman
@ 2021-12-01 18:35 ` Greg Kroah-Hartman
  2021-12-01 18:35 ` [PATCH v3 3/3] HID: check for valid USB device for many HID drivers Greg Kroah-Hartman
  1 sibling, 0 replies; 8+ messages in thread
From: Greg Kroah-Hartman @ 2021-12-01 18:35 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires; +Cc: linux-input, Greg Kroah-Hartman

The wacom driver accepts devices of more than just USB types, but some
code paths can cause problems if the device being controlled is not a
USB device due to a lack of checking.  Add the needed checks to ensure
that the USB device accesses are only happening on a "real" USB device,
and not one on some other bus.

Cc: Jiri Kosina <jikos@kernel.org>
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: linux-input@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
v3: no change
v2: split into separate patch
    refined test to not abort probe for any non-USB device as the driver
    works for many other types based on Benjamin's review

 drivers/hid/wacom_sys.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 22d73772fbc5..066c567dbaa2 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -726,7 +726,7 @@ static void wacom_retrieve_hid_descriptor(struct hid_device *hdev,
 	 * Skip the query for this type and modify defaults based on
 	 * interface number.
 	 */
-	if (features->type == WIRELESS) {
+	if (features->type == WIRELESS && intf) {
 		if (intf->cur_altsetting->desc.bInterfaceNumber == 0)
 			features->device_type = WACOM_DEVICETYPE_WL_MONITOR;
 		else
@@ -2451,6 +2451,9 @@ static void wacom_wireless_work(struct work_struct *work)
 
 	wacom_destroy_battery(wacom);
 
+	if (!usbdev)
+		return;
+
 	/* Stylus interface */
 	hdev1 = usb_get_intfdata(usbdev->config->interface[1]);
 	wacom1 = hid_get_drvdata(hdev1);
@@ -2730,8 +2733,6 @@ static void wacom_mode_change_work(struct work_struct *work)
 static int wacom_probe(struct hid_device *hdev,
 		const struct hid_device_id *id)
 {
-	struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
-	struct usb_device *dev = interface_to_usbdev(intf);
 	struct wacom *wacom;
 	struct wacom_wac *wacom_wac;
 	struct wacom_features *features;
@@ -2766,8 +2767,14 @@ static int wacom_probe(struct hid_device *hdev,
 	wacom_wac->hid_data.inputmode = -1;
 	wacom_wac->mode_report = -1;
 
-	wacom->usbdev = dev;
-	wacom->intf = intf;
+	if (hid_is_usb(hdev)) {
+		struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
+		struct usb_device *dev = interface_to_usbdev(intf);
+
+		wacom->usbdev = dev;
+		wacom->intf = intf;
+	}
+
 	mutex_init(&wacom->lock);
 	INIT_DELAYED_WORK(&wacom->init_work, wacom_init_work);
 	INIT_WORK(&wacom->wireless_work, wacom_wireless_work);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 3/3] HID: check for valid USB device for many HID drivers
  2021-12-01 18:35 [PATCH v3 1/3] HID: add hid_is_usb() function to make it simpler for USB detection Greg Kroah-Hartman
  2021-12-01 18:35 ` [PATCH v3 2/3] HID: wacom: fix problems when device is not a valid USB device Greg Kroah-Hartman
@ 2021-12-01 18:35 ` Greg Kroah-Hartman
  2021-12-02  9:08   ` Benjamin Tissoires
  1 sibling, 1 reply; 8+ messages in thread
From: Greg Kroah-Hartman @ 2021-12-01 18:35 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires
  Cc: linux-input, Greg Kroah-Hartman, Michael Zaidman, Stefan Achatz,
	Maxime Coquelin, Alexandre Torgue

Many HID drivers assume that the HID device assigned to them is a USB
device as that was the only way HID devices used to be able to be
created in Linux.  However, with the additional ways that HID devices
can be created for many different bus types, that is no longer true, so
properly check that we have a USB device associated with the HID device
before allowing a driver that makes this assumption to claim it.

Cc: Jiri Kosina <jikos@kernel.org>
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: Michael Zaidman <michael.zaidman@gmail.com>
Cc: Stefan Achatz <erazor_de@users.sourceforge.net>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
Cc: linux-input@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
v3: add check in hid-sony based on fix from Benjamin.
v2: holtek_kbd_probe() changes to test for USB device before calling
    hid_probe() and dereferenced the usb interface pointer based on
    Benjamin's review.

 drivers/hid/hid-chicony.c         |  3 +++
 drivers/hid/hid-corsair.c         |  7 ++++++-
 drivers/hid/hid-elan.c            |  2 +-
 drivers/hid/hid-elo.c             |  3 +++
 drivers/hid/hid-ft260.c           |  3 +++
 drivers/hid/hid-holtek-kbd.c      |  9 +++++++--
 drivers/hid/hid-holtek-mouse.c    |  9 +++++++++
 drivers/hid/hid-lg.c              | 10 ++++++++--
 drivers/hid/hid-prodikeys.c       | 10 ++++++++--
 drivers/hid/hid-roccat-arvo.c     |  3 +++
 drivers/hid/hid-roccat-isku.c     |  3 +++
 drivers/hid/hid-roccat-kone.c     |  3 +++
 drivers/hid/hid-roccat-koneplus.c |  3 +++
 drivers/hid/hid-roccat-konepure.c |  3 +++
 drivers/hid/hid-roccat-kovaplus.c |  3 +++
 drivers/hid/hid-roccat-lua.c      |  3 +++
 drivers/hid/hid-roccat-pyra.c     |  3 +++
 drivers/hid/hid-roccat-ryos.c     |  3 +++
 drivers/hid/hid-roccat-savu.c     |  3 +++
 drivers/hid/hid-samsung.c         |  3 +++
 drivers/hid/hid-sony.c            |  6 +++++-
 drivers/hid/hid-thrustmaster.c    |  3 +++
 drivers/hid/hid-uclogic-core.c    |  3 +++
 23 files changed, 92 insertions(+), 9 deletions(-)

diff --git a/drivers/hid/hid-chicony.c b/drivers/hid/hid-chicony.c
index ca556d39da2a..f04d2aa23efe 100644
--- a/drivers/hid/hid-chicony.c
+++ b/drivers/hid/hid-chicony.c
@@ -114,6 +114,9 @@ static int ch_probe(struct hid_device *hdev, const struct hid_device_id *id)
 {
 	int ret;
 
+	if (!hid_is_usb(hdev))
+		return -EINVAL;
+
 	hdev->quirks |= HID_QUIRK_INPUT_PER_APP;
 	ret = hid_parse(hdev);
 	if (ret) {
diff --git a/drivers/hid/hid-corsair.c b/drivers/hid/hid-corsair.c
index 902a60e249ed..8c895c820b67 100644
--- a/drivers/hid/hid-corsair.c
+++ b/drivers/hid/hid-corsair.c
@@ -553,7 +553,12 @@ static int corsair_probe(struct hid_device *dev, const struct hid_device_id *id)
 	int ret;
 	unsigned long quirks = id->driver_data;
 	struct corsair_drvdata *drvdata;
-	struct usb_interface *usbif = to_usb_interface(dev->dev.parent);
+	struct usb_interface *usbif;
+
+	if (!hid_is_usb(dev))
+		return -EINVAL;
+
+	usbif = to_usb_interface(dev->dev.parent);
 
 	drvdata = devm_kzalloc(&dev->dev, sizeof(struct corsair_drvdata),
 			       GFP_KERNEL);
diff --git a/drivers/hid/hid-elan.c b/drivers/hid/hid-elan.c
index 021049805bb7..3091355d48df 100644
--- a/drivers/hid/hid-elan.c
+++ b/drivers/hid/hid-elan.c
@@ -50,7 +50,7 @@ struct elan_drvdata {
 
 static int is_not_elan_touchpad(struct hid_device *hdev)
 {
-	if (hdev->bus == BUS_USB) {
+	if (hid_is_usb(hdev)) {
 		struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
 
 		return (intf->altsetting->desc.bInterfaceNumber !=
diff --git a/drivers/hid/hid-elo.c b/drivers/hid/hid-elo.c
index 383dfda8c12f..8e960d7b233b 100644
--- a/drivers/hid/hid-elo.c
+++ b/drivers/hid/hid-elo.c
@@ -230,6 +230,9 @@ static int elo_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	int ret;
 	struct usb_device *udev;
 
+	if (!hid_is_usb(hdev))
+		return -EINVAL;
+
 	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 4ef1c3b8094e..183eeb3863b3 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -915,6 +915,9 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	struct ft260_get_chip_version_report version;
 	int ret;
 
+	if (!hid_is_usb(hdev))
+		return -EINVAL;
+
 	dev = devm_kzalloc(&hdev->dev, sizeof(*dev), GFP_KERNEL);
 	if (!dev)
 		return -ENOMEM;
diff --git a/drivers/hid/hid-holtek-kbd.c b/drivers/hid/hid-holtek-kbd.c
index 0a38e8e9bc78..403506b9697e 100644
--- a/drivers/hid/hid-holtek-kbd.c
+++ b/drivers/hid/hid-holtek-kbd.c
@@ -140,12 +140,17 @@ static int holtek_kbd_input_event(struct input_dev *dev, unsigned int type,
 static int holtek_kbd_probe(struct hid_device *hdev,
 		const struct hid_device_id *id)
 {
-	struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
-	int ret = hid_parse(hdev);
+	struct usb_interface *intf;
+	int ret;
+
+	if (!hid_is_usb(hdev))
+		return -EINVAL;
 
+	ret = hid_parse(hdev);
 	if (!ret)
 		ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
 
+	intf = to_usb_interface(hdev->dev.parent);
 	if (!ret && intf->cur_altsetting->desc.bInterfaceNumber == 1) {
 		struct hid_input *hidinput;
 		list_for_each_entry(hidinput, &hdev->inputs, list) {
diff --git a/drivers/hid/hid-holtek-mouse.c b/drivers/hid/hid-holtek-mouse.c
index 195b735b001d..b7172c48ef9f 100644
--- a/drivers/hid/hid-holtek-mouse.c
+++ b/drivers/hid/hid-holtek-mouse.c
@@ -62,6 +62,14 @@ static __u8 *holtek_mouse_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 	return rdesc;
 }
 
+static int holtek_mouse_probe(struct hid_device *hdev,
+			      const struct hid_device_id *id)
+{
+	if (!hid_is_usb(hdev))
+		return -EINVAL;
+	return 0;
+}
+
 static const struct hid_device_id holtek_mouse_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_HOLTEK_ALT,
 			USB_DEVICE_ID_HOLTEK_ALT_MOUSE_A067) },
@@ -83,6 +91,7 @@ static struct hid_driver holtek_mouse_driver = {
 	.name = "holtek_mouse",
 	.id_table = holtek_mouse_devices,
 	.report_fixup = holtek_mouse_report_fixup,
+	.probe = holtek_mouse_probe,
 };
 
 module_hid_driver(holtek_mouse_driver);
diff --git a/drivers/hid/hid-lg.c b/drivers/hid/hid-lg.c
index d40af911df63..fb3f7258009c 100644
--- a/drivers/hid/hid-lg.c
+++ b/drivers/hid/hid-lg.c
@@ -749,12 +749,18 @@ static int lg_raw_event(struct hid_device *hdev, struct hid_report *report,
 
 static int lg_probe(struct hid_device *hdev, const struct hid_device_id *id)
 {
-	struct usb_interface *iface = to_usb_interface(hdev->dev.parent);
-	__u8 iface_num = iface->cur_altsetting->desc.bInterfaceNumber;
+	struct usb_interface *iface;
+	__u8 iface_num;
 	unsigned int connect_mask = HID_CONNECT_DEFAULT;
 	struct lg_drv_data *drv_data;
 	int ret;
 
+	if (!hid_is_usb(hdev))
+		return -EINVAL;
+
+	iface = to_usb_interface(hdev->dev.parent);
+	iface_num = iface->cur_altsetting->desc.bInterfaceNumber;
+
 	/* G29 only work with the 1st interface */
 	if ((hdev->product == USB_DEVICE_ID_LOGITECH_G29_WHEEL) &&
 	    (iface_num != 0)) {
diff --git a/drivers/hid/hid-prodikeys.c b/drivers/hid/hid-prodikeys.c
index 2666af02d5c1..e4e9471d0f1e 100644
--- a/drivers/hid/hid-prodikeys.c
+++ b/drivers/hid/hid-prodikeys.c
@@ -798,12 +798,18 @@ static int pk_raw_event(struct hid_device *hdev, struct hid_report *report,
 static int pk_probe(struct hid_device *hdev, const struct hid_device_id *id)
 {
 	int ret;
-	struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
-	unsigned short ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
+	struct usb_interface *intf;
+	unsigned short ifnum;
 	unsigned long quirks = id->driver_data;
 	struct pk_device *pk;
 	struct pcmidi_snd *pm = NULL;
 
+	if (!hid_is_usb(hdev))
+		return -EINVAL;
+
+	intf = to_usb_interface(hdev->dev.parent);
+	ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
+
 	pk = kzalloc(sizeof(*pk), GFP_KERNEL);
 	if (pk == NULL) {
 		hid_err(hdev, "can't alloc descriptor\n");
diff --git a/drivers/hid/hid-roccat-arvo.c b/drivers/hid/hid-roccat-arvo.c
index 4556d2a50f75..d94ee0539421 100644
--- a/drivers/hid/hid-roccat-arvo.c
+++ b/drivers/hid/hid-roccat-arvo.c
@@ -344,6 +344,9 @@ static int arvo_probe(struct hid_device *hdev,
 {
 	int retval;
 
+	if (!hid_is_usb(hdev))
+		return -EINVAL;
+
 	retval = hid_parse(hdev);
 	if (retval) {
 		hid_err(hdev, "parse failed\n");
diff --git a/drivers/hid/hid-roccat-isku.c b/drivers/hid/hid-roccat-isku.c
index ce5f22519956..e95d59cd8d07 100644
--- a/drivers/hid/hid-roccat-isku.c
+++ b/drivers/hid/hid-roccat-isku.c
@@ -324,6 +324,9 @@ static int isku_probe(struct hid_device *hdev,
 {
 	int retval;
 
+	if (!hid_is_usb(hdev))
+		return -EINVAL;
+
 	retval = hid_parse(hdev);
 	if (retval) {
 		hid_err(hdev, "parse failed\n");
diff --git a/drivers/hid/hid-roccat-kone.c b/drivers/hid/hid-roccat-kone.c
index ea17abc7ad52..76da04801ca9 100644
--- a/drivers/hid/hid-roccat-kone.c
+++ b/drivers/hid/hid-roccat-kone.c
@@ -749,6 +749,9 @@ static int kone_probe(struct hid_device *hdev, const struct hid_device_id *id)
 {
 	int retval;
 
+	if (!hid_is_usb(hdev))
+		return -EINVAL;
+
 	retval = hid_parse(hdev);
 	if (retval) {
 		hid_err(hdev, "parse failed\n");
diff --git a/drivers/hid/hid-roccat-koneplus.c b/drivers/hid/hid-roccat-koneplus.c
index 0316edf8c5bb..1896c69ea512 100644
--- a/drivers/hid/hid-roccat-koneplus.c
+++ b/drivers/hid/hid-roccat-koneplus.c
@@ -431,6 +431,9 @@ static int koneplus_probe(struct hid_device *hdev,
 {
 	int retval;
 
+	if (!hid_is_usb(hdev))
+		return -EINVAL;
+
 	retval = hid_parse(hdev);
 	if (retval) {
 		hid_err(hdev, "parse failed\n");
diff --git a/drivers/hid/hid-roccat-konepure.c b/drivers/hid/hid-roccat-konepure.c
index 5248b3c7cf78..cf8eeb33a125 100644
--- a/drivers/hid/hid-roccat-konepure.c
+++ b/drivers/hid/hid-roccat-konepure.c
@@ -133,6 +133,9 @@ static int konepure_probe(struct hid_device *hdev,
 {
 	int retval;
 
+	if (!hid_is_usb(hdev))
+		return -EINVAL;
+
 	retval = hid_parse(hdev);
 	if (retval) {
 		hid_err(hdev, "parse failed\n");
diff --git a/drivers/hid/hid-roccat-kovaplus.c b/drivers/hid/hid-roccat-kovaplus.c
index 960012881570..6fb9b9563769 100644
--- a/drivers/hid/hid-roccat-kovaplus.c
+++ b/drivers/hid/hid-roccat-kovaplus.c
@@ -501,6 +501,9 @@ static int kovaplus_probe(struct hid_device *hdev,
 {
 	int retval;
 
+	if (!hid_is_usb(hdev))
+		return -EINVAL;
+
 	retval = hid_parse(hdev);
 	if (retval) {
 		hid_err(hdev, "parse failed\n");
diff --git a/drivers/hid/hid-roccat-lua.c b/drivers/hid/hid-roccat-lua.c
index 4a88a76d5c62..d5ddf0d68346 100644
--- a/drivers/hid/hid-roccat-lua.c
+++ b/drivers/hid/hid-roccat-lua.c
@@ -160,6 +160,9 @@ static int lua_probe(struct hid_device *hdev,
 {
 	int retval;
 
+	if (!hid_is_usb(hdev))
+		return -EINVAL;
+
 	retval = hid_parse(hdev);
 	if (retval) {
 		hid_err(hdev, "parse failed\n");
diff --git a/drivers/hid/hid-roccat-pyra.c b/drivers/hid/hid-roccat-pyra.c
index 989927defe8d..4fcc8e7d276f 100644
--- a/drivers/hid/hid-roccat-pyra.c
+++ b/drivers/hid/hid-roccat-pyra.c
@@ -449,6 +449,9 @@ static int pyra_probe(struct hid_device *hdev, const struct hid_device_id *id)
 {
 	int retval;
 
+	if (!hid_is_usb(hdev))
+		return -EINVAL;
+
 	retval = hid_parse(hdev);
 	if (retval) {
 		hid_err(hdev, "parse failed\n");
diff --git a/drivers/hid/hid-roccat-ryos.c b/drivers/hid/hid-roccat-ryos.c
index 3956a6c9c521..5bf1971a2b14 100644
--- a/drivers/hid/hid-roccat-ryos.c
+++ b/drivers/hid/hid-roccat-ryos.c
@@ -141,6 +141,9 @@ static int ryos_probe(struct hid_device *hdev,
 {
 	int retval;
 
+	if (!hid_is_usb(hdev))
+		return -EINVAL;
+
 	retval = hid_parse(hdev);
 	if (retval) {
 		hid_err(hdev, "parse failed\n");
diff --git a/drivers/hid/hid-roccat-savu.c b/drivers/hid/hid-roccat-savu.c
index 818701f7a028..a784bb4ee651 100644
--- a/drivers/hid/hid-roccat-savu.c
+++ b/drivers/hid/hid-roccat-savu.c
@@ -113,6 +113,9 @@ static int savu_probe(struct hid_device *hdev,
 {
 	int retval;
 
+	if (!hid_is_usb(hdev))
+		return -EINVAL;
+
 	retval = hid_parse(hdev);
 	if (retval) {
 		hid_err(hdev, "parse failed\n");
diff --git a/drivers/hid/hid-samsung.c b/drivers/hid/hid-samsung.c
index 2e1c31156eca..cf5992e97094 100644
--- a/drivers/hid/hid-samsung.c
+++ b/drivers/hid/hid-samsung.c
@@ -152,6 +152,9 @@ static int samsung_probe(struct hid_device *hdev,
 	int ret;
 	unsigned int cmask = HID_CONNECT_DEFAULT;
 
+	if (!hid_is_usb(hdev))
+		return -EINVAL;
+
 	ret = hid_parse(hdev);
 	if (ret) {
 		hid_err(hdev, "parse failed\n");
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index d1b107d547f5..c186af552129 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -3000,7 +3000,6 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	sc->quirks = quirks;
 	hid_set_drvdata(hdev, sc);
 	sc->hdev = hdev;
-	usbdev = to_usb_device(sc->hdev->dev.parent->parent);
 
 	ret = hid_parse(hdev);
 	if (ret) {
@@ -3043,6 +3042,11 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	}
 
 	if (sc->quirks & (GHL_GUITAR_PS3WIIU | GHL_GUITAR_PS4)) {
+		if (!hid_is_usb(hdev))
+			return -EINVAL;
+
+		usbdev = to_usb_device(sc->hdev->dev.parent->parent);
+
 		sc->ghl_urb = usb_alloc_urb(0, GFP_ATOMIC);
 		if (!sc->ghl_urb)
 			return -ENOMEM;
diff --git a/drivers/hid/hid-thrustmaster.c b/drivers/hid/hid-thrustmaster.c
index d44550aa8805..0c92b7f9b8b8 100644
--- a/drivers/hid/hid-thrustmaster.c
+++ b/drivers/hid/hid-thrustmaster.c
@@ -274,6 +274,9 @@ static int thrustmaster_probe(struct hid_device *hdev, const struct hid_device_i
 	int ret = 0;
 	struct tm_wheel *tm_wheel = 0;
 
+	if (!hid_is_usb(hdev))
+		return -EINVAL;
+
 	ret = hid_parse(hdev);
 	if (ret) {
 		hid_err(hdev, "parse failed with error %d\n", ret);
diff --git a/drivers/hid/hid-uclogic-core.c b/drivers/hid/hid-uclogic-core.c
index 6a9865dd703c..d8ab0139e5cd 100644
--- a/drivers/hid/hid-uclogic-core.c
+++ b/drivers/hid/hid-uclogic-core.c
@@ -164,6 +164,9 @@ static int uclogic_probe(struct hid_device *hdev,
 	struct uclogic_drvdata *drvdata = NULL;
 	bool params_initialized = false;
 
+	if (!hid_is_usb(hdev))
+		return -EINVAL;
+
 	/*
 	 * libinput requires the pad interface to be on a different node
 	 * than the pen, so use QUIRK_MULTI_INPUT for all tablets.
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 3/3] HID: check for valid USB device for many HID drivers
  2021-12-01 18:35 ` [PATCH v3 3/3] HID: check for valid USB device for many HID drivers Greg Kroah-Hartman
@ 2021-12-02  9:08   ` Benjamin Tissoires
  2021-12-02  9:25     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 8+ messages in thread
From: Benjamin Tissoires @ 2021-12-02  9:08 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Jiri Kosina, open list:HID CORE LAYER, Michael Zaidman,
	Stefan Achatz, Maxime Coquelin, Alexandre Torgue

On Wed, Dec 1, 2021 at 7:35 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> Many HID drivers assume that the HID device assigned to them is a USB
> device as that was the only way HID devices used to be able to be
> created in Linux.  However, with the additional ways that HID devices
> can be created for many different bus types, that is no longer true, so
> properly check that we have a USB device associated with the HID device
> before allowing a driver that makes this assumption to claim it.
>
> Cc: Jiri Kosina <jikos@kernel.org>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Cc: Michael Zaidman <michael.zaidman@gmail.com>
> Cc: Stefan Achatz <erazor_de@users.sourceforge.net>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
> Cc: linux-input@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> v3: add check in hid-sony based on fix from Benjamin.

Sigh. This morning I realized my fix was incomplete as the error path
of the GHL devices handling is not releasing the hidraw node.
I have a followup patch ready that I am about to send as soon as I get
the full tests run.

Also, I guess we want this series to land in 5.16-rc4 or rc5 and mark
it as stable, no?
If so, I'll add the cc stable@vger.kernel.org marks so you don't need
to keep tabs on it.

Besides that, I tested the wacom change on an I2C screen, and it is
still working as expected, so I think the wacom changes are safe.
The full series is
Tested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Cheers,
Benjamin

> v2: holtek_kbd_probe() changes to test for USB device before calling
>     hid_probe() and dereferenced the usb interface pointer based on
>     Benjamin's review.
>
>  drivers/hid/hid-chicony.c         |  3 +++
>  drivers/hid/hid-corsair.c         |  7 ++++++-
>  drivers/hid/hid-elan.c            |  2 +-
>  drivers/hid/hid-elo.c             |  3 +++
>  drivers/hid/hid-ft260.c           |  3 +++
>  drivers/hid/hid-holtek-kbd.c      |  9 +++++++--
>  drivers/hid/hid-holtek-mouse.c    |  9 +++++++++
>  drivers/hid/hid-lg.c              | 10 ++++++++--
>  drivers/hid/hid-prodikeys.c       | 10 ++++++++--
>  drivers/hid/hid-roccat-arvo.c     |  3 +++
>  drivers/hid/hid-roccat-isku.c     |  3 +++
>  drivers/hid/hid-roccat-kone.c     |  3 +++
>  drivers/hid/hid-roccat-koneplus.c |  3 +++
>  drivers/hid/hid-roccat-konepure.c |  3 +++
>  drivers/hid/hid-roccat-kovaplus.c |  3 +++
>  drivers/hid/hid-roccat-lua.c      |  3 +++
>  drivers/hid/hid-roccat-pyra.c     |  3 +++
>  drivers/hid/hid-roccat-ryos.c     |  3 +++
>  drivers/hid/hid-roccat-savu.c     |  3 +++
>  drivers/hid/hid-samsung.c         |  3 +++
>  drivers/hid/hid-sony.c            |  6 +++++-
>  drivers/hid/hid-thrustmaster.c    |  3 +++
>  drivers/hid/hid-uclogic-core.c    |  3 +++
>  23 files changed, 92 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/hid/hid-chicony.c b/drivers/hid/hid-chicony.c
> index ca556d39da2a..f04d2aa23efe 100644
> --- a/drivers/hid/hid-chicony.c
> +++ b/drivers/hid/hid-chicony.c
> @@ -114,6 +114,9 @@ static int ch_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  {
>         int ret;
>
> +       if (!hid_is_usb(hdev))
> +               return -EINVAL;
> +
>         hdev->quirks |= HID_QUIRK_INPUT_PER_APP;
>         ret = hid_parse(hdev);
>         if (ret) {
> diff --git a/drivers/hid/hid-corsair.c b/drivers/hid/hid-corsair.c
> index 902a60e249ed..8c895c820b67 100644
> --- a/drivers/hid/hid-corsair.c
> +++ b/drivers/hid/hid-corsair.c
> @@ -553,7 +553,12 @@ static int corsair_probe(struct hid_device *dev, const struct hid_device_id *id)
>         int ret;
>         unsigned long quirks = id->driver_data;
>         struct corsair_drvdata *drvdata;
> -       struct usb_interface *usbif = to_usb_interface(dev->dev.parent);
> +       struct usb_interface *usbif;
> +
> +       if (!hid_is_usb(dev))
> +               return -EINVAL;
> +
> +       usbif = to_usb_interface(dev->dev.parent);
>
>         drvdata = devm_kzalloc(&dev->dev, sizeof(struct corsair_drvdata),
>                                GFP_KERNEL);
> diff --git a/drivers/hid/hid-elan.c b/drivers/hid/hid-elan.c
> index 021049805bb7..3091355d48df 100644
> --- a/drivers/hid/hid-elan.c
> +++ b/drivers/hid/hid-elan.c
> @@ -50,7 +50,7 @@ struct elan_drvdata {
>
>  static int is_not_elan_touchpad(struct hid_device *hdev)
>  {
> -       if (hdev->bus == BUS_USB) {
> +       if (hid_is_usb(hdev)) {
>                 struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
>
>                 return (intf->altsetting->desc.bInterfaceNumber !=
> diff --git a/drivers/hid/hid-elo.c b/drivers/hid/hid-elo.c
> index 383dfda8c12f..8e960d7b233b 100644
> --- a/drivers/hid/hid-elo.c
> +++ b/drivers/hid/hid-elo.c
> @@ -230,6 +230,9 @@ static int elo_probe(struct hid_device *hdev, const struct hid_device_id *id)
>         int ret;
>         struct usb_device *udev;
>
> +       if (!hid_is_usb(hdev))
> +               return -EINVAL;
> +
>         priv = kzalloc(sizeof(*priv), GFP_KERNEL);
>         if (!priv)
>                 return -ENOMEM;
> diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
> index 4ef1c3b8094e..183eeb3863b3 100644
> --- a/drivers/hid/hid-ft260.c
> +++ b/drivers/hid/hid-ft260.c
> @@ -915,6 +915,9 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
>         struct ft260_get_chip_version_report version;
>         int ret;
>
> +       if (!hid_is_usb(hdev))
> +               return -EINVAL;
> +
>         dev = devm_kzalloc(&hdev->dev, sizeof(*dev), GFP_KERNEL);
>         if (!dev)
>                 return -ENOMEM;
> diff --git a/drivers/hid/hid-holtek-kbd.c b/drivers/hid/hid-holtek-kbd.c
> index 0a38e8e9bc78..403506b9697e 100644
> --- a/drivers/hid/hid-holtek-kbd.c
> +++ b/drivers/hid/hid-holtek-kbd.c
> @@ -140,12 +140,17 @@ static int holtek_kbd_input_event(struct input_dev *dev, unsigned int type,
>  static int holtek_kbd_probe(struct hid_device *hdev,
>                 const struct hid_device_id *id)
>  {
> -       struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
> -       int ret = hid_parse(hdev);
> +       struct usb_interface *intf;
> +       int ret;
> +
> +       if (!hid_is_usb(hdev))
> +               return -EINVAL;
>
> +       ret = hid_parse(hdev);
>         if (!ret)
>                 ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
>
> +       intf = to_usb_interface(hdev->dev.parent);
>         if (!ret && intf->cur_altsetting->desc.bInterfaceNumber == 1) {
>                 struct hid_input *hidinput;
>                 list_for_each_entry(hidinput, &hdev->inputs, list) {
> diff --git a/drivers/hid/hid-holtek-mouse.c b/drivers/hid/hid-holtek-mouse.c
> index 195b735b001d..b7172c48ef9f 100644
> --- a/drivers/hid/hid-holtek-mouse.c
> +++ b/drivers/hid/hid-holtek-mouse.c
> @@ -62,6 +62,14 @@ static __u8 *holtek_mouse_report_fixup(struct hid_device *hdev, __u8 *rdesc,
>         return rdesc;
>  }
>
> +static int holtek_mouse_probe(struct hid_device *hdev,
> +                             const struct hid_device_id *id)
> +{
> +       if (!hid_is_usb(hdev))
> +               return -EINVAL;
> +       return 0;
> +}
> +
>  static const struct hid_device_id holtek_mouse_devices[] = {
>         { HID_USB_DEVICE(USB_VENDOR_ID_HOLTEK_ALT,
>                         USB_DEVICE_ID_HOLTEK_ALT_MOUSE_A067) },
> @@ -83,6 +91,7 @@ static struct hid_driver holtek_mouse_driver = {
>         .name = "holtek_mouse",
>         .id_table = holtek_mouse_devices,
>         .report_fixup = holtek_mouse_report_fixup,
> +       .probe = holtek_mouse_probe,
>  };
>
>  module_hid_driver(holtek_mouse_driver);
> diff --git a/drivers/hid/hid-lg.c b/drivers/hid/hid-lg.c
> index d40af911df63..fb3f7258009c 100644
> --- a/drivers/hid/hid-lg.c
> +++ b/drivers/hid/hid-lg.c
> @@ -749,12 +749,18 @@ static int lg_raw_event(struct hid_device *hdev, struct hid_report *report,
>
>  static int lg_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  {
> -       struct usb_interface *iface = to_usb_interface(hdev->dev.parent);
> -       __u8 iface_num = iface->cur_altsetting->desc.bInterfaceNumber;
> +       struct usb_interface *iface;
> +       __u8 iface_num;
>         unsigned int connect_mask = HID_CONNECT_DEFAULT;
>         struct lg_drv_data *drv_data;
>         int ret;
>
> +       if (!hid_is_usb(hdev))
> +               return -EINVAL;
> +
> +       iface = to_usb_interface(hdev->dev.parent);
> +       iface_num = iface->cur_altsetting->desc.bInterfaceNumber;
> +
>         /* G29 only work with the 1st interface */
>         if ((hdev->product == USB_DEVICE_ID_LOGITECH_G29_WHEEL) &&
>             (iface_num != 0)) {
> diff --git a/drivers/hid/hid-prodikeys.c b/drivers/hid/hid-prodikeys.c
> index 2666af02d5c1..e4e9471d0f1e 100644
> --- a/drivers/hid/hid-prodikeys.c
> +++ b/drivers/hid/hid-prodikeys.c
> @@ -798,12 +798,18 @@ static int pk_raw_event(struct hid_device *hdev, struct hid_report *report,
>  static int pk_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  {
>         int ret;
> -       struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
> -       unsigned short ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
> +       struct usb_interface *intf;
> +       unsigned short ifnum;
>         unsigned long quirks = id->driver_data;
>         struct pk_device *pk;
>         struct pcmidi_snd *pm = NULL;
>
> +       if (!hid_is_usb(hdev))
> +               return -EINVAL;
> +
> +       intf = to_usb_interface(hdev->dev.parent);
> +       ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
> +
>         pk = kzalloc(sizeof(*pk), GFP_KERNEL);
>         if (pk == NULL) {
>                 hid_err(hdev, "can't alloc descriptor\n");
> diff --git a/drivers/hid/hid-roccat-arvo.c b/drivers/hid/hid-roccat-arvo.c
> index 4556d2a50f75..d94ee0539421 100644
> --- a/drivers/hid/hid-roccat-arvo.c
> +++ b/drivers/hid/hid-roccat-arvo.c
> @@ -344,6 +344,9 @@ static int arvo_probe(struct hid_device *hdev,
>  {
>         int retval;
>
> +       if (!hid_is_usb(hdev))
> +               return -EINVAL;
> +
>         retval = hid_parse(hdev);
>         if (retval) {
>                 hid_err(hdev, "parse failed\n");
> diff --git a/drivers/hid/hid-roccat-isku.c b/drivers/hid/hid-roccat-isku.c
> index ce5f22519956..e95d59cd8d07 100644
> --- a/drivers/hid/hid-roccat-isku.c
> +++ b/drivers/hid/hid-roccat-isku.c
> @@ -324,6 +324,9 @@ static int isku_probe(struct hid_device *hdev,
>  {
>         int retval;
>
> +       if (!hid_is_usb(hdev))
> +               return -EINVAL;
> +
>         retval = hid_parse(hdev);
>         if (retval) {
>                 hid_err(hdev, "parse failed\n");
> diff --git a/drivers/hid/hid-roccat-kone.c b/drivers/hid/hid-roccat-kone.c
> index ea17abc7ad52..76da04801ca9 100644
> --- a/drivers/hid/hid-roccat-kone.c
> +++ b/drivers/hid/hid-roccat-kone.c
> @@ -749,6 +749,9 @@ static int kone_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  {
>         int retval;
>
> +       if (!hid_is_usb(hdev))
> +               return -EINVAL;
> +
>         retval = hid_parse(hdev);
>         if (retval) {
>                 hid_err(hdev, "parse failed\n");
> diff --git a/drivers/hid/hid-roccat-koneplus.c b/drivers/hid/hid-roccat-koneplus.c
> index 0316edf8c5bb..1896c69ea512 100644
> --- a/drivers/hid/hid-roccat-koneplus.c
> +++ b/drivers/hid/hid-roccat-koneplus.c
> @@ -431,6 +431,9 @@ static int koneplus_probe(struct hid_device *hdev,
>  {
>         int retval;
>
> +       if (!hid_is_usb(hdev))
> +               return -EINVAL;
> +
>         retval = hid_parse(hdev);
>         if (retval) {
>                 hid_err(hdev, "parse failed\n");
> diff --git a/drivers/hid/hid-roccat-konepure.c b/drivers/hid/hid-roccat-konepure.c
> index 5248b3c7cf78..cf8eeb33a125 100644
> --- a/drivers/hid/hid-roccat-konepure.c
> +++ b/drivers/hid/hid-roccat-konepure.c
> @@ -133,6 +133,9 @@ static int konepure_probe(struct hid_device *hdev,
>  {
>         int retval;
>
> +       if (!hid_is_usb(hdev))
> +               return -EINVAL;
> +
>         retval = hid_parse(hdev);
>         if (retval) {
>                 hid_err(hdev, "parse failed\n");
> diff --git a/drivers/hid/hid-roccat-kovaplus.c b/drivers/hid/hid-roccat-kovaplus.c
> index 960012881570..6fb9b9563769 100644
> --- a/drivers/hid/hid-roccat-kovaplus.c
> +++ b/drivers/hid/hid-roccat-kovaplus.c
> @@ -501,6 +501,9 @@ static int kovaplus_probe(struct hid_device *hdev,
>  {
>         int retval;
>
> +       if (!hid_is_usb(hdev))
> +               return -EINVAL;
> +
>         retval = hid_parse(hdev);
>         if (retval) {
>                 hid_err(hdev, "parse failed\n");
> diff --git a/drivers/hid/hid-roccat-lua.c b/drivers/hid/hid-roccat-lua.c
> index 4a88a76d5c62..d5ddf0d68346 100644
> --- a/drivers/hid/hid-roccat-lua.c
> +++ b/drivers/hid/hid-roccat-lua.c
> @@ -160,6 +160,9 @@ static int lua_probe(struct hid_device *hdev,
>  {
>         int retval;
>
> +       if (!hid_is_usb(hdev))
> +               return -EINVAL;
> +
>         retval = hid_parse(hdev);
>         if (retval) {
>                 hid_err(hdev, "parse failed\n");
> diff --git a/drivers/hid/hid-roccat-pyra.c b/drivers/hid/hid-roccat-pyra.c
> index 989927defe8d..4fcc8e7d276f 100644
> --- a/drivers/hid/hid-roccat-pyra.c
> +++ b/drivers/hid/hid-roccat-pyra.c
> @@ -449,6 +449,9 @@ static int pyra_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  {
>         int retval;
>
> +       if (!hid_is_usb(hdev))
> +               return -EINVAL;
> +
>         retval = hid_parse(hdev);
>         if (retval) {
>                 hid_err(hdev, "parse failed\n");
> diff --git a/drivers/hid/hid-roccat-ryos.c b/drivers/hid/hid-roccat-ryos.c
> index 3956a6c9c521..5bf1971a2b14 100644
> --- a/drivers/hid/hid-roccat-ryos.c
> +++ b/drivers/hid/hid-roccat-ryos.c
> @@ -141,6 +141,9 @@ static int ryos_probe(struct hid_device *hdev,
>  {
>         int retval;
>
> +       if (!hid_is_usb(hdev))
> +               return -EINVAL;
> +
>         retval = hid_parse(hdev);
>         if (retval) {
>                 hid_err(hdev, "parse failed\n");
> diff --git a/drivers/hid/hid-roccat-savu.c b/drivers/hid/hid-roccat-savu.c
> index 818701f7a028..a784bb4ee651 100644
> --- a/drivers/hid/hid-roccat-savu.c
> +++ b/drivers/hid/hid-roccat-savu.c
> @@ -113,6 +113,9 @@ static int savu_probe(struct hid_device *hdev,
>  {
>         int retval;
>
> +       if (!hid_is_usb(hdev))
> +               return -EINVAL;
> +
>         retval = hid_parse(hdev);
>         if (retval) {
>                 hid_err(hdev, "parse failed\n");
> diff --git a/drivers/hid/hid-samsung.c b/drivers/hid/hid-samsung.c
> index 2e1c31156eca..cf5992e97094 100644
> --- a/drivers/hid/hid-samsung.c
> +++ b/drivers/hid/hid-samsung.c
> @@ -152,6 +152,9 @@ static int samsung_probe(struct hid_device *hdev,
>         int ret;
>         unsigned int cmask = HID_CONNECT_DEFAULT;
>
> +       if (!hid_is_usb(hdev))
> +               return -EINVAL;
> +
>         ret = hid_parse(hdev);
>         if (ret) {
>                 hid_err(hdev, "parse failed\n");
> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index d1b107d547f5..c186af552129 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -3000,7 +3000,6 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
>         sc->quirks = quirks;
>         hid_set_drvdata(hdev, sc);
>         sc->hdev = hdev;
> -       usbdev = to_usb_device(sc->hdev->dev.parent->parent);
>
>         ret = hid_parse(hdev);
>         if (ret) {
> @@ -3043,6 +3042,11 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
>         }
>
>         if (sc->quirks & (GHL_GUITAR_PS3WIIU | GHL_GUITAR_PS4)) {
> +               if (!hid_is_usb(hdev))
> +                       return -EINVAL;
> +
> +               usbdev = to_usb_device(sc->hdev->dev.parent->parent);
> +
>                 sc->ghl_urb = usb_alloc_urb(0, GFP_ATOMIC);
>                 if (!sc->ghl_urb)
>                         return -ENOMEM;
> diff --git a/drivers/hid/hid-thrustmaster.c b/drivers/hid/hid-thrustmaster.c
> index d44550aa8805..0c92b7f9b8b8 100644
> --- a/drivers/hid/hid-thrustmaster.c
> +++ b/drivers/hid/hid-thrustmaster.c
> @@ -274,6 +274,9 @@ static int thrustmaster_probe(struct hid_device *hdev, const struct hid_device_i
>         int ret = 0;
>         struct tm_wheel *tm_wheel = 0;
>
> +       if (!hid_is_usb(hdev))
> +               return -EINVAL;
> +
>         ret = hid_parse(hdev);
>         if (ret) {
>                 hid_err(hdev, "parse failed with error %d\n", ret);
> diff --git a/drivers/hid/hid-uclogic-core.c b/drivers/hid/hid-uclogic-core.c
> index 6a9865dd703c..d8ab0139e5cd 100644
> --- a/drivers/hid/hid-uclogic-core.c
> +++ b/drivers/hid/hid-uclogic-core.c
> @@ -164,6 +164,9 @@ static int uclogic_probe(struct hid_device *hdev,
>         struct uclogic_drvdata *drvdata = NULL;
>         bool params_initialized = false;
>
> +       if (!hid_is_usb(hdev))
> +               return -EINVAL;
> +
>         /*
>          * libinput requires the pad interface to be on a different node
>          * than the pen, so use QUIRK_MULTI_INPUT for all tablets.
> --
> 2.34.1
>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 3/3] HID: check for valid USB device for many HID drivers
  2021-12-02  9:08   ` Benjamin Tissoires
@ 2021-12-02  9:25     ` Greg Kroah-Hartman
  2021-12-02 11:54       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 8+ messages in thread
From: Greg Kroah-Hartman @ 2021-12-02  9:25 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Jiri Kosina, open list:HID CORE LAYER, Michael Zaidman,
	Stefan Achatz, Maxime Coquelin, Alexandre Torgue

On Thu, Dec 02, 2021 at 10:08:07AM +0100, Benjamin Tissoires wrote:
> On Wed, Dec 1, 2021 at 7:35 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > Many HID drivers assume that the HID device assigned to them is a USB
> > device as that was the only way HID devices used to be able to be
> > created in Linux.  However, with the additional ways that HID devices
> > can be created for many different bus types, that is no longer true, so
> > properly check that we have a USB device associated with the HID device
> > before allowing a driver that makes this assumption to claim it.
> >
> > Cc: Jiri Kosina <jikos@kernel.org>
> > Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > Cc: Michael Zaidman <michael.zaidman@gmail.com>
> > Cc: Stefan Achatz <erazor_de@users.sourceforge.net>
> > Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> > Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
> > Cc: linux-input@vger.kernel.org
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> > v3: add check in hid-sony based on fix from Benjamin.
> 
> Sigh. This morning I realized my fix was incomplete as the error path
> of the GHL devices handling is not releasing the hidraw node.
> I have a followup patch ready that I am about to send as soon as I get
> the full tests run.
> 
> Also, I guess we want this series to land in 5.16-rc4 or rc5 and mark
> it as stable, no?
> If so, I'll add the cc stable@vger.kernel.org marks so you don't need
> to keep tabs on it.

That would be great, thanks!

> Besides that, I tested the wacom change on an I2C screen, and it is
> still working as expected, so I think the wacom changes are safe.
> The full series is
> Tested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Wonderful.

Note, I did just get some really odd 0-day error messages from this
patch series:
	https://lore.kernel.org/r/202112021000.3GKuE8mS-lkp@intel.com
	https://lore.kernel.org/r/202112020808.AZ5usuXj-lkp@intel.com

Which looks like a dependancy issue that might have always been there.
I'll work on resolving that after lunch...

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 3/3] HID: check for valid USB device for many HID drivers
  2021-12-02  9:25     ` Greg Kroah-Hartman
@ 2021-12-02 11:54       ` Greg Kroah-Hartman
  2021-12-02 15:21         ` Benjamin Tissoires
  0 siblings, 1 reply; 8+ messages in thread
From: Greg Kroah-Hartman @ 2021-12-02 11:54 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Jiri Kosina, open list:HID CORE LAYER, Michael Zaidman,
	Stefan Achatz, Maxime Coquelin, Alexandre Torgue

On Thu, Dec 02, 2021 at 10:25:46AM +0100, Greg Kroah-Hartman wrote:
> On Thu, Dec 02, 2021 at 10:08:07AM +0100, Benjamin Tissoires wrote:
> > On Wed, Dec 1, 2021 at 7:35 PM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > Many HID drivers assume that the HID device assigned to them is a USB
> > > device as that was the only way HID devices used to be able to be
> > > created in Linux.  However, with the additional ways that HID devices
> > > can be created for many different bus types, that is no longer true, so
> > > properly check that we have a USB device associated with the HID device
> > > before allowing a driver that makes this assumption to claim it.
> > >
> > > Cc: Jiri Kosina <jikos@kernel.org>
> > > Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > > Cc: Michael Zaidman <michael.zaidman@gmail.com>
> > > Cc: Stefan Achatz <erazor_de@users.sourceforge.net>
> > > Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> > > Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
> > > Cc: linux-input@vger.kernel.org
> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > ---
> > > v3: add check in hid-sony based on fix from Benjamin.
> > 
> > Sigh. This morning I realized my fix was incomplete as the error path
> > of the GHL devices handling is not releasing the hidraw node.
> > I have a followup patch ready that I am about to send as soon as I get
> > the full tests run.
> > 
> > Also, I guess we want this series to land in 5.16-rc4 or rc5 and mark
> > it as stable, no?
> > If so, I'll add the cc stable@vger.kernel.org marks so you don't need
> > to keep tabs on it.
> 
> That would be great, thanks!
> 
> > Besides that, I tested the wacom change on an I2C screen, and it is
> > still working as expected, so I think the wacom changes are safe.
> > The full series is
> > Tested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> 
> Wonderful.
> 
> Note, I did just get some really odd 0-day error messages from this
> patch series:
> 	https://lore.kernel.org/r/202112021000.3GKuE8mS-lkp@intel.com
> 	https://lore.kernel.org/r/202112020808.AZ5usuXj-lkp@intel.com
> 
> Which looks like a dependancy issue that might have always been there.
> I'll work on resolving that after lunch...

Now sent:
	https://lore.kernel.org/r/20211202114819.2511954-1-gregkh@linuxfoundation.org

I can resend it as a v4 series that adds this to the end of the series
if you want me to.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 3/3] HID: check for valid USB device for many HID drivers
  2021-12-02 11:54       ` Greg Kroah-Hartman
@ 2021-12-02 15:21         ` Benjamin Tissoires
  2021-12-02 16:47           ` Benjamin Tissoires
  0 siblings, 1 reply; 8+ messages in thread
From: Benjamin Tissoires @ 2021-12-02 15:21 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Jiri Kosina, open list:HID CORE LAYER, Michael Zaidman,
	Stefan Achatz, Maxime Coquelin, Alexandre Torgue

On Thu, Dec 2, 2021 at 12:54 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Thu, Dec 02, 2021 at 10:25:46AM +0100, Greg Kroah-Hartman wrote:
> > On Thu, Dec 02, 2021 at 10:08:07AM +0100, Benjamin Tissoires wrote:
> > > On Wed, Dec 1, 2021 at 7:35 PM Greg Kroah-Hartman
> > > <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > Many HID drivers assume that the HID device assigned to them is a USB
> > > > device as that was the only way HID devices used to be able to be
> > > > created in Linux.  However, with the additional ways that HID devices
> > > > can be created for many different bus types, that is no longer true, so
> > > > properly check that we have a USB device associated with the HID device
> > > > before allowing a driver that makes this assumption to claim it.
> > > >
> > > > Cc: Jiri Kosina <jikos@kernel.org>
> > > > Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > > > Cc: Michael Zaidman <michael.zaidman@gmail.com>
> > > > Cc: Stefan Achatz <erazor_de@users.sourceforge.net>
> > > > Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> > > > Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
> > > > Cc: linux-input@vger.kernel.org
> > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > ---
> > > > v3: add check in hid-sony based on fix from Benjamin.
> > >
> > > Sigh. This morning I realized my fix was incomplete as the error path
> > > of the GHL devices handling is not releasing the hidraw node.
> > > I have a followup patch ready that I am about to send as soon as I get
> > > the full tests run.
> > >
> > > Also, I guess we want this series to land in 5.16-rc4 or rc5 and mark
> > > it as stable, no?
> > > If so, I'll add the cc stable@vger.kernel.org marks so you don't need
> > > to keep tabs on it.
> >
> > That would be great, thanks!
> >
> > > Besides that, I tested the wacom change on an I2C screen, and it is
> > > still working as expected, so I think the wacom changes are safe.
> > > The full series is
> > > Tested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> >
> > Wonderful.
> >
> > Note, I did just get some really odd 0-day error messages from this
> > patch series:
> >       https://lore.kernel.org/r/202112021000.3GKuE8mS-lkp@intel.com
> >       https://lore.kernel.org/r/202112020808.AZ5usuXj-lkp@intel.com
> >
> > Which looks like a dependancy issue that might have always been there.
> > I'll work on resolving that after lunch...
>
> Now sent:
>         https://lore.kernel.org/r/20211202114819.2511954-1-gregkh@linuxfoundation.org

\o/

>
> I can resend it as a v4 series that adds this to the end of the series
> if you want me to.

That's fine. I already locally scheduled the v3, this other series
above and my 2 first patches from my series that you reviewed today.
I also added the cc: stable markers (though I haven't checked exactly
which versions it applies to).

Once my automated suite is happy I'll push them to hid.git.

Thanks for your time Greg!

Cheers,
Benjamin

>
> thanks,
>
> greg k-h
>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 3/3] HID: check for valid USB device for many HID drivers
  2021-12-02 15:21         ` Benjamin Tissoires
@ 2021-12-02 16:47           ` Benjamin Tissoires
  0 siblings, 0 replies; 8+ messages in thread
From: Benjamin Tissoires @ 2021-12-02 16:47 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Jiri Kosina, open list:HID CORE LAYER, Michael Zaidman,
	Stefan Achatz, Maxime Coquelin, Alexandre Torgue

On Thu, Dec 2, 2021 at 4:21 PM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> On Thu, Dec 2, 2021 at 12:54 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Thu, Dec 02, 2021 at 10:25:46AM +0100, Greg Kroah-Hartman wrote:
> > > On Thu, Dec 02, 2021 at 10:08:07AM +0100, Benjamin Tissoires wrote:
> > > > On Wed, Dec 1, 2021 at 7:35 PM Greg Kroah-Hartman
> > > > <gregkh@linuxfoundation.org> wrote:
> > > > >
> > > > > Many HID drivers assume that the HID device assigned to them is a USB
> > > > > device as that was the only way HID devices used to be able to be
> > > > > created in Linux.  However, with the additional ways that HID devices
> > > > > can be created for many different bus types, that is no longer true, so
> > > > > properly check that we have a USB device associated with the HID device
> > > > > before allowing a driver that makes this assumption to claim it.
> > > > >
> > > > > Cc: Jiri Kosina <jikos@kernel.org>
> > > > > Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > > > > Cc: Michael Zaidman <michael.zaidman@gmail.com>
> > > > > Cc: Stefan Achatz <erazor_de@users.sourceforge.net>
> > > > > Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> > > > > Cc: Alexandre Torgue <alexandre.torgue@foss.st.com>
> > > > > Cc: linux-input@vger.kernel.org
> > > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > > ---
> > > > > v3: add check in hid-sony based on fix from Benjamin.
> > > >
> > > > Sigh. This morning I realized my fix was incomplete as the error path
> > > > of the GHL devices handling is not releasing the hidraw node.
> > > > I have a followup patch ready that I am about to send as soon as I get
> > > > the full tests run.
> > > >
> > > > Also, I guess we want this series to land in 5.16-rc4 or rc5 and mark
> > > > it as stable, no?
> > > > If so, I'll add the cc stable@vger.kernel.org marks so you don't need
> > > > to keep tabs on it.
> > >
> > > That would be great, thanks!
> > >
> > > > Besides that, I tested the wacom change on an I2C screen, and it is
> > > > still working as expected, so I think the wacom changes are safe.
> > > > The full series is
> > > > Tested-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > >
> > > Wonderful.
> > >
> > > Note, I did just get some really odd 0-day error messages from this
> > > patch series:
> > >       https://lore.kernel.org/r/202112021000.3GKuE8mS-lkp@intel.com
> > >       https://lore.kernel.org/r/202112020808.AZ5usuXj-lkp@intel.com
> > >
> > > Which looks like a dependancy issue that might have always been there.
> > > I'll work on resolving that after lunch...
> >
> > Now sent:
> >         https://lore.kernel.org/r/20211202114819.2511954-1-gregkh@linuxfoundation.org
>
> \o/
>
> >
> > I can resend it as a v4 series that adds this to the end of the series
> > if you want me to.
>
> That's fine. I already locally scheduled the v3, this other series
> above and my 2 first patches from my series that you reviewed today.
> I also added the cc: stable markers (though I haven't checked exactly
> which versions it applies to).
>
> Once my automated suite is happy I'll push them to hid.git.
>
> Thanks for your time Greg!
>

Series is applied to hid.git branch for-5.16/upstream-fixes

Cheers,
Benjamin


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-12-02 16:48 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-01 18:35 [PATCH v3 1/3] HID: add hid_is_usb() function to make it simpler for USB detection Greg Kroah-Hartman
2021-12-01 18:35 ` [PATCH v3 2/3] HID: wacom: fix problems when device is not a valid USB device Greg Kroah-Hartman
2021-12-01 18:35 ` [PATCH v3 3/3] HID: check for valid USB device for many HID drivers Greg Kroah-Hartman
2021-12-02  9:08   ` Benjamin Tissoires
2021-12-02  9:25     ` Greg Kroah-Hartman
2021-12-02 11:54       ` Greg Kroah-Hartman
2021-12-02 15:21         ` Benjamin Tissoires
2021-12-02 16:47           ` Benjamin Tissoires

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.