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
>
