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@rong.moe>
---
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 73446a123b65..9817d661f93d 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);@@ -427,7 +435,7 @@ int hid_haptic_init(struct hid_device *hdev,
haptic->hid_usage_map = kzalloc_objs(u16, haptic->max_waveform_id + 1);
if (!haptic->hid_usage_map) {
ret = -ENOMEM;
- goto exit;
+ goto put_device;
}
haptic->duration_map = kzalloc_objs(u32, haptic->max_duration_id + 1);
if (!haptic->duration_map) {@@ -494,8 +502,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");@@ -508,23 +514,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);@@ -543,7 +534,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