[PATCH 2/9] HID: haptic: Get rid of input_ff_destroy() in error paths

[Rong Zhang]

It's painful to have error paths after calling input_ff_create(),
because input_ff_destroy() frees too much things but does not restore
some other things.

On get_device failure, the module's bail-out path calls module_put()
right before hid_haptic_destroy() calling it again, causing refcount
underflow.

Besides, every error path calling input_ff_destroy() frees ff->private,
while the others don't. This divergence has caused quite a lot of
trouble.

Get the module and the device beforehand, so that there is no error path
after input_ff_create(), eliminating the call to input_ff_destroy(). In
this manner, we don't need to restore dev->flush and dev->event either.

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

diff --git a/drivers/hid/hid-haptic.c b/drivers/hid/hid-haptic.c
index 133cf2dcd4ac..9cbacc827dc9 100644
--- a/drivers/hid/hid-haptic.c
+++ b/drivers/hid/hid-haptic.c
@@ -386,9 +386,6 @@ static void hid_haptic_destroy(struct ff_device *ff)
 	destroy_workqueue(haptic->wq);
 	haptic->wq = NULL;
 
-	if (hdev)
-		put_device(&hdev->dev);
-
 	kfree(haptic->stop_effect.report_buf);
 	haptic->stop_effect.report_buf = NULL;
 
@@ -405,6 +402,9 @@ static void hid_haptic_destroy(struct ff_device *ff)
 	kfree(haptic->hid_usage_map);
 	haptic->hid_usage_map = NULL;
 
+	if (hdev)
+		put_device(&hdev->dev);
+
 	module_put(THIS_MODULE);
 }
 
@@ -417,8 +417,16 @@ int hid_haptic_init(struct hid_device *hdev,
 	struct ff_haptic_effect stop_effect = {
 		.hid_usage = HID_HP_WAVEFORMSTOP & HID_USAGE,
 	};
-	int (*flush)(struct input_dev *dev, struct file *file);
-	int (*event)(struct input_dev *dev, unsigned int type, unsigned int code, int value);
+
+	if (!try_module_get(THIS_MODULE)) {
+		dev_err(&hdev->dev, "Failed to increase module count.\n");
+		return -ENXIO;
+	}
+	if (!get_device(&hdev->dev)) {
+		dev_err(&hdev->dev, "Failed to get hdev device.\n");
+		ret = -ENXIO;
+		goto exit;
+	}
 
 	haptic->hdev = hdev;
 	haptic->max_waveform_id = max(2u, haptic->max_waveform_id);
@@ -428,7 +436,7 @@ int hid_haptic_init(struct hid_device *hdev,
 					sizeof(u16), GFP_KERNEL);
 	if (!haptic->hid_usage_map) {
 		ret = -ENOMEM;
-		goto exit;
+		goto put_device;
 	}
 	haptic->duration_map = kcalloc(haptic->max_duration_id + 1,
 				       sizeof(u32), GFP_KERNEL);
@@ -496,8 +504,6 @@ 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");
@@ -510,23 +516,8 @@ 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;
-	}
 	return 0;
 
-input_free:
-	input_ff_destroy(dev);
-	/* Restore dev flush and event */
-	dev->flush = flush;
-	dev->event = event;
-	return ret;
 reset_evbit:
 	__clear_bit(EV_FF, dev->evbit);
 	destroy_workqueue(haptic->wq);
@@ -545,7 +536,10 @@ int hid_haptic_init(struct hid_device *hdev,
 usage_map:
 	kfree(haptic->hid_usage_map);
 	haptic->hid_usage_map = NULL;
+put_device:
+	put_device(&hdev->dev);
 exit:
+	module_put(THIS_MODULE);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(hid_haptic_init);

-- 
2.55.0