Re: [PATCH v3 4/8] HID: asus: cleanup keyboard listener on failure: avoid use-after-free

[Antheas Kapenekakis]

On Sat, 13 Jun 2026 at 17:30, Denis Benato <denis.benato@xxxxxxxxx> wrote:
>
> asus_kbd_register_leds(), I noticed it registers a listener to a global list
> and uses devm_kzalloc(). If a subsequent initialization step in asus_probe()
> fails the driver returns without unregistering the listener, and the devres
> subsystem will automatically free the memory, leaving a dangling pointer
> in the global list.
>
> Fixes: b34b5945a769 ("HID: asus: listen to the asus-wmi brightness device instead of creating one")
> Reported-by: sahiko-bot@xxxxxxxxxx
> Signed-off-by: Denis Benato <denis.benato@xxxxxxxxx>
> ---
>  drivers/hid/hid-asus.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index 0a6c97155549..f38b18ad65c6 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -1426,11 +1426,17 @@ static int asus_probe(struct hid_device *hdev, const struct hid_device_id *id)
>                 if (drvdata->tp) {
>                         ret = asus_start_multitouch(hdev);
>                         if (ret)
> -                               goto err_stop_hw;
> +                               goto err_unregister_backlight;
>                 }

This block currently does two things, rename and init the touchpad.
Instead,  you may pull the touchpad init to be below the hid_hw_start
block and keep the same bail.

>         }
>
>         return 0;
> +err_unregister_backlight:
> +       if (drvdata->kbd_backlight) {
> +               asus_hid_unregister_listener(&drvdata->kbd_backlight->listener);
> +               devm_kfree(&hdev->dev, drvdata->kbd_backlight);
> +               drvdata->kbd_backlight = NULL;
> +       }
>  err_stop_hw:

OR add asus_hid_unregister_listener(&drvdata->kbd_backlight->listener) here.

Here, you'd also drop err_unregister_backlight, devm_kfree, and
nulling. The driver is closing. It is not necessary.

err_stop_hw is only used once, you do not need a secondary tag.

>         hid_hw_stop(hdev);
>         return ret;
> --
> 2.47.3
>
>