[PATCH v10 0/2] HID: multitouch: fix haptic lifetime on delayed input release

Linux Input/HID development
 help / color / mirror / Atom feed

* [PATCH v10 0/2] HID: multitouch: fix haptic lifetime on delayed input release
@ 2026-10-17 14:38 Aldo Ariel Panzardo
  2026-10-17 14:38 ` [PATCH v10 1/2] HID: haptic: make FF creation the last fallible init step Aldo Ariel Panzardo
  2026-10-17 14:38 ` [PATCH v10 2/2] HID: multitouch: give FF core ownership of haptic state Aldo Ariel Panzardo
  0 siblings, 2 replies; 4+ messages in thread
From: Aldo Ariel Panzardo @ 2026-10-17 14:38 UTC (permalink / raw)
  To: jikos, bentiss
  Cc: dmitry.torokhov, linux-input, linux-kernel, stable,
	Aldo Ariel Panzardo

v9 review found that a non-multitouch touchpad report can precede
the actual multitouch input on devices using HID_QUIRK_MULTI_INPUT.
The first report could take the only FF instance and become populated
solely because FF was added. The multitouch input would then lack FF.

Patch 2 now waits until mt_input_configured() has identified a
multitouch touchpad report before initializing haptics. Only the input
that receives FF gets INPUT_PROP_PRESSUREPAD from the haptic setup.
Patch 2 also uses scope cleanup for the cloned state and all of its
subresources, removing the mixture of __free() and goto labels.
Patch 1 is unchanged from v9.

Tested on x86_64 with W=1: built-in HID objects, a minimal vmlinux
and modules build, and hid-multitouch.o with HID_HAPTIC=n. QEMU/KVM
with KASAN ran 10 virtual UHID haptic cycles and 10 multi-input cycles
with a non-multitouch report before the multitouch report. The tests
checked FF_HAPTIC, effect upload and playback, HID removal with an
open evdev fd, and final release. No KASAN report was emitted.
Physical hardware was not tested.

Base: 58161ead29e50d70a317ea26b472e0c7508e69da
v9: https://lore.kernel.org/linux-input/20261017140332.1480375-1-qwe.aldo@gmail.com/

Aldo Ariel Panzardo (2):
  HID: haptic: make FF creation the last fallible init step
  HID: multitouch: give FF core ownership of haptic state

 drivers/hid/hid-haptic.c     | 161 +++++++++++++++++------------------
 drivers/hid/hid-haptic.h     |   6 +-
 drivers/hid/hid-multitouch.c |  36 ++++++--
 3 files changed, 107 insertions(+), 96 deletions(-)

-- 
2.53.0

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v10 1/2] HID: haptic: make FF creation the last fallible init step
  2026-10-17 14:38 [PATCH v10 0/2] HID: multitouch: fix haptic lifetime on delayed input release Aldo Ariel Panzardo
@ 2026-10-17 14:38 ` Aldo Ariel Panzardo
  2026-10-17 14:38 ` [PATCH v10 2/2] HID: multitouch: give FF core ownership of haptic state Aldo Ariel Panzardo
  1 sibling, 0 replies; 4+ messages in thread
From: Aldo Ariel Panzardo @ 2026-10-17 14:38 UTC (permalink / raw)
  To: jikos, bentiss
  Cc: dmitry.torokhov, linux-input, linux-kernel, stable,
	Aldo Ariel Panzardo

The caller holds the module while setting up the device. Use
__module_get() after input_ff_create() succeeds, leaving no fallible
step after the FF callbacks are installed.

Drain haptic work through ff->stop when the input device is
unregistered, before it drops its parent reference. The destroy
callback also drains work for devices that were never registered.
After unregistration, that callback only frees local allocations, so
remove the extra HID device reference and its error path.

Clear the FF capability bits if input_ff_create() fails. Return the
error from hid_haptic_input_configured() and save it in the multitouch
driver. Probe must fail even if hidraw lets hid_connect() succeed after
hidinput_connect() has unwound.

Fixes: 8d0bf7908b5a ("HID: multitouch: add haptic multitouch support")
Cc: stable@vger.kernel.org
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
 drivers/hid/hid-haptic.c     | 48 ++++++++++++++----------------------
 drivers/hid/hid-multitouch.c | 18 ++++++++++++--
 2 files changed, 35 insertions(+), 31 deletions(-)

diff --git a/drivers/hid/hid-haptic.c b/drivers/hid/hid-haptic.c
index 8760eeb..0a6126e 100644
--- a/drivers/hid/hid-haptic.c
+++ b/drivers/hid/hid-haptic.c
@@ -96,7 +96,7 @@ int hid_haptic_input_configured(struct hid_device *hdev,
 	if (error) {
 		dev_warn(&hdev->dev, "Cannot allocate haptic for %s\n",
 			 hdev->name);
-		return 0;
+		return error;
 	}
 
 	return 1;
@@ -377,14 +377,22 @@ static int hid_haptic_erase(struct input_dev *dev, int effect_id)
 	return 0;
 }
 
+static void hid_haptic_stop(struct ff_device *ff)
+{
+	struct hid_haptic_device *haptic = ff->private;
+
+	if (haptic->wq) {
+		destroy_workqueue(haptic->wq);
+		haptic->wq = NULL;
+	}
+}
+
 static void hid_haptic_destroy(struct ff_device *ff)
 {
 	struct hid_haptic_device *haptic = ff->private;
-	struct hid_device *hdev = haptic->hdev;
 	int r;
 
-	if (hdev)
-		put_device(&hdev->dev);
+	hid_haptic_stop(ff);
 
 	kfree(haptic->stop_effect.report_buf);
 	haptic->stop_effect.report_buf = NULL;
@@ -396,9 +404,6 @@ static void hid_haptic_destroy(struct ff_device *ff)
 	}
 	haptic->effect = NULL;
 
-	destroy_workqueue(haptic->wq);
-	haptic->wq = NULL;
-
 	kfree(haptic->duration_map);
 	haptic->duration_map = NULL;
 
@@ -419,8 +424,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,38 +504,25 @@ int hid_haptic_init(struct hid_device *hdev,
 
 	input_set_capability(dev, EV_FF, FF_HAPTIC);
 
-	flush = dev->flush;
-	event = dev->event;
 	ret = input_ff_create(dev, FF_MAX_EFFECTS);
 	if (ret) {
 		dev_err(&hdev->dev, "Failed to create ff device.\n");
-		goto stop_buffer_free;
+		goto ff_cap_clear;
 	}
 
 	ff = dev->ff;
-	ff->private = haptic;
 	ff->upload = hid_haptic_upload_effect;
 	ff->playback = hid_haptic_playback;
 	ff->erase = hid_haptic_erase;
+	ff->stop = hid_haptic_stop;
 	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;
-	}
+	__module_get(THIS_MODULE);
+	ff->private = haptic;
 	return 0;
 
-input_free:
-	input_ff_destroy(dev);
-	/* Restore dev flush and event */
-	dev->flush = flush;
-	dev->event = event;
-	return ret;
-stop_buffer_free:
+ff_cap_clear:
+	__clear_bit(FF_HAPTIC, dev->ffbit);
+	__clear_bit(EV_FF, dev->evbit);
 	kfree(haptic->stop_effect.report_buf);
 	haptic->stop_effect.report_buf = NULL;
 buffer_free:
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 4e19a0c..154dc94 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -174,6 +174,7 @@ struct mt_device {
 	struct mt_class mtclass;	/* our mt device class */
 	struct timer_list release_timer;	/* to release sticky fingers */
 	struct hid_haptic_device *haptic;	/* haptic related configuration */
+	int haptic_init_error;
 	struct hid_device *hdev;	/* hid_device we're attached to */
 	unsigned long mt_io_flags;	/* mt flags (MT_IO_FLAGS_RUNNING) */
 	unsigned long *active_slots;	/* bitmap of slots with an active
@@ -1958,8 +1959,16 @@ static int mt_input_configured(struct hid_device *hdev, struct hid_input *hi)
 	if (td->is_haptic_touchpad && (td->mtclass.name == MT_CLS_WIN_8 ||
 	    td->mtclass.name == MT_CLS_WIN_8_FORCE_MULTI_INPUT ||
 	    td->mtclass.name == MT_CLS_WIN_8_KEEP_LATENCY_ON_CLOSE)) {
-		if (hid_haptic_input_configured(hdev, td->haptic, hi) == 0)
-			td->is_haptic_touchpad = false;
+		if (hi->application == HID_DG_TOUCHPAD) {
+			ret = hid_haptic_input_configured(hdev, td->haptic, hi);
+			if (ret < 0) {
+				td->haptic_init_error = ret;
+				td->is_haptic_touchpad = false;
+				return ret;
+			}
+			if (ret == 0)
+				td->is_haptic_touchpad = false;
+		}
 	} else {
 		td->is_haptic_touchpad = false;
 	}
@@ -2197,6 +2206,11 @@ 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;
+	if (td->haptic_init_error) {
+		ret = td->haptic_init_error;
+		hid_hw_stop(hdev);
+		return ret;
+	}
 
 	ret = sysfs_create_group(&hdev->dev.kobj, &mt_attribute_group);
 	if (ret)
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v10 2/2] HID: multitouch: give FF core ownership of haptic state
  2026-10-17 14:38 [PATCH v10 0/2] HID: multitouch: fix haptic lifetime on delayed input release Aldo Ariel Panzardo
  2026-10-17 14:38 ` [PATCH v10 1/2] HID: haptic: make FF creation the last fallible init step Aldo Ariel Panzardo
@ 2026-10-17 14:38 ` Aldo Ariel Panzardo
  2026-10-17 14:52   ` sashiko-bot
  1 sibling, 1 reply; 4+ messages in thread
From: Aldo Ariel Panzardo @ 2026-10-17 14:38 UTC (permalink / raw)
  To: jikos, bentiss
  Cc: dmitry.torokhov, linux-input, linux-kernel, stable,
	Aldo Ariel Panzardo

An open evdev fd can delay input_ff_destroy() until after HID unbind.
The descriptor mapping state is devres managed and cannot serve as
ff->private for that lifetime.

Clone the mapped state in hid_haptic_init(). Transfer the allocation
to ff->private only after input_ff_create() succeeds; the input core
then frees it at final input device release. Scope cleanup releases
the clone and its subresources on initialization failure.

On devices with multiple inputs, attach FF to the first input with a
multitouch touchpad report. An earlier touchpad report without a
multitouch collection must not consume the single FF instance. Other
inputs skip FF initialization and do not acquire INPUT_PROP_PRESSUREPAD
from the haptic setup. The HID reports are shared, so multiple FF
instances would write the same fields under different mutexes.

Fixes: 8d0bf7908b5a ("HID: multitouch: add haptic multitouch support")
Cc: stable@vger.kernel.org
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
 drivers/hid/hid-haptic.c     | 131 ++++++++++++++++++-----------------
 drivers/hid/hid-haptic.h     |   6 +-
 drivers/hid/hid-multitouch.c |  38 +++++-----
 3 files changed, 91 insertions(+), 84 deletions(-)

diff --git a/drivers/hid/hid-haptic.c b/drivers/hid/hid-haptic.c
index 0a6126e..ec2a891 100644
--- a/drivers/hid/hid-haptic.c
+++ b/drivers/hid/hid-haptic.c
@@ -5,6 +5,7 @@
  *  Copyright (c) 2021 Angela Czubak <acz@semihalf.com>
  */
 
+#include <linux/cleanup.h>
 #include <linux/input/mt.h>
 #include <linux/module.h>
 
@@ -90,7 +91,9 @@ int hid_haptic_input_configured(struct hid_device *hdev,
 	if (!haptic->auto_trigger_report || !haptic->manual_trigger_report)
 		return 0;
 
-	__set_bit(INPUT_PROP_PRESSUREPAD, hi->input->propbit);
+	/* The HID reports are shared; create only one FF instance. */
+	if (haptic->input_dev)
+		return 1;
 
 	error = hid_haptic_init(hdev, haptic, hi->input);
 	if (error) {
@@ -99,6 +102,9 @@ int hid_haptic_input_configured(struct hid_device *hdev,
 		return error;
 	}
 
+	__set_bit(INPUT_PROP_PRESSUREPAD, hi->input->propbit);
+	haptic->input_dev = hi->input;
+
 	return 1;
 }
 EXPORT_SYMBOL_GPL(hid_haptic_input_configured);
@@ -377,53 +383,76 @@ static int hid_haptic_erase(struct input_dev *dev, int effect_id)
 	return 0;
 }
 
-static void hid_haptic_stop(struct ff_device *ff)
+static void hid_haptic_stop_work(struct hid_haptic_device *haptic)
 {
-	struct hid_haptic_device *haptic = ff->private;
-
 	if (haptic->wq) {
 		destroy_workqueue(haptic->wq);
 		haptic->wq = NULL;
 	}
 }
 
-static void hid_haptic_destroy(struct ff_device *ff)
+static void hid_haptic_stop(struct ff_device *ff)
+{
+	hid_haptic_stop_work(ff->private);
+}
+
+static void hid_haptic_free_resources(struct hid_haptic_device *haptic)
 {
-	struct hid_haptic_device *haptic = ff->private;
 	int r;
 
-	hid_haptic_stop(ff);
+	hid_haptic_stop_work(haptic);
 
 	kfree(haptic->stop_effect.report_buf);
-	haptic->stop_effect.report_buf = NULL;
 
 	if (haptic->effect) {
-		for (r = 0; r < ff->max_effects; r++)
+		for (r = 0; r < FF_MAX_EFFECTS; r++)
 			kfree(haptic->effect[r].report_buf);
 		kfree(haptic->effect);
 	}
-	haptic->effect = NULL;
 
 	kfree(haptic->duration_map);
-	haptic->duration_map = NULL;
-
 	kfree(haptic->hid_usage_map);
-	haptic->hid_usage_map = NULL;
+}
+
+static void hid_haptic_free_state(struct hid_haptic_device *haptic)
+{
+	if (haptic) {
+		hid_haptic_free_resources(haptic);
+		kfree(haptic);
+	}
+}
+
+DEFINE_FREE(hid_haptic_state, struct hid_haptic_device *, hid_haptic_free_state(_T));
+
+static void hid_haptic_destroy(struct ff_device *ff)
+{
+	struct hid_haptic_device *haptic = ff->private;
+
+	hid_haptic_free_resources(haptic);
 
 	module_put(THIS_MODULE);
 }
 
 int hid_haptic_init(struct hid_device *hdev,
-		    struct hid_haptic_device *haptic,
+		    const struct hid_haptic_device *config,
 		    struct input_dev *dev)
 {
+	struct hid_haptic_device *haptic __free(hid_haptic_state) =
+		kmemdup(config, sizeof(*config), GFP_KERNEL);
 	struct ff_device *ff;
-	int ret = 0, r;
+	int ret, r;
 	struct ff_haptic_effect stop_effect = {
 		.hid_usage = HID_HP_WAVEFORMSTOP & HID_USAGE,
 	};
-	const char *prefix = "hid-haptic";
-	char *name;
+	if (!haptic)
+		return -ENOMEM;
+
+	/* The mapping state does not own these allocations. */
+	haptic->hid_usage_map = NULL;
+	haptic->duration_map = NULL;
+	haptic->effect = NULL;
+	haptic->stop_effect.report_buf = NULL;
+	haptic->wq = NULL;
 
 	haptic->hdev = hdev;
 	haptic->max_waveform_id = max(2u, haptic->max_waveform_id);
@@ -431,16 +460,12 @@ int hid_haptic_init(struct hid_device *hdev,
 
 	haptic->hid_usage_map = kcalloc(haptic->max_waveform_id + 1,
 					sizeof(u16), GFP_KERNEL);
-	if (!haptic->hid_usage_map) {
-		ret = -ENOMEM;
-		goto exit;
-	}
+	if (!haptic->hid_usage_map)
+		return -ENOMEM;
 	haptic->duration_map = kcalloc(haptic->max_duration_id + 1,
 				       sizeof(u32), GFP_KERNEL);
-	if (!haptic->duration_map) {
-		ret = -ENOMEM;
-		goto usage_map;
-	}
+	if (!haptic->duration_map)
+		return -ENOMEM;
 
 	if (haptic->max_waveform_id != haptic->max_duration_id)
 		dev_warn(&hdev->dev,
@@ -460,21 +485,19 @@ int hid_haptic_init(struct hid_device *hdev,
 	haptic->manual_trigger_report_len =
 		hid_report_len(haptic->manual_trigger_report);
 	mutex_init(&haptic->manual_trigger_mutex);
-	name = kmalloc(strlen(prefix) + strlen(hdev->name) + 2, GFP_KERNEL);
-	if (name) {
-		sprintf(name, "%s %s", prefix, hdev->name);
+	{
+		char *name __free(kfree) = kasprintf(GFP_KERNEL,
+						     "hid-haptic %s", hdev->name);
+
+		if (!name)
+			return -ENOMEM;
 		haptic->wq = create_singlethread_workqueue(name);
-		kfree(name);
-	}
-	if (!haptic->wq) {
-		ret = -ENOMEM;
-		goto duration_map;
 	}
+	if (!haptic->wq)
+		return -ENOMEM;
 	haptic->effect = kzalloc_objs(struct hid_haptic_effect, FF_MAX_EFFECTS);
-	if (!haptic->effect) {
-		ret = -ENOMEM;
-		goto output_queue;
-	}
+	if (!haptic->effect)
+		return -ENOMEM;
 	for (r = 0; r < FF_MAX_EFFECTS; r++) {
 		haptic->effect[r].report_buf =
 			hid_alloc_report_buf(haptic->manual_trigger_report,
@@ -482,8 +505,7 @@ int hid_haptic_init(struct hid_device *hdev,
 		if (!haptic->effect[r].report_buf) {
 			dev_err(&hdev->dev,
 				"Failed to allocate a buffer for an effect.\n");
-			ret = -ENOMEM;
-			goto buffer_free;
+			return -ENOMEM;
 		}
 		haptic->effect[r].input_dev = dev;
 		INIT_WORK(&haptic->effect[r].work, haptic_work_handler);
@@ -494,8 +516,7 @@ int hid_haptic_init(struct hid_device *hdev,
 	if (!haptic->stop_effect.report_buf) {
 		dev_err(&hdev->dev,
 			"Failed to allocate a buffer for stop effect.\n");
-		ret = -ENOMEM;
-		goto buffer_free;
+		return -ENOMEM;
 	}
 	haptic->stop_effect.input_dev = dev;
 	INIT_WORK(&haptic->stop_effect.work, haptic_work_handler);
@@ -507,7 +528,9 @@ int hid_haptic_init(struct hid_device *hdev,
 	ret = input_ff_create(dev, FF_MAX_EFFECTS);
 	if (ret) {
 		dev_err(&hdev->dev, "Failed to create ff device.\n");
-		goto ff_cap_clear;
+		__clear_bit(FF_HAPTIC, dev->ffbit);
+		__clear_bit(EV_FF, dev->evbit);
+		return ret;
 	}
 
 	ff = dev->ff;
@@ -517,30 +540,8 @@ int hid_haptic_init(struct hid_device *hdev,
 	ff->stop = hid_haptic_stop;
 	ff->destroy = hid_haptic_destroy;
 	__module_get(THIS_MODULE);
-	ff->private = haptic;
+	ff->private = no_free_ptr(haptic);
 	return 0;
-
-ff_cap_clear:
-	__clear_bit(FF_HAPTIC, dev->ffbit);
-	__clear_bit(EV_FF, dev->evbit);
-	kfree(haptic->stop_effect.report_buf);
-	haptic->stop_effect.report_buf = NULL;
-buffer_free:
-	while (--r >= 0)
-		kfree(haptic->effect[r].report_buf);
-	kfree(haptic->effect);
-	haptic->effect = NULL;
-output_queue:
-	destroy_workqueue(haptic->wq);
-	haptic->wq = NULL;
-duration_map:
-	kfree(haptic->duration_map);
-	haptic->duration_map = NULL;
-usage_map:
-	kfree(haptic->hid_usage_map);
-	haptic->hid_usage_map = NULL;
-exit:
-	return ret;
 }
 EXPORT_SYMBOL_GPL(hid_haptic_init);
 
diff --git a/drivers/hid/hid-haptic.h b/drivers/hid/hid-haptic.h
index 6332991..661b605 100644
--- a/drivers/hid/hid-haptic.h
+++ b/drivers/hid/hid-haptic.h
@@ -69,7 +69,8 @@ int hid_haptic_input_mapping(struct hid_device *hdev,
 int hid_haptic_input_configured(struct hid_device *hdev,
 				struct hid_haptic_device *haptic,
 				struct hid_input *hi);
-int hid_haptic_init(struct hid_device *hdev, struct hid_haptic_device *haptic,
+int hid_haptic_init(struct hid_device *hdev,
+		    const struct hid_haptic_device *config,
 		    struct input_dev *dev);
 void hid_haptic_handle_press_release(struct hid_haptic_device *haptic);
 void hid_haptic_pressure_reset(struct hid_haptic_device *haptic);
@@ -108,7 +109,8 @@ static inline
 void hid_haptic_reset(struct hid_device *hdev, struct hid_haptic_device *haptic)
 {}
 static inline
-int hid_haptic_init(struct hid_device *hdev, struct hid_haptic_device *haptic,
+int hid_haptic_init(struct hid_device *hdev,
+		    const struct hid_haptic_device *config,
 		    struct input_dev *dev)
 {
 	return 0;
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 154dc94..e53389e 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -1954,25 +1954,9 @@ static int mt_input_configured(struct hid_device *hdev, struct hid_input *hi)
 	struct mt_report_data *rdata;
 	struct mt_application *mt_application = NULL;
 	struct hid_report *report;
+	bool has_mt_touchpad = false;
 	int ret;
 
-	if (td->is_haptic_touchpad && (td->mtclass.name == MT_CLS_WIN_8 ||
-	    td->mtclass.name == MT_CLS_WIN_8_FORCE_MULTI_INPUT ||
-	    td->mtclass.name == MT_CLS_WIN_8_KEEP_LATENCY_ON_CLOSE)) {
-		if (hi->application == HID_DG_TOUCHPAD) {
-			ret = hid_haptic_input_configured(hdev, td->haptic, hi);
-			if (ret < 0) {
-				td->haptic_init_error = ret;
-				td->is_haptic_touchpad = false;
-				return ret;
-			}
-			if (ret == 0)
-				td->is_haptic_touchpad = false;
-		}
-	} else {
-		td->is_haptic_touchpad = false;
-	}
-
 	list_for_each_entry(report, &hi->reports, hidinput_list) {
 		rdata = mt_find_report_data(td, report);
 		if (!rdata) {
@@ -1987,9 +1971,29 @@ static int mt_input_configured(struct hid_device *hdev, struct hid_input *hi)
 							mt_application);
 			if (ret)
 				return ret;
+			if (hi->application == HID_DG_TOUCHPAD)
+				has_mt_touchpad = true;
 		}
 	}
 
+	if (td->is_haptic_touchpad &&
+	    (td->mtclass.name == MT_CLS_WIN_8 ||
+	     td->mtclass.name == MT_CLS_WIN_8_FORCE_MULTI_INPUT ||
+	     td->mtclass.name == MT_CLS_WIN_8_KEEP_LATENCY_ON_CLOSE)) {
+		if (has_mt_touchpad) {
+			ret = hid_haptic_input_configured(hdev, td->haptic, hi);
+			if (ret < 0) {
+				td->haptic_init_error = ret;
+				td->is_haptic_touchpad = false;
+				return ret;
+			}
+			if (ret == 0)
+				td->is_haptic_touchpad = false;
+		}
+	} else {
+		td->is_haptic_touchpad = false;
+	}
+
 	switch (hi->application) {
 	case HID_GD_KEYBOARD:
 	case HID_GD_KEYPAD:
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v10 2/2] HID: multitouch: give FF core ownership of haptic state
  2026-10-17 14:38 ` [PATCH v10 2/2] HID: multitouch: give FF core ownership of haptic state Aldo Ariel Panzardo
@ 2026-10-17 14:52   ` sashiko-bot
  0 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-10-17 14:52 UTC (permalink / raw)
  To: Aldo Ariel Panzardo; +Cc: linux-input, dmitry.torokhov

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] HID: multitouch: mt_input_configured() validates is_haptic_touchpad too late
--

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

HID: multitouch: give FF core ownership of haptic state

This commit clones the mapped haptic state in hid_haptic_init() and
transfers ownership to ff->private so the input core frees it on final
input device release. It also restricts FF initialization to the first
input device containing a multitouch touchpad report.

> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 154dc948b156e..e53389e28ad53 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -1954,25 +1954,9 @@ static int mt_input_configured(struct hid_device *hdev, struct hid_input *hi)
>  	struct mt_report_data *rdata;
>  	struct mt_application *mt_application = NULL;
>  	struct hid_report *report;
> +	bool has_mt_touchpad = false;
>  	int ret;
>  
> -	if (td->is_haptic_touchpad && (td->mtclass.name == MT_CLS_WIN_8 ||
> -	    td->mtclass.name == MT_CLS_WIN_8_FORCE_MULTI_INPUT ||
> -	    td->mtclass.name == MT_CLS_WIN_8_KEEP_LATENCY_ON_CLOSE)) {
> -		if (hi->application == HID_DG_TOUCHPAD) {
> -			ret = hid_haptic_input_configured(hdev, td->haptic, hi);
> -			if (ret < 0) {
> -				td->haptic_init_error = ret;
> -				td->is_haptic_touchpad = false;
> -				return ret;
> -			}
> -			if (ret == 0)
> -				td->is_haptic_touchpad = false;
> -		}
> -	} else {
> -		td->is_haptic_touchpad = false;
> -	}
> -
>  	list_for_each_entry(report, &hi->reports, hidinput_list) {
>  		rdata = mt_find_report_data(td, report);
>  		if (!rdata) {
> @@ -1987,9 +1971,29 @@ static int mt_input_configured(struct hid_device *hdev, struct hid_input *hi)
>  							mt_application);
>  			if (ret)
>  				return ret;
> +			if (hi->application == HID_DG_TOUCHPAD)
> +				has_mt_touchpad = true;
>  		}
>  	}
>  
> +	if (td->is_haptic_touchpad &&
> +	    (td->mtclass.name == MT_CLS_WIN_8 ||
> +	     td->mtclass.name == MT_CLS_WIN_8_FORCE_MULTI_INPUT ||
> +	     td->mtclass.name == MT_CLS_WIN_8_KEEP_LATENCY_ON_CLOSE)) {
> +		if (has_mt_touchpad) {
> +			ret = hid_haptic_input_configured(hdev, td->haptic, hi);
> +			if (ret < 0) {
> +				td->haptic_init_error = ret;
> +				td->is_haptic_touchpad = false;
> +				return ret;
> +			}
> +			if (ret == 0)
> +				td->is_haptic_touchpad = false;
> +		}
> +	} else {
> +		td->is_haptic_touchpad = false;
> +	}

[Severity: Medium]
Does moving the td->is_haptic_touchpad validation after the hi->reports loop
cause mt_touch_input_configured() to observe an unvalidated
td->is_haptic_touchpad state?

During descriptor mapping, mt_touch_input_mapping() sets
td->is_haptic_touchpad = true whenever hid_haptic_check_pressure_unit() sees
a HID_DG_TIPPRESSURE field with gram or newton units, without checking
td->mtclass.name or whether Simple Haptic Controller trigger reports exist:

drivers/hid/hid-multitouch.c:mt_touch_input_mapping()
	case HID_DG_TIPPRESSURE:
		set_abs(hi->input, ABS_MT_PRESSURE, field,
			cls->sn_pressure);
		td->is_haptic_touchpad =
			hid_haptic_check_pressure_unit(td->haptic,
						       hi, field);

Inside the hi->reports loop, mt_touch_input_configured() checks
td->is_haptic_touchpad and passes INPUT_MT_TOTAL_FORCE to
input_mt_init_slots():

drivers/hid/hid-multitouch.c:mt_touch_input_configured()
	if (td->is_haptic_touchpad)
		app->mt_flags |= INPUT_MT_TOTAL_FORCE;
	...
	ret = input_mt_init_slots(input, td->maxcontacts, app->mt_flags);

Because this now runs before mt_input_configured() checks td->mtclass.name
or calls hid_haptic_input_configured(), INPUT_MT_TOTAL_FORCE is already set in
input->mt->flags even if td->is_haptic_touchpad is cleared to false right
after the loop (or on non-haptic MT inputs of a multi-input device), causing
input_mt_report_pointer_emulation() to sum ABS_MT_PRESSURE across contacts.

Also, if a Win8-class device sets td->is_haptic_touchpad on a non-touchpad
MT collection and has no HID_DG_TOUCHPAD MT collection, has_mt_touchpad will
be false for all inputs, leaving td->is_haptic_touchpad true so mt_probe()
keeps td->haptic and mt_process_slot() / mt_sync_frame() update
td->haptic->pressure_sum on every frame.

>  
>  	switch (hi->application) {
>  	case HID_GD_KEYBOARD:
>  	case HID_GD_KEYPAD:

[ ... ]

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-17 14:52 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-17 14:38 [PATCH v10 0/2] HID: multitouch: fix haptic lifetime on delayed input release Aldo Ariel Panzardo
2026-10-17 14:38 ` [PATCH v10 1/2] HID: haptic: make FF creation the last fallible init step Aldo Ariel Panzardo
2026-10-17 14:38 ` [PATCH v10 2/2] HID: multitouch: give FF core ownership of haptic state Aldo Ariel Panzardo
2026-10-17 14:52   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox