[PATCH 1/9] HID: haptic: Fix use-after-free when draining workqueue

[Rong Zhang]

The haptic->wq is destroyed after freeing effects and their report
buffers. This is too late. If any work is still pending or running
before the workqueue destruction, haptic_work_handler() will access the
just-freed contents, causing a use-after-free. If put_device() happens
to decrement the reference count to zero, it may access freed devres-
managed resources, too.

Fix it by destroying the workqueue before putting the HID device and
freeing haptic resources. The allocation sequence is also adjusted to
the reverse.

Since the workqueue allocation is anyway touched, also replace the
legacy creation API with the modern alloc_ordered_workqueue(). In this
manner, the temporary buffer on heap can be replaced by the API's
formatting.

Also call __clear_bit(EV_FF, dev->evbit) in the bail-out path to match
the behavior of input_ff_destroy().

Fixes: 344ff3584957 ("HID: haptic: initialize haptic device")
Signed-off-by: Rong Zhang <i@xxxxxxxx>
---
 drivers/hid/hid-haptic.c | 36 ++++++++++++++++--------------------
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/drivers/hid/hid-haptic.c b/drivers/hid/hid-haptic.c
index 8760eeb08b2c..133cf2dcd4ac 100644
--- a/drivers/hid/hid-haptic.c
+++ b/drivers/hid/hid-haptic.c
@@ -383,6 +383,9 @@ static void hid_haptic_destroy(struct ff_device *ff)
 	struct hid_device *hdev = haptic->hdev;
 	int r;
 
+	destroy_workqueue(haptic->wq);
+	haptic->wq = NULL;
+
 	if (hdev)
 		put_device(&hdev->dev);
 
@@ -396,9 +399,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;
 
@@ -417,8 +417,6 @@ int hid_haptic_init(struct hid_device *hdev,
 	struct ff_haptic_effect stop_effect = {
 		.hid_usage = HID_HP_WAVEFORMSTOP & HID_USAGE,
 	};
-	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);
 
@@ -457,20 +455,11 @@ 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);
-		haptic->wq = create_singlethread_workqueue(name);
-		kfree(name);
-	}
-	if (!haptic->wq) {
-		ret = -ENOMEM;
-		goto duration_map;
-	}
+
 	haptic->effect = kzalloc_objs(struct hid_haptic_effect, FF_MAX_EFFECTS);
 	if (!haptic->effect) {
 		ret = -ENOMEM;
-		goto output_queue;
+		goto duration_map;
 	}
 	for (r = 0; r < FF_MAX_EFFECTS; r++) {
 		haptic->effect[r].report_buf =
@@ -499,6 +488,12 @@ int hid_haptic_init(struct hid_device *hdev,
 	fill_effect_buf(haptic, &stop_effect, &haptic->stop_effect,
 			HID_HAPTIC_ORDINAL_WAVEFORMSTOP);
 
+	haptic->wq = alloc_ordered_workqueue("hid-haptic %s", WQ_MEM_RECLAIM, hdev->name);
+	if (!haptic->wq) {
+		ret = -ENOMEM;
+		goto stop_buffer_free;
+	}
+
 	input_set_capability(dev, EV_FF, FF_HAPTIC);
 
 	flush = dev->flush;
@@ -506,7 +501,7 @@ 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 stop_buffer_free;
+		goto reset_evbit;
 	}
 
 	ff = dev->ff;
@@ -532,6 +527,10 @@ int hid_haptic_init(struct hid_device *hdev,
 	dev->flush = flush;
 	dev->event = event;
 	return ret;
+reset_evbit:
+	__clear_bit(EV_FF, dev->evbit);
+	destroy_workqueue(haptic->wq);
+	haptic->wq = NULL;
 stop_buffer_free:
 	kfree(haptic->stop_effect.report_buf);
 	haptic->stop_effect.report_buf = NULL;
@@ -540,9 +539,6 @@ int hid_haptic_init(struct hid_device *hdev,
 		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;

-- 
2.55.0