* [PATCH v2] HID: haptic: fix use-after-free of devm haptic data in hid_haptic_destroy()
@ 2026-10-09 3:26 Aldo Ariel Panzardo
2026-10-09 11:05 ` [PATCH v3] HID: multitouch: use kzalloc for haptic data to fix use-after-free Aldo Ariel Panzardo
0 siblings, 1 reply; 3+ messages in thread
From: Aldo Ariel Panzardo @ 2026-10-09 3:26 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, stable, Aldo Ariel Panzardo
mt_probe() allocates td->haptic with devm_kzalloc(), tying its
lifetime to the HID device's driver unbind. hid_haptic_init() then
installs hid_haptic_destroy() as the force-feedback destroy callback,
which dereferences haptic->hdev on its very first line.
When the HID device is removed while a process still holds an evdev
fd, devres frees td->haptic at unbind time, but hid_haptic_destroy()
runs later from input_dev_release() when the last fd closes. The
callback operates on freed memory.
The existing get_device()/put_device() pair in init/destroy pins the
struct hid_device but does not keep its devres allocations alive,
since devres runs at driver unbind, not at the final device kref put.
Replace devm_kzalloc() with plain kzalloc() for the haptic struct so
it outlives the driver. Free it at the end of hid_haptic_destroy(),
which already tears down every sub-allocation manually and holds a
device reference that keeps hdev alive until the kfree. On the
non-haptic path and the pre-hid_hw_start error paths in mt_probe(),
use kfree() instead of devm_kfree().
Fixes: 8d0bf7908b5a ("HID: multitouch: add haptic multitouch support")
Cc: stable@vger.kernel.org
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
v2: do not free td->haptic on hid_hw_start() failure — if
hid_haptic_init() ran inside hid_hw_start() and installed the
FF destroy callback, the internal cleanup already frees the
struct via hid_haptic_destroy(). Only free it on the error
paths before hid_hw_start() (found by Sashiko AI review).
drivers/hid/hid-haptic.c | 2 ++
drivers/hid/hid-multitouch.c | 16 +++++++++++-----
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/hid-haptic.c b/drivers/hid/hid-haptic.c
index 8760eeb08..6c365dbf0 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);
}
int hid_haptic_init(struct hid_device *hdev,
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 4e19a0c4d..f4d8d640d 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -2132,7 +2132,7 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
dev_err(&hdev->dev, "cannot allocate multitouch data\n");
return -ENOMEM;
}
- td->haptic = devm_kzalloc(&hdev->dev, sizeof(*(td->haptic)), GFP_KERNEL);
+ td->haptic = kzalloc(sizeof(*(td->haptic)), GFP_KERNEL);
if (!td->haptic)
return -ENOMEM;
@@ -2181,12 +2181,14 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
ret = hid_parse(hdev);
if (ret != 0)
- return ret;
+ goto err_free_haptic;
if (mtclass->name == MT_CLS_APPLE_TOUCHBAR &&
!hid_find_field(hdev, HID_INPUT_REPORT,
- HID_DG_TOUCHPAD, HID_DG_TRANSDUCER_INDEX))
- return -ENODEV;
+ HID_DG_TOUCHPAD, HID_DG_TRANSDUCER_INDEX)) {
+ ret = -ENODEV;
+ goto err_free_haptic;
+ }
if (mtclass->quirks & MT_QUIRK_FIX_CONST_CONTACT_ID)
mt_fix_const_fields(hdev, HID_DG_CONTACTID);
@@ -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);
return 0;
+
+err_free_haptic:
+ kfree(td->haptic);
+ return ret;
}
static int mt_suspend(struct hid_device *hdev, pm_message_t state)
--
2.43.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH v3] HID: multitouch: use kzalloc for haptic data to fix use-after-free
2026-10-09 3:26 [PATCH v2] HID: haptic: fix use-after-free of devm haptic data in hid_haptic_destroy() Aldo Ariel Panzardo
@ 2026-10-09 11:05 ` Aldo Ariel Panzardo
2026-10-09 13:33 ` [PATCH v4] HID: multitouch: fix use-after-free of haptic data on delayed input release Aldo Ariel Panzardo
0 siblings, 1 reply; 3+ messages in thread
From: Aldo Ariel Panzardo @ 2026-10-09 11:05 UTC (permalink / raw)
To: jikos, bentiss
Cc: dmitry.torokhov, linux-input, linux-kernel, stable, Aldo Ariel Panzardo
mt_probe() allocates td->haptic with devm_kzalloc(), tying its
lifetime to the HID device's driver unbind. hid_haptic_init() then
stores the pointer in ff->private and installs hid_haptic_destroy()
as the force-feedback destroy callback.
When the HID device is removed while a process still holds an evdev
fd, devres frees td->haptic at unbind time. hid_haptic_destroy()
runs later from input_dev_release() and dereferences freed memory.
The input core then calls kfree(ff->private) on the same pointer,
double-freeing it.
The existing get_device()/put_device() pair in init/destroy pins the
struct hid_device but does not keep its devres allocations alive,
since devres runs at driver unbind, not at the final device kref put.
Replace devm_kzalloc() with plain kzalloc() so the haptic struct
survives driver unbind. The input core's input_ff_destroy() already
calls kfree(ff->private) after the destroy callback, so
hid_haptic_destroy() must not free the struct itself -- it only needs
to tear down the sub-allocations it owns.
On the non-haptic path in mt_probe(), replace devm_kfree() with
kfree(). On the error paths before hid_hw_start(), free the struct
explicitly since ownership has not yet transferred to the input core.
On hid_hw_start() failure, only free the struct when the haptic
subsystem was not initialized (td->is_haptic_touchpad is false);
otherwise the input core's teardown already freed it.
Fixes: 8d0bf7908b5a ("HID: multitouch: add haptic multitouch support")
Cc: stable@vger.kernel.org
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
v3: drop the kfree(haptic) added to hid_haptic_destroy() in v1/v2 --
input_ff_destroy() already calls kfree(ff->private) after the
destroy callback, so the explicit kfree was a guaranteed double
free (found by Sashiko AI review). Also handle the hid_hw_start()
failure path: free td->haptic only when the haptic subsystem was
not initialized (!td->is_haptic_touchpad), since otherwise the
input core's teardown already freed it.
v2: do not free td->haptic on hid_hw_start() failure (found by
Sashiko AI review).
v1: https://lore.kernel.org/linux-input/20261009031207.3233206-1-qwe.aldo@gmail.com/
drivers/hid/hid-multitouch.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 4e19a0c4d..9cbe61832 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -2132,7 +2132,7 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
dev_err(&hdev->dev, "cannot allocate multitouch data\n");
return -ENOMEM;
}
- td->haptic = devm_kzalloc(&hdev->dev, sizeof(*(td->haptic)), GFP_KERNEL);
+ td->haptic = kzalloc(sizeof(*(td->haptic)), GFP_KERNEL);
if (!td->haptic)
return -ENOMEM;
@@ -2181,12 +2181,14 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
ret = hid_parse(hdev);
if (ret != 0)
- return ret;
+ goto err_free_haptic;
if (mtclass->name == MT_CLS_APPLE_TOUCHBAR &&
!hid_find_field(hdev, HID_INPUT_REPORT,
- HID_DG_TOUCHPAD, HID_DG_TRANSDUCER_INDEX))
- return -ENODEV;
+ HID_DG_TOUCHPAD, HID_DG_TRANSDUCER_INDEX)) {
+ ret = -ENODEV;
+ goto err_free_haptic;
+ }
if (mtclass->quirks & MT_QUIRK_FIX_CONST_CONTACT_ID)
mt_fix_const_fields(hdev, HID_DG_CONTACTID);
@@ -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;
+ }
ret = sysfs_create_group(&hdev->dev.kobj, &mt_attribute_group);
if (ret)
@@ -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;
+
+err_free_haptic:
+ kfree(td->haptic);
+ return ret;
}
static int mt_suspend(struct hid_device *hdev, pm_message_t state)
--
2.43.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH v4] HID: multitouch: fix use-after-free of haptic data on delayed input release
2026-10-09 11:05 ` [PATCH v3] HID: multitouch: use kzalloc for haptic data to fix use-after-free Aldo Ariel Panzardo
@ 2026-10-09 13:33 ` Aldo Ariel Panzardo
0 siblings, 0 replies; 3+ messages in thread
From: Aldo Ariel Panzardo @ 2026-10-09 13:33 UTC (permalink / raw)
To: jikos, bentiss
Cc: dmitry.torokhov, linux-input, linux-kernel, stable, Aldo Ariel Panzardo
mt_probe() allocates td->haptic with devm_kzalloc(), tying its
lifetime to the HID device's driver unbind. hid_haptic_init() then
stores the pointer in ff->private and installs hid_haptic_destroy()
as the force-feedback destroy callback.
When the HID device is removed while a process still holds an evdev
fd, devres frees td->haptic at unbind time. hid_haptic_destroy()
runs later from input_dev_release() and dereferences freed memory.
The input core then calls kfree(ff->private) on the same pointer,
double-freeing it.
The existing get_device()/put_device() pair in init/destroy pins the
struct hid_device but does not keep its devres allocations alive,
since devres runs at driver unbind, not at the final device kref put.
Replace devm_kzalloc() with plain kzalloc() so the haptic struct
survives driver unbind. Track ownership explicitly:
- Add an 'owner' back-pointer (to td->haptic) and an 'ff_owned'
flag to struct hid_haptic_device.
- hid_haptic_destroy() NULLs *owner under a lock before the input
core frees ff->private, so td->haptic becomes NULL and later
kfree(td->haptic) calls are harmless.
- hid_haptic_release() safely frees the struct only when the FF
subsystem does not own it.
- In mt_remove(), hid_haptic_detach() disconnects the back-pointer
before hid_hw_stop() so a deferred destroy (from a still-open
evdev fd) does not write into the devres-freed mt_device.
Additionally, reorder hid_haptic_init() to call try_module_get() and
get_device() before input_ff_create(), eliminating the input_free
error path that called input_ff_destroy() without setting a return
code -- which left td->haptic as a dangling pointer on those
(theoretical) failures.
Fixes: 8d0bf7908b5a ("HID: multitouch: add haptic multitouch support")
Cc: stable@vger.kernel.org
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
v4: track ownership explicitly with an owner back-pointer, an
ff_owned flag, and hid_haptic_release()/hid_haptic_detach()
helpers. Reorder hid_haptic_init() so try_module_get() and
get_device() run before input_ff_create(), eliminating the
input_free error path that left ret == 0 on failure. In
mt_remove(), detach the back-pointer before hid_hw_stop() so a
deferred destroy from a still-open evdev fd does not write into
the devres-freed mt_device. All findings from Sashiko AI review
on v1-v3 are addressed.
v3: drop kfree(haptic) from hid_haptic_destroy() -- input_ff_destroy()
already calls kfree(ff->private) after the callback. Handle
hid_hw_start() failure with conditional kfree.
v2: do not free td->haptic on hid_hw_start() failure.
v1: https://lore.kernel.org/linux-input/20261009031207.3233206-1-qwe.aldo@gmail.com/
drivers/hid/hid-haptic.c | 73 ++++++++++++++++++++++++++----------
drivers/hid/hid-haptic.h | 14 +++++++
drivers/hid/hid-multitouch.c | 27 ++++++++++---
3 files changed, 89 insertions(+), 25 deletions(-)
diff --git a/drivers/hid/hid-haptic.c b/drivers/hid/hid-haptic.c
index 8760eeb..e4e6eb3 100644
--- a/drivers/hid/hid-haptic.c
+++ b/drivers/hid/hid-haptic.c
@@ -10,6 +10,8 @@
#include "hid-haptic.h"
+static DEFINE_MUTEX(haptic_owner_lock);
+
void hid_haptic_feature_mapping(struct hid_device *hdev,
struct hid_haptic_device *haptic,
struct hid_field *field, struct hid_usage *usage)
@@ -383,6 +385,12 @@ static void hid_haptic_destroy(struct ff_device *ff)
struct hid_device *hdev = haptic->hdev;
int r;
+ mutex_lock(&haptic_owner_lock);
+ if (haptic->owner)
+ *haptic->owner = NULL;
+ haptic->owner = NULL;
+ mutex_unlock(&haptic_owner_lock);
+
if (hdev)
put_device(&hdev->dev);
@@ -408,6 +416,37 @@ static void hid_haptic_destroy(struct ff_device *ff)
module_put(THIS_MODULE);
}
+void hid_haptic_release(struct hid_haptic_device **owner)
+{
+ struct hid_haptic_device *haptic;
+
+ mutex_lock(&haptic_owner_lock);
+ haptic = *owner;
+ if (haptic) {
+ haptic->owner = NULL;
+ *owner = NULL;
+ if (!haptic->ff_owned)
+ kfree(haptic);
+ }
+ mutex_unlock(&haptic_owner_lock);
+}
+EXPORT_SYMBOL_GPL(hid_haptic_release);
+
+bool hid_haptic_detach(struct hid_haptic_device **owner)
+{
+ bool ff_owned = false;
+
+ mutex_lock(&haptic_owner_lock);
+ if (*owner) {
+ (*owner)->owner = NULL;
+ ff_owned = (*owner)->ff_owned;
+ }
+ mutex_unlock(&haptic_owner_lock);
+
+ return ff_owned;
+}
+EXPORT_SYMBOL_GPL(hid_haptic_detach);
+
int hid_haptic_init(struct hid_device *hdev,
struct hid_haptic_device *haptic,
struct input_dev *dev)
@@ -419,8 +458,6 @@ int hid_haptic_init(struct hid_device *hdev,
};
const char *prefix = "hid-haptic";
char *name;
- int (*flush)(struct input_dev *dev, struct file *file);
- int (*event)(struct input_dev *dev, unsigned int type, unsigned int code, int value);
haptic->hdev = hdev;
haptic->max_waveform_id = max(2u, haptic->max_waveform_id);
@@ -501,11 +538,23 @@ int hid_haptic_init(struct hid_device *hdev,
input_set_capability(dev, EV_FF, FF_HAPTIC);
- flush = dev->flush;
- event = dev->event;
+ if (!try_module_get(THIS_MODULE)) {
+ dev_err(&hdev->dev, "Failed to increase module count.\n");
+ ret = -ENODEV;
+ goto stop_buffer_free;
+ }
+ if (!get_device(&hdev->dev)) {
+ dev_err(&hdev->dev, "Failed to get hdev device.\n");
+ ret = -ENODEV;
+ module_put(THIS_MODULE);
+ goto stop_buffer_free;
+ }
+
ret = input_ff_create(dev, FF_MAX_EFFECTS);
if (ret) {
dev_err(&hdev->dev, "Failed to create ff device.\n");
+ put_device(&hdev->dev);
+ module_put(THIS_MODULE);
goto stop_buffer_free;
}
@@ -515,23 +564,9 @@ int hid_haptic_init(struct hid_device *hdev,
ff->playback = hid_haptic_playback;
ff->erase = hid_haptic_erase;
ff->destroy = hid_haptic_destroy;
- if (!try_module_get(THIS_MODULE)) {
- dev_err(&hdev->dev, "Failed to increase module count.\n");
- goto input_free;
- }
- if (!get_device(&hdev->dev)) {
- dev_err(&hdev->dev, "Failed to get hdev device.\n");
- module_put(THIS_MODULE);
- goto input_free;
- }
+ haptic->ff_owned = true;
return 0;
-input_free:
- input_ff_destroy(dev);
- /* Restore dev flush and event */
- dev->flush = flush;
- dev->event = event;
- return ret;
stop_buffer_free:
kfree(haptic->stop_effect.report_buf);
haptic->stop_effect.report_buf = NULL;
diff --git a/drivers/hid/hid-haptic.h b/drivers/hid/hid-haptic.h
index 6332991..0a44993 100644
--- a/drivers/hid/hid-haptic.h
+++ b/drivers/hid/hid-haptic.h
@@ -6,6 +6,7 @@
*/
#include <linux/hid.h>
+#include <linux/slab.h>
#define HID_HAPTIC_ORDINAL_WAVEFORMNONE 1
#define HID_HAPTIC_ORDINAL_WAVEFORMSTOP 2
@@ -29,6 +30,8 @@ struct hid_haptic_effect_node {
struct hid_haptic_device {
struct input_dev *input_dev;
struct hid_device *hdev;
+ struct hid_haptic_device **owner;
+ bool ff_owned;
struct hid_report *auto_trigger_report;
struct mutex auto_trigger_mutex;
struct workqueue_struct *wq;
@@ -75,6 +78,8 @@ void hid_haptic_handle_press_release(struct hid_haptic_device *haptic);
void hid_haptic_pressure_reset(struct hid_haptic_device *haptic);
void hid_haptic_pressure_increase(struct hid_haptic_device *haptic,
__s32 pressure);
+void hid_haptic_release(struct hid_haptic_device **owner);
+bool hid_haptic_detach(struct hid_haptic_device **owner);
#else
static inline
void hid_haptic_feature_mapping(struct hid_device *hdev,
@@ -126,4 +131,13 @@ static inline
void hid_haptic_pressure_increase(struct hid_haptic_device *haptic,
__s32 pressure)
{}
+static inline void hid_haptic_release(struct hid_haptic_device **owner)
+{
+ kfree(*owner);
+ *owner = NULL;
+}
+static inline bool hid_haptic_detach(struct hid_haptic_device **owner)
+{
+ return false;
+}
#endif
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 4e19a0c..961ddeb 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -2132,10 +2132,11 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
dev_err(&hdev->dev, "cannot allocate multitouch data\n");
return -ENOMEM;
}
- td->haptic = devm_kzalloc(&hdev->dev, sizeof(*(td->haptic)), GFP_KERNEL);
+ td->haptic = kzalloc(sizeof(*(td->haptic)), GFP_KERNEL);
if (!td->haptic)
return -ENOMEM;
+ td->haptic->owner = &td->haptic;
td->haptic->hdev = hdev;
td->hdev = hdev;
td->mtclass = *mtclass;
@@ -2181,12 +2182,14 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
ret = hid_parse(hdev);
if (ret != 0)
- return ret;
+ goto err_free_haptic;
if (mtclass->name == MT_CLS_APPLE_TOUCHBAR &&
!hid_find_field(hdev, HID_INPUT_REPORT,
- HID_DG_TOUCHPAD, HID_DG_TRANSDUCER_INDEX))
- return -ENODEV;
+ HID_DG_TOUCHPAD, HID_DG_TRANSDUCER_INDEX)) {
+ ret = -ENODEV;
+ goto err_free_haptic;
+ }
if (mtclass->quirks & MT_QUIRK_FIX_CONST_CONTACT_ID)
mt_fix_const_fields(hdev, HID_DG_CONTACTID);
@@ -2195,8 +2198,10 @@ 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) {
+ hid_haptic_release(&td->haptic);
return ret;
+ }
ret = sysfs_create_group(&hdev->dev.kobj, &mt_attribute_group);
if (ret)
@@ -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);
+ hid_haptic_release(&td->haptic);
return 0;
+
+err_free_haptic:
+ hid_haptic_release(&td->haptic);
+ return ret;
}
static int mt_suspend(struct hid_device *hdev, pm_message_t state)
@@ -2248,11 +2257,17 @@ static int mt_resume(struct hid_device *hdev)
static void mt_remove(struct hid_device *hdev)
{
struct mt_device *td = hid_get_drvdata(hdev);
+ bool ff_owned;
timer_shutdown_sync(&td->release_timer);
sysfs_remove_group(&hdev->dev.kobj, &mt_attribute_group);
+ ff_owned = hid_haptic_detach(&td->haptic);
hid_hw_stop(hdev);
+ if (!ff_owned)
+ hid_haptic_release(&td->haptic);
+ else
+ td->haptic = NULL;
}
static void mt_on_hid_hw_open(struct hid_device *hdev)
--
2.43.0
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-09 13:33 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-09 3:26 [PATCH v2] HID: haptic: fix use-after-free of devm haptic data in hid_haptic_destroy() Aldo Ariel Panzardo
2026-10-09 11:05 ` [PATCH v3] HID: multitouch: use kzalloc for haptic data to fix use-after-free Aldo Ariel Panzardo
2026-10-09 13:33 ` [PATCH v4] HID: multitouch: fix use-after-free of haptic data on delayed input release Aldo Ariel Panzardo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®