Re: [PATCH v3] HID: multitouch: use kzalloc for haptic data to fix use-after-free

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] hid: hid-multitouch: UAF / Double-free of td->haptic on hid_haptic_init() failure
- [Medium] hid: memory leak of td->haptic due to premature flag set
--

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

    HID: multitouch: use kzalloc for haptic data to fix use-after-free

This commit replaces devm_kzalloc() with plain kzalloc() for td->haptic
to ensure it survives driver unbind. It introduces manual kfree() calls on
error paths and when the haptic subsystem is not used.

> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 4e19a0c4d2381..9cbe61832328c 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
[ ... ]
> @@ -2195,8 +2197,11 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
>  		hdev->quirks |= HID_QUIRK_NOGET;
>  
>  	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> -	if (ret)
> +	if (ret) {
> +		if (!td->is_haptic_touchpad)
> +			kfree(td->haptic);
>  		return ret;
> +	}

[Severity: Medium]
Does this error path leak td->haptic if hid_hw_start() fails after mapping
but before configuration?

td->is_haptic_touchpad is set to true during the report mapping phase in
mt_touch_input_mapping(). If hid_hw_start() fails after this mapping is done,
but before the configuration phase (for example, if hidinput_connect() fails
to allocate an input device), td->is_haptic_touchpad will be true. 

The input core has not taken ownership of td->haptic yet, so skipping kfree()
here would leak the memory.

[ ... ]
> @@ -2206,9 +2211,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);
>  
>  	return 0;

[Severity: High]
Does this skip kfree() and lead to a memory leak if
hid_haptic_input_configured() returns an error?

In mt_input_configured(), the flag is only cleared if the configuration
returns 0:

drivers/hid/hid-multitouch.c:mt_input_configured() {
    ...
    if (hid_haptic_input_configured(hdev, td->haptic, hi) == 0)
        td->is_haptic_touchpad = false;
    ...
}

If hid_haptic_input_configured() returns -1 (e.g., when the device lacks a
touchpad application), the flag remains true. The success path in mt_probe()
would then skip freeing td->haptic, leading to a memory leak.

Additionally, can this cause a use-after-free or a double-free on
hid_haptic_init() failure?

If try_module_get() fails in hid_haptic_init(), it jumps to the input_free
label without setting an error code:

drivers/hid/hid-haptic.c:hid_haptic_init() {
    if (!try_module_get(THIS_MODULE)) {
        dev_err(&hdev->dev, "Failed to increase module count.\n");
        goto input_free;
    }
    ...
input_free:
    input_ff_destroy(dev);
    ...
}

This path calls input_ff_destroy(), which unconditionally frees ff->private
(which points to td->haptic). Because no error code is returned,
mt_input_configured() incorrectly assumes success and leaves
td->is_haptic_touchpad set to true.

This leaves a dangling pointer in td->haptic since it was already freed by
input_ff_destroy(), potentially leading to a use-after-free on subsequent
touch events.

If the missing error code in hid_haptic_init() is corrected,
mt_input_configured() would set the flag to false, and this success path
in mt_probe() would call kfree(td->haptic) on already freed memory, causing
a double-free.

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