[PATCH v3 4/9] HID: haptic, multitouch: Fix haptic device double-free and unnecessary allocation

[Rong Zhang <i@rong.moe>]

The haptic device is allocated by devm_kzalloc(), which will be freed by
devres on HID device release. At that time, the ff core has already
freed it on input device release previously, causing double-free.

Allocate the haptic device using kzalloc_obj() to ensure its release can
be safely delegates to the ff core.

The allocation code is also moved to hid-haptic.h, so that it can be
replaced by a stub in !CONFIG_HID_HAPTIC builds, eliminating the
meaningless alloc-and-free sequence.

Fixes: 8d0bf7908b5a ("HID: multitouch: add haptic multitouch support")
Signed-off-by: Rong Zhang <i@rong.moe>
---
Changes in v3:
- Correct the Fixes: tag (thanks Sashiko)
---
 drivers/hid/hid-haptic.h     | 17 +++++++++++++++++
 drivers/hid/hid-multitouch.c | 39 +++++++++++++++++++++++++++++----------
 2 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/drivers/hid/hid-haptic.h b/drivers/hid/hid-haptic.h
index cd4d9c74d2dc..7878cd73ccdb 100644
--- a/drivers/hid/hid-haptic.h
+++ b/drivers/hid/hid-haptic.h
@@ -5,6 +5,7 @@
  *  Copyright (c) 2021 Angela Czubak <acz@semihalf.com>
  */
 
+#include <linux/err.h>
 #include <linux/hid.h>
 
 #define HID_HAPTIC_ORDINAL_WAVEFORMNONE 1
@@ -54,6 +55,17 @@ struct hid_haptic_device {
 };
 
 #if IS_ENABLED(CONFIG_HID_HAPTIC)
+static inline struct hid_haptic_device *hid_haptic_alloc(struct hid_device *hdev)
+{
+	struct hid_haptic_device *haptic;
+
+	haptic = kzalloc_obj(struct hid_haptic_device);
+	if (haptic)
+		haptic->hdev = hdev;
+
+	return haptic;
+}
+
 static inline bool hid_haptic_connected_to_input(struct hid_haptic_device *haptic,
 						 struct input_dev *input)
 {
@@ -80,6 +92,11 @@ void hid_haptic_pressure_reset(struct hid_haptic_device *haptic);
 void hid_haptic_pressure_increase(struct hid_haptic_device *haptic,
 				  __s32 pressure);
 #else
+static inline struct hid_haptic_device *hid_haptic_alloc(struct hid_device *hdev)
+{
+	return ERR_PTR(-EOPNOTSUPP);
+}
+
 static inline bool hid_haptic_connected_to_input(struct hid_haptic_device *haptic,
 						 struct input_dev *input)
 {
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 5eea774dee44..a9c8396c804e 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -34,6 +34,7 @@
 #include <linux/bitmap.h>
 #include <linux/bits.h>
 #include <linux/device.h>
+#include <linux/err.h>
 #include <linux/hid.h>
 #include <linux/module.h>
 #include <linux/slab.h>
@@ -2129,11 +2130,18 @@ 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_obj(&hdev->dev, *(td->haptic));
-	if (!td->haptic)
+
+	td->haptic = hid_haptic_alloc(hdev);
+	if (IS_ERR(td->haptic)) {
+		ret = PTR_ERR(td->haptic);
+		if (ret == -EOPNOTSUPP)
+			td->haptic = NULL;
+		else
+			return ret;
+	} else if (!td->haptic) {
 		return -ENOMEM;
+	}
 
-	td->haptic->hdev = hdev;
 	td->hdev = hdev;
 	td->mtclass = *mtclass;
 	td->inputmode_value = MT_INPUTMODE_TOUCHSCREEN;
@@ -2178,12 +2186,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 out;
 
 	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 out;
+	}
 
 	if (mtclass->quirks & MT_QUIRK_FIX_CONST_CONTACT_ID)
 		mt_fix_const_fields(hdev, HID_DG_CONTACTID);
@@ -2193,7 +2203,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 out;
 
 	ret = sysfs_create_group(&hdev->dev.kobj, &mt_attribute_group);
 	if (ret)
@@ -2202,10 +2212,19 @@ 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);
+	ret = 0;
 
-	return 0;
+out:
+	/*
+	 * Don't double free when the haptic device has been delegated to the ff
+	 * core.
+	 */
+	if (td->haptic && !td->is_haptic_touchpad) {
+		kfree(td->haptic);
+		td->haptic = NULL;
+	}
+
+	return ret;
 }
 
 static int mt_suspend(struct hid_device *hdev, pm_message_t state)

-- 
2.55.0