Re: [PATCH 4/4] HID: lg-g15: cancel LED work only after stopping the hardware

[Hans de Goede]

Hi,

On 9-Nov-26 10:44, Torstein Eide wrote:
> lg_g15_remove() cancels g15->work before calling hid_hw_stop(). In
> between, a backlight or game-mode key press makes raw_event call
> schedule_work() again. The work then runs after remove() has returned
> and devm has freed g15, which is a use-after-free in
> lg_g15_leds_changed_work().
> 
> Cancel the work after hid_hw_stop(), once no more raw events can arrive.

This is not necessary the hid-core avoids the raw_event callback getting
called during remove() by hid_device_remove() doing:

	down(&hdev->driver_input_lock);

before calling the drivers remove callback.

Regards,

Hans




> 
> Fixes: 7705b4140d18 ("HID: lg-g15: cancel pending work on remove to fix a use-after-free")
> Signed-off-by: Torstein Eide <torsteine+linux@xxxxxxxxx>
> Assisted-by: Claude:claude-opus-5-5
> ---
>  drivers/hid/hid-lg-g15.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/hid/hid-lg-g15.c b/drivers/hid/hid-lg-g15.c
> index 592c9747738e..a341b0aca249 100644
> --- a/drivers/hid/hid-lg-g15.c
> +++ b/drivers/hid/hid-lg-g15.c
> @@ -1927,18 +1927,19 @@ static void lg_g15_remove(struct hid_device *hdev)
>  
>  	g15 = hid_get_drvdata(hdev);
>  
> +	if (g15)
> +		lg_g15_unregister_fb(g15);
> +
> +	hid_hw_stop(hdev);
> +
>  	/*
>  	 * g15->work is only initialized for the models that schedule it
>  	 * (G15, G15 v2, G510). The G13 and Z-10 leave it zeroed, so only
> -	 * cancel it when it was set up.
> +	 * cancel it when it was set up. Cancel it after hid_hw_stop(), as
> +	 * until then raw_event can still schedule it again.
>  	 */
>  	if (g15 && g15->work.func)
>  		cancel_work_sync(&g15->work);
> -
> -	if (g15)
> -		lg_g15_unregister_fb(g15);
> -
> -	hid_hw_stop(hdev);
>  }
>  
>  /*