Re: [PATCH] HID: haptic: fix use-after-free of devm haptic data in hid_haptic_destroy()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] input: guaranteed double free of haptic structure in hid_haptic_destroy()
- [High] hid: double free of td->haptic on mt_probe() error paths
- [Medium] hid: memory leak of td->haptic for non-touchpad devices with pressure
--

commit 9428ea91bd775df855a2be1ddad3deee003770fa
Author: Aldo Ariel Panzardo <qwe.aldo@gmail.com>

HID: haptic: fix use-after-free of devm haptic data in hid_haptic_destroy()

This commit changes td->haptic allocation from devm_kzalloc() to kzalloc() 
to prevent a use-after-free when the HID device is unbound while evdev is 
still open. It also adds explicit kfree() calls in the destruction callback
and error paths to manage the memory manually.

> diff --git a/drivers/hid/hid-haptic.c b/drivers/hid/hid-haptic.c
> index 8760eeb08b2cd..6c365dbf0b9cf 100644
> --- a/drivers/hid/hid-haptic.c
> +++ b/drivers/hid/hid-haptic.c
[ ... ]
> @@ -406,6 +406,8 @@ static void hid_haptic_destroy(struct ff_device *ff)
>  	haptic->hid_usage_map = NULL;
>  
>  	module_put(THIS_MODULE);
> +
> +	kfree(haptic);

[Severity: High]
Does this code introduce a double free?

This function is registered as the ff->destroy callback for the
force-feedback device in hid_haptic_init(). The input core's
input_ff_destroy() function unconditionally calls kfree(ff->private)
immediately after executing the ff->destroy callback.

Since haptic and ff->private point to the exact same memory, it looks
like this will result in a double-free whenever the force-feedback device
is destroyed.

>  }
>  
>  int hid_haptic_init(struct hid_device *hdev,
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 4e19a0c4d2381..4f7b97cd4baa5 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
[ ... ]
> @@ -2196,7 +2198,7 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  
>  	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
>  	if (ret)
> -		return ret;
> +		goto err_free_haptic;
>  
>  	ret = sysfs_create_group(&hdev->dev.kobj, &mt_attribute_group);
>  	if (ret)
> @@ -2206,9 +2208,13 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  	mt_set_modes(hdev, HID_LATENCY_NORMAL, TOUCHPAD_REPORT_ALL);
>  
>  	if (!td->is_haptic_touchpad)
> -		devm_kfree(&hdev->dev, td->haptic);
> +		kfree(td->haptic);

[Severity: Medium]
Will this leak td->haptic for non-touchpad devices reporting pressure?

If the device reports pressure, mt_input_mapping() sets 
td->is_haptic_touchpad = true. Then in mt_input_configured(), if the
device application is not a touchpad (e.g. a touchscreen), 
hid_haptic_input_configured() immediately returns -1.

Because -1 is not 0, mt_input_configured() fails to reset 
td->is_haptic_touchpad to false.

The haptic setup is aborted, so the memory is never attached to the input
core. Later, mt_probe() sees td->is_haptic_touchpad as true and bypasses
this kfree(), leaking the memory since it is no longer devm-managed.

>  
>  	return 0;
> +
> +err_free_haptic:
> +	kfree(td->haptic);

[Severity: High]
Can this kfree() result in a double-free during partial initialization
failures?

If hid_hw_start() successfully sets up the haptic device via 
hid_haptic_init(), ownership of td->haptic is transferred to the input core 
as ff->private.

If a subsequent initialization step inside hid_hw_start() fails (like 
registering a secondary input interface), the HID core unwinds by 
destroying registered input devices. This calls input_ff_destroy() and 
frees ff->private.

When hid_hw_start() returns an error code, mt_probe() jumps here to 
err_free_haptic and explicitly executes kfree(td->haptic), freeing the
same memory again.

> +	return ret;
>  }
>  
>  static int mt_suspend(struct hid_device *hdev, pm_message_t state)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009031203.3127352-1-qwe.aldo@gmail.com?part=1