mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
To: jikos@kernel.org, bentiss@kernel.org
Cc: dmitry.torokhov@gmail.com, linux-input@vger.kernel.org,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org,
	Aldo Ariel Panzardo <qwe.aldo@gmail.com>
Subject: [PATCH v4] HID: multitouch: fix use-after-free of haptic data on delayed input release
Date: Fri,  9 Oct 2026 10:33:16 -0300	[thread overview]
Message-ID: <20261009133316.3371188-1-qwe.aldo@gmail.com> (raw)
In-Reply-To: <20261009110509.2432308-1-qwe.aldo@gmail.com>

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


      reply	other threads:[~2026-10-09 13:33 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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   ` Aldo Ariel Panzardo [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261009133316.3371188-1-qwe.aldo@gmail.com \
    --to=qwe.aldo@gmail.com \
    --cc=bentiss@kernel.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=jikos@kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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®