Oliver Neukum 3bef7b
From cacdb14b1c8d3804a3a7d31773bc7569837b71a4 Mon Sep 17 00:00:00 2001
Oliver Neukum 3bef7b
From: Hyunwoo Kim <imv4bel@gmail.com>
Oliver Neukum 3bef7b
Date: Sun, 4 Sep 2022 12:31:15 -0700
Oliver Neukum 3bef7b
Subject: [PATCH] HID: roccat: Fix use-after-free in roccat_read()
Oliver Neukum 3bef7b
Patch-mainline: v6.1-rc1
Oliver Neukum 3bef7b
Git-commit: cacdb14b1c8d3804a3a7d31773bc7569837b71a4
Oliver Neukum 3bef7b
References: bsc#1203960 CVE-2022-41850
Oliver Neukum 3bef7b
Oliver Neukum 3bef7b
roccat_report_event() is responsible for registering
Oliver Neukum 3bef7b
roccat-related reports in struct roccat_device.
Oliver Neukum 3bef7b
Oliver Neukum 3bef7b
int roccat_report_event(int minor, u8 const *data)
Oliver Neukum 3bef7b
{
Oliver Neukum 3bef7b
	struct roccat_device *device;
Oliver Neukum 3bef7b
	struct roccat_reader *reader;
Oliver Neukum 3bef7b
	struct roccat_report *report;
Oliver Neukum 3bef7b
	uint8_t *new_value;
Oliver Neukum 3bef7b
Oliver Neukum 3bef7b
	device = devices[minor];
Oliver Neukum 3bef7b
Oliver Neukum 3bef7b
	new_value = kmemdup(data, device->report_size, GFP_ATOMIC);
Oliver Neukum 3bef7b
	if (!new_value)
Oliver Neukum 3bef7b
		return -ENOMEM;
Oliver Neukum 3bef7b
Oliver Neukum 3bef7b
	report = &device->cbuf[device->cbuf_end];
Oliver Neukum 3bef7b
Oliver Neukum 3bef7b
	/* passing NULL is safe */
Oliver Neukum 3bef7b
	kfree(report->value);
Oliver Neukum 3bef7b
	...
Oliver Neukum 3bef7b
Oliver Neukum 3bef7b
The registered report is stored in the struct roccat_device member
Oliver Neukum 3bef7b
"struct roccat_report cbuf[ROCCAT_CBUF_SIZE];".
Oliver Neukum 3bef7b
If more reports are received than the "ROCCAT_CBUF_SIZE" value,
Oliver Neukum 3bef7b
kfree() the saved report from cbuf[0] and allocates a new reprot.
Oliver Neukum 3bef7b
Since there is no lock when this kfree() is performed,
Oliver Neukum 3bef7b
kfree() can be performed even while reading the saved report.
Oliver Neukum 3bef7b
Oliver Neukum 3bef7b
static ssize_t roccat_read(struct file *file, char __user *buffer,
Oliver Neukum 3bef7b
		size_t count, loff_t *ppos)
Oliver Neukum 3bef7b
{
Oliver Neukum 3bef7b
	struct roccat_reader *reader = file->private_data;
Oliver Neukum 3bef7b
	struct roccat_device *device = reader->device;
Oliver Neukum 3bef7b
	struct roccat_report *report;
Oliver Neukum 3bef7b
	ssize_t retval = 0, len;
Oliver Neukum 3bef7b
	DECLARE_WAITQUEUE(wait, current);
Oliver Neukum 3bef7b
Oliver Neukum 3bef7b
	mutex_lock(&device->cbuf_lock);
Oliver Neukum 3bef7b
Oliver Neukum 3bef7b
	...
Oliver Neukum 3bef7b
Oliver Neukum 3bef7b
	report = &device->cbuf[reader->cbuf_start];
Oliver Neukum 3bef7b
	/*
Oliver Neukum 3bef7b
	 * If report is larger than requested amount of data, rest of report
Oliver Neukum 3bef7b
	 * is lost!
Oliver Neukum 3bef7b
	 */
Oliver Neukum 3bef7b
	len = device->report_size > count ? count : device->report_size;
Oliver Neukum 3bef7b
Oliver Neukum 3bef7b
	if (copy_to_user(buffer, report->value, len)) {
Oliver Neukum 3bef7b
		retval = -EFAULT;
Oliver Neukum 3bef7b
		goto exit_unlock;
Oliver Neukum 3bef7b
	}
Oliver Neukum 3bef7b
	...
Oliver Neukum 3bef7b
Oliver Neukum 3bef7b
The roccat_read() function receives the device->cbuf report and
Oliver Neukum 3bef7b
delivers it to the user through copy_to_user().
Oliver Neukum 3bef7b
If the N+ROCCAT_CBUF_SIZE th report is received while copying of
Oliver Neukum 3bef7b
the Nth report->value is in progress, the pointer that copy_to_user()
Oliver Neukum 3bef7b
is working on is kfree()ed and UAF read may occur. (race condition)
Oliver Neukum 3bef7b
Oliver Neukum 3bef7b
Since the device node of this driver does not set separate permissions,
Oliver Neukum 3bef7b
this is not a security vulnerability, but because it is used for
Oliver Neukum 3bef7b
requesting screen display of profile or dpi settings,
Oliver Neukum 3bef7b
a user using the roccat device can apply udev to this device node or
Oliver Neukum 3bef7b
There is a possibility to use it by giving.
Oliver Neukum 3bef7b
Oliver Neukum 3bef7b
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
Oliver Neukum 3bef7b
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Oliver Neukum 3bef7b
Signed-off-by: Oliver Neukum <oneukum@suse.com>
Oliver Neukum 3bef7b
---
Oliver Neukum 3bef7b
 drivers/hid/hid-roccat.c | 4 ++++
Oliver Neukum 3bef7b
 1 file changed, 4 insertions(+)
Oliver Neukum 3bef7b
Oliver Neukum 3bef7b
diff --git a/drivers/hid/hid-roccat.c b/drivers/hid/hid-roccat.c
Oliver Neukum 3bef7b
index 26373b82fe81..6da80e442fdd 100644
Oliver Neukum 3bef7b
--- a/drivers/hid/hid-roccat.c
Oliver Neukum 3bef7b
+++ b/drivers/hid/hid-roccat.c
Oliver Neukum 3bef7b
@@ -257,6 +257,8 @@ int roccat_report_event(int minor, u8 const *data)
Oliver Neukum 3bef7b
 	if (!new_value)
Oliver Neukum 3bef7b
 		return -ENOMEM;
Oliver Neukum 3bef7b
 
Oliver Neukum 3bef7b
+	mutex_lock(&device->cbuf_lock);
Oliver Neukum 3bef7b
+
Oliver Neukum 3bef7b
 	report = &device->cbuf[device->cbuf_end];
Oliver Neukum 3bef7b
 
Oliver Neukum 3bef7b
 	/* passing NULL is safe */
Oliver Neukum 3bef7b
@@ -276,6 +278,8 @@ int roccat_report_event(int minor, u8 const *data)
Oliver Neukum 3bef7b
 			reader->cbuf_start = (reader->cbuf_start + 1) % ROCCAT_CBUF_SIZE;
Oliver Neukum 3bef7b
 	}
Oliver Neukum 3bef7b
 
Oliver Neukum 3bef7b
+	mutex_unlock(&device->cbuf_lock);
Oliver Neukum 3bef7b
+
Oliver Neukum 3bef7b
 	wake_up_interruptible(&device->wait);
Oliver Neukum 3bef7b
 	return 0;
Oliver Neukum 3bef7b
 }
Oliver Neukum 3bef7b
-- 
Oliver Neukum 3bef7b
2.35.3
Oliver Neukum 3bef7b