Re: [PATCH v2 2/2] HID: lenovo-go: drop dead NULL check on to_usb_interface()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Resource Leak and Use-After-Free due to Asymmetric Device Cleanup
--

commit 7509c2865ff8f5400d44c180711298e07d9096ba
Author: Louis Clinckx <clinckx.louis@gmail.com>

HID: lenovo-go: drop dead NULL check on to_usb_interface()

to_usb_interface() is a container_of_const() macro: it performs
pointer arithmetic and never returns NULL. The if (!intf) and if
(intf) tests in get_endpoint_address() can never fire. Remove them
in both drivers.

[ ... ]

> diff --git a/drivers/hid/hid-lenovo-go.c b/drivers/hid/hid-lenovo-go.c
> index 3fa1fe83f7e5e..e0c9d5ec9451b 100644
> --- a/drivers/hid/hid-lenovo-go.c
> +++ b/drivers/hid/hid-lenovo-go.c
> @@ -641,9 +641,6 @@ static int get_endpoint_address(struct hid_device *hdev)
>  	struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
>  	struct usb_host_endpoint *ep;
>  
> -	if (!intf)
> -		return -ENODEV;
> -
>  	ep = intf->cur_altsetting->endpoint;
>  	if (!ep)
>  		return -ENODEV;

This isn't a regression introduced by your patch, but I noticed an issue with
how -ENODEV is handled by the driver.

If get_endpoint_address() returns -ENODEV (e.g., when the USB interface has no
endpoints and ep is NULL), hid_go_probe() successfully binds the device:

hid_go_probe() {
    ...
    ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
    ...
    ret = hid_hw_open(hdev);
    ...
    ep = get_endpoint_address(hdev);
    if (ep != GO_GP_INTF_IN) {
        dev_dbg(&hdev->dev, "Started interface %x as generic HID device\n", ep);
        return 0;
    }
    ...
}

However, hid_go_remove() skips the cleanup if ep is -ENODEV:

hid_go_remove() {
    int ep = get_endpoint_address(hdev);

    if (ep <= 0)
        return;
    ...
}

Does this asymmetric cleanup cause a use-after-free or leak of the input devices
and HID parser buffers since hid_hw_close() and hid_hw_stop() are skipped?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1778701376.git.clinckx.louis@gmail.com?part=2