On Wed, Dec 1, 2021 at 7:35 PM Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxx> > Cc: Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx> > Cc: Michael Zaidman <michael.zaidman@xxxxxxxxx> > Cc: Stefan Achatz <erazor_de@xxxxxxxxxxxxxxxxxxxxx> > Cc: Maxime Coquelin <mcoquelin.stm32@xxxxxxxxx> > Cc: Alexandre Torgue <alexandre.torgue@xxxxxxxxxxx> > Cc: linux-input@xxxxxxxxxxxxxxx > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> > --- > 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@xxxxxxxxxxxxxxx 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@xxxxxxxxxx> 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 >