lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20190328155151.577687-1-benjamin.tissoires@redhat.com>
Date:   Thu, 28 Mar 2019 16:51:51 +0100
From:   Benjamin Tissoires <benjamin.tissoires@...hat.com>
To:     Jiri Kosina <jikos@...nel.org>,
        Bruno Prémont <bonbons@...ux-vserver.org>,
        Jonathan Cameron <jic23@...nel.org>,
        Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
        Hans de Goede <hdegoede@...hat.com>,
        Jason Gerecke <killertofu@...il.com>,
        Ping Cheng <pinglinux@...il.com>
Cc:     linux-input@...r.kernel.org, linux-kernel@...r.kernel.org,
        Benjamin Tissoires <benjamin.tissoires@...hat.com>
Subject: [PATCH] HID: force setting drvdata to NULL when removing the driver

Or when the probe failed.

This is a common pattern in the HID drivers to reset the drvdata. Some
do it properly, some do it only in case of failure.
Anyway, it's never a good thing to have breadcrumbs, so force a clean
state when removing or when probe is failing.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@...hat.com>
---
 drivers/hid/hid-core.c           |  2 ++
 drivers/hid/hid-cougar.c         |  6 ++----
 drivers/hid/hid-gfrm.c           |  7 -------
 drivers/hid/hid-lenovo.c         |  2 --
 drivers/hid/hid-logitech-dj.c    |  2 --
 drivers/hid/hid-logitech-hidpp.c |  8 +++-----
 drivers/hid/hid-picolcd_core.c   |  7 +------
 drivers/hid/hid-sensor-hub.c     |  1 -
 drivers/hid/wacom_sys.c          | 18 +++++-------------
 9 files changed, 13 insertions(+), 40 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index b5a8af8daa74..011e49da4d63 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2203,6 +2203,7 @@ static int hid_device_probe(struct device *dev)
 		if (ret) {
 			hid_close_report(hdev);
 			hdev->driver = NULL;
+			hid_set_drvdata(hdev, NULL);
 		}
 	}
 unlock:
@@ -2231,6 +2232,7 @@ static int hid_device_remove(struct device *dev)
 		else /* default remove */
 			hid_hw_stop(hdev);
 		hid_close_report(hdev);
+		hid_set_drvdata(hdev, NULL);
 		hdev->driver = NULL;
 	}
 
diff --git a/drivers/hid/hid-cougar.c b/drivers/hid/hid-cougar.c
index e0bb7b34f3a4..4ff3bc1d25e2 100644
--- a/drivers/hid/hid-cougar.c
+++ b/drivers/hid/hid-cougar.c
@@ -207,7 +207,7 @@ static int cougar_probe(struct hid_device *hdev,
 	error = hid_parse(hdev);
 	if (error) {
 		hid_err(hdev, "parse failed\n");
-		goto fail;
+		return error;
 	}
 
 	if (hdev->collection->usage == COUGAR_VENDOR_USAGE) {
@@ -219,7 +219,7 @@ static int cougar_probe(struct hid_device *hdev,
 	error = hid_hw_start(hdev, connect_mask);
 	if (error) {
 		hid_err(hdev, "hw start failed\n");
-		goto fail;
+		return error;
 	}
 
 	error = cougar_bind_shared_data(hdev, cougar);
@@ -249,8 +249,6 @@ static int cougar_probe(struct hid_device *hdev,
 
 fail_stop_and_cleanup:
 	hid_hw_stop(hdev);
-fail:
-	hid_set_drvdata(hdev, NULL);
 	return error;
 }
 
diff --git a/drivers/hid/hid-gfrm.c b/drivers/hid/hid-gfrm.c
index cf477f8c8f4c..1a20997e7750 100644
--- a/drivers/hid/hid-gfrm.c
+++ b/drivers/hid/hid-gfrm.c
@@ -127,12 +127,6 @@ static int gfrm_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	return ret;
 }
 
-static void gfrm_remove(struct hid_device *hdev)
-{
-	hid_hw_stop(hdev);
-	hid_set_drvdata(hdev, NULL);
-}
-
 static const struct hid_device_id gfrm_devices[] = {
 	{ HID_BLUETOOTH_DEVICE(0x58, 0x2000),
 		.driver_data = GFRM100 },
@@ -146,7 +140,6 @@ static struct hid_driver gfrm_driver = {
 	.name = "gfrm",
 	.id_table = gfrm_devices,
 	.probe = gfrm_probe,
-	.remove = gfrm_remove,
 	.input_mapping = gfrm_input_mapping,
 	.raw_event = gfrm_raw_event,
 	.input_configured = gfrm_input_configured,
diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index eacc76d2ab96..cde9d22bb3ec 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -869,8 +869,6 @@ static void lenovo_remove_tpkbd(struct hid_device *hdev)
 
 	led_classdev_unregister(&data_pointer->led_micmute);
 	led_classdev_unregister(&data_pointer->led_mute);
-
-	hid_set_drvdata(hdev, NULL);
 }
 
 static void lenovo_remove_cptkbd(struct hid_device *hdev)
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 826fa1e1c8d9..a75101293755 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -1094,7 +1094,6 @@ static int logi_dj_probe(struct hid_device *hdev,
 hid_parse_fail:
 	kfifo_free(&djrcv_dev->notif_fifo);
 	kfree(djrcv_dev);
-	hid_set_drvdata(hdev, NULL);
 	return retval;
 
 }
@@ -1145,7 +1144,6 @@ static void logi_dj_remove(struct hid_device *hdev)
 
 	kfifo_free(&djrcv_dev->notif_fifo);
 	kfree(djrcv_dev);
-	hid_set_drvdata(hdev, NULL);
 }
 
 static const struct hid_device_id logi_dj_receivers[] = {
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 199cc256e9d9..b950a44157a2 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -3237,15 +3237,15 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP) {
 		ret = wtp_allocate(hdev, id);
 		if (ret)
-			goto allocate_fail;
+			return ret;
 	} else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560) {
 		ret = m560_allocate(hdev);
 		if (ret)
-			goto allocate_fail;
+			return ret;
 	} else if (hidpp->quirks & HIDPP_QUIRK_CLASS_K400) {
 		ret = k400_allocate(hdev);
 		if (ret)
-			goto allocate_fail;
+			return ret;
 	}
 
 	INIT_WORK(&hidpp->work, delayed_work_cb);
@@ -3343,8 +3343,6 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	sysfs_remove_group(&hdev->dev.kobj, &ps_attribute_group);
 	cancel_work_sync(&hidpp->work);
 	mutex_destroy(&hidpp->send_mutex);
-allocate_fail:
-	hid_set_drvdata(hdev, NULL);
 	return ret;
 }
 
diff --git a/drivers/hid/hid-picolcd_core.c b/drivers/hid/hid-picolcd_core.c
index c1b29a9eb41a..a5d30f267a7b 100644
--- a/drivers/hid/hid-picolcd_core.c
+++ b/drivers/hid/hid-picolcd_core.c
@@ -550,8 +550,7 @@ static int picolcd_probe(struct hid_device *hdev,
 	data = kzalloc(sizeof(struct picolcd_data), GFP_KERNEL);
 	if (data == NULL) {
 		hid_err(hdev, "can't allocate space for Minibox PicoLCD device data\n");
-		error = -ENOMEM;
-		goto err_no_cleanup;
+		return -ENOMEM;
 	}
 
 	spin_lock_init(&data->lock);
@@ -613,9 +612,6 @@ static int picolcd_probe(struct hid_device *hdev,
 	hid_hw_stop(hdev);
 err_cleanup_data:
 	kfree(data);
-err_no_cleanup:
-	hid_set_drvdata(hdev, NULL);
-
 	return error;
 }
 
@@ -651,7 +647,6 @@ static void picolcd_remove(struct hid_device *hdev)
 	picolcd_exit_cir(data);
 	picolcd_exit_keys(data);
 
-	hid_set_drvdata(hdev, NULL);
 	mutex_destroy(&data->mutex);
 	/* Finally, clean up the picolcd data itself */
 	kfree(data);
diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index 4256fdc5cd6d..a3db5d8b382d 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -755,7 +755,6 @@ static void sensor_hub_remove(struct hid_device *hdev)
 	}
 	spin_unlock_irqrestore(&data->lock, flags);
 	mfd_remove_devices(&hdev->dev);
-	hid_set_drvdata(hdev, NULL);
 	mutex_destroy(&data->mutex);
 }
 
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index a8633b1437b2..c2fb614bff44 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -2716,14 +2716,12 @@ static int wacom_probe(struct hid_device *hdev,
 	wacom_wac->features = *((struct wacom_features *)id->driver_data);
 	features = &wacom_wac->features;
 
-	if (features->check_for_hid_type && features->hid_type != hdev->type) {
-		error = -ENODEV;
-		goto fail;
-	}
+	if (features->check_for_hid_type && features->hid_type != hdev->type)
+		return -ENODEV;
 
 	error = kfifo_alloc(&wacom_wac->pen_fifo, WACOM_PKGLEN_MAX, GFP_KERNEL);
 	if (error)
-		goto fail;
+		return error;
 
 	wacom_wac->hid_data.inputmode = -1;
 	wacom_wac->mode_report = -1;
@@ -2741,12 +2739,12 @@ static int wacom_probe(struct hid_device *hdev,
 	error = hid_parse(hdev);
 	if (error) {
 		hid_err(hdev, "parse failed\n");
-		goto fail;
+		return error;
 	}
 
 	error = wacom_parse_and_register(wacom, false);
 	if (error)
-		goto fail;
+		return error;
 
 	if (hdev->bus == BUS_BLUETOOTH) {
 		error = device_create_file(&hdev->dev, &dev_attr_speed);
@@ -2757,10 +2755,6 @@ static int wacom_probe(struct hid_device *hdev,
 	}
 
 	return 0;
-
-fail:
-	hid_set_drvdata(hdev, NULL);
-	return error;
 }
 
 static void wacom_remove(struct hid_device *hdev)
@@ -2789,8 +2783,6 @@ static void wacom_remove(struct hid_device *hdev)
 		wacom_release_resources(wacom);
 
 	kfifo_free(&wacom_wac->pen_fifo);
-
-	hid_set_drvdata(hdev, NULL);
 }
 
 #ifdef CONFIG_PM
-- 
2.19.2

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ