Re: [PATCH] HID: asus: fix touchpad multitouch initialization regression

Hi Denis,

Thanks for the fast reply!
Good catch on the UAF risk. It turns out simply adding `if
(!drvdata->tp)` to skip the keyboard init loop was actually all that
was needed to fix the touchpad probe on the E200HA.

That way, `asus_start_multitouch()` stays safely inside the claimed
input check as before without touching any of the security logic, and
the && condition doesn't need to be changed back to ||.

I've tested this on hardware (ASUS E200HA and VivoBook Flip 14
TP401MA) and just sent v2.

Sincerely,
PANZ

On Tue, Aug 18, 2026 at 11:15 AM <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
>
> New issues:
> - [High] Unconditionally calling `asus_start_multitouch()` outside the `HID_CLAIMED_INPUT` check introduces a Use-After-Free if input registration fails.
>
> Pre-existing issues:
> - [High] Incorrect cleanup order in `asus_remove()` creates a Use-After-Free race condition by cancelling the workqueue before disabling hardware events.
> - [High] Missing `hid_is_usb()` check in `asus_kbd_register_leds()` allows an invalid cast to `usb_interface`, causing out-of-bounds reads or crashes.
> - [Medium] Modifying `input->name` after device registration creates a data race and violates input subsystem API requirements.
> --
>
> --- Patch [1]: [PATCH] HID: asus: fix touchpad multitouch initialization regression ---
> commit 53becd94719f665ca15fc8c3f4b7876a402da45f
> Author: Panz Dev <[email protected]>
>
> HID: asus: fix touchpad multitouch initialization regression
>
> This patch fixes probe regressions for ASUS I2C/HID touchpads by skipping
> keyboard report initialization for touchpads and moving
> asus_start_multitouch() outside the claimed input check block.
>
> > diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> > index 3f5e96900b67a..72b93bdeb4e8f 100644
> > --- a/drivers/hid/hid-asus.c
> > +++ b/drivers/hid/hid-asus.c
> [ ... ]
> > @@ -1327,12 +1329,12 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
> >                       drvdata->input->name = "Asus TouchPad";
> >               else
> >                       drvdata->input->name = "Asus Keyboard";
>
> [Severity: Medium]
> This is a pre-existing issue, but can modifying drvdata->input->name here
> after device registration (which occurs earlier during hid_hw_start) create a
> data race?
>
> If a userspace process like udev reads the name attribute concurrently, might
> it read the default string or race with the pointer reassignment?
>
> > +     }
> >
> > -             if (drvdata->tp) {
> > -                     ret = asus_start_multitouch(hdev);
> > -                     if (ret)
> > -                             goto err_stop_hw;
> > -             }
> > +     if (drvdata->tp) {
> > +             ret = asus_start_multitouch(hdev);
> > +             if (ret)
> > +                     goto err_stop_hw;
> >       }
>
> [Severity: High]
> Does moving asus_start_multitouch() outside the HID_CLAIMED_INPUT check
> introduce a use-after-free regression if input registration fails?
>
> If input_register_device() fails during hid_hw_start(), hidinput_connect()
> frees the allocated input_dev and returns an error without setting
> HID_CLAIMED_INPUT.
>
> Since the HID_CLAIMED_INPUT safety guard is now removed, the driver
> will force the touchpad hardware to start sending reports. When the hardware
> interrupt arrives, asus_raw_event() intercepts it and routes to
> asus_report_input():
>
> drivers/hid/hid-asus.c:asus_raw_event() {
>     ...
>         if (drvdata->tp && data[0] == INPUT_REPORT_ID)
>                 return asus_report_input(drvdata, data, size);
>     ...
> }
>
> This blindly dereferences the freed drvdata->input pointer, which was assigned
> earlier in asus_input_configured():
>
> drivers/hid/hid-asus.c:asus_report_input() {
>     ...
>                 input_mt_slot(drvdat->input, i);
>                 input_mt_report_slot_state(drvdat->input, toolType, down);
>     ...
> }
>
> Is it possible for this to cause a use-after-free, for instance if triggered
> via uhid device spoofing or resource allocation failures during probe?
>
> >
> >       return 0;
>
> [Severity: High]
> This isn't a bug introduced by this patch, but does the cleanup order in
> asus_remove() create a use-after-free race condition?
>
> In asus_remove(), the driver calls:
>
> drivers/hid/hid-asus.c:asus_remove() {
>     ...
>         if (drvdata->quirks & QUIRK_HID_FN_LOCK)
>                 cancel_work_sync(&drvdata->fn_lock_sync_work);
>
>         hid_hw_stop(hdev);
>     ...
> }
>
> If the device sends a KEY_FN_ESC report via hardware interrupt immediately
> after cancel_work_sync() returns but before hid_hw_stop() completes, might
> asus_event() re-queue the work via schedule_work()? Once asus_remove()
> finishes, the devm-managed drvdata is freed, which could leave the work item
> scheduled and lead to a use-after-free when the worker executes.
>
> [Severity: High]
> This is a pre-existing issue, but could the lack of a hid_is_usb() check in
> asus_kbd_register_leds() allow an invalid cast?
>
> When handling the QUIRK_ROG_ALLY_XPAD quirk:
>
> drivers/hid/hid-asus.c:asus_kbd_register_leds() {
>     ...
>         if (drvdata->quirks & QUIRK_ROG_ALLY_XPAD) {
>                 intf = to_usb_interface(hdev->dev.parent);
>                 udev = interface_to_usbdev(intf);
>     ...
> }
>
> If the device is connected via another transport (like Bluetooth, I2C, or
> spoofed via uhid), the parent device is not a USB interface. Would calling
> interface_to_usbdev() on this invalid pointer lead to out-of-bounds
> memory access or a crash?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1