linux-kernel - [PATCH] HID: multitouch: prevent memleak with the allocated name

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]

Date:	Wed, 29 May 2013 10:45:09 +0200
From:	Benjamin Tissoires <benjamin.tissoires@...hat.com>
To:	Benjamin Tissoires <benjamin.tissoires@...il.com>,
	Henrik Rydberg <rydberg@...omail.se>,
	Jiri Kosina <jkosina@...e.cz>,
	Stephane Chatty <chatty@...c.fr>, linux-input@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: [PATCH] HID: multitouch: prevent memleak with the allocated name

mt_free_input_name() was never called during .remove():
hid_hw_stop() removes the hid_input items in hdev->inputs, and so the
list is therefore empty after the call. In the end, we never free the
special names that has been allocated during .probe().

Restore the original name before freeing it to avoid acessing already
freed pointer.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@...hat.com>
---

Hi Jiri,

I just spotted this one yesterday... My guess is that this way is safe (without
a locking mechanism to prevent accessing hi->input->name), but I'm not 100% sure.

Cheers,
Benjamin

 drivers/hid/hid-multitouch.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index d99b959..cb0e361 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -264,9 +264,12 @@ static struct mt_class mt_classes[] = {
 static void mt_free_input_name(struct hid_input *hi)
 {
 	struct hid_device *hdev = hi->report->device;
+	const char *name = hi->input->name;
 
-	if (hi->input->name != hdev->name)
-		kfree(hi->input->name);
+	if (name != hdev->name) {
+		hi->input->name = hdev->name;
+		kfree(name);
+	}
 }
 
 static ssize_t mt_show_quirks(struct device *dev,
@@ -1040,11 +1043,11 @@ static void mt_remove(struct hid_device *hdev)
 	struct hid_input *hi;
 
 	sysfs_remove_group(&hdev->dev.kobj, &mt_attribute_group);
-	hid_hw_stop(hdev);
-
 	list_for_each_entry(hi, &hdev->inputs, list)
 		mt_free_input_name(hi);
 
+	hid_hw_stop(hdev);
+
 	kfree(td);
 	hid_set_drvdata(hdev, NULL);
 }
-- 
1.8.2.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/