* [PATCH AUTOSEL 4.9 03/10] hid: topre: Add driver fixing report descriptor
[not found] <20221013002802.1896096-1-sashal@kernel.org>
@ 2022-10-13 0:27 ` Sasha Levin
2022-10-17 12:48 ` Pavel Machek
2022-10-13 0:27 ` [PATCH AUTOSEL 4.9 04/10] HID: roccat: Fix use-after-free in roccat_read() Sasha Levin
1 sibling, 1 reply; 3+ messages in thread
From: Sasha Levin @ 2022-10-13 0:27 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Harry Stern, Benjamin Tissoires, Sasha Levin, jikos, linux-input
From: Harry Stern <harry@harrystern.net>
[ Upstream commit a109d5c45b3d6728b9430716b915afbe16eef27c ]
The Topre REALFORCE R2 firmware incorrectly reports that interface
descriptor number 1, input report descriptor 2's events are array events
rather than variable events. That particular report descriptor is used
to report keypresses when there are more than 6 keys held at a time.
This bug prevents events from this interface from being registered
properly, so only 6 keypresses (from a different interface) can be
registered at once, rather than full n-key rollover.
This commit fixes the bug by setting the correct value in a report_fixup
function.
The original bug report can be found here:
Link: https://gitlab.freedesktop.org/libinput/libinput/-/issues/804
Thanks to Benjamin Tissoires for diagnosing the issue with the report
descriptor.
Signed-off-by: Harry Stern <harry@harrystern.net>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Link: https://lore.kernel.org/r/20220911003614.297613-1-harry@harrystern.net
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/Kconfig | 6 +++++
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 3 +++
drivers/hid/hid-topre.c | 49 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 59 insertions(+)
create mode 100644 drivers/hid/hid-topre.c
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 2b5ae00f8df2..80c54cd437db 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -837,6 +837,12 @@ config HID_TOPSEED
Say Y if you have a TopSeed Cyberlink or BTC Emprex or Conceptronic
CLLRCMCE remote control.
+config HID_TOPRE
+ tristate "Topre REALFORCE keyboards"
+ depends on HID
+ help
+ Say Y for N-key rollover support on Topre REALFORCE R2 108 key keyboards.
+
config HID_THINGM
tristate "ThingM blink(1) USB RGB LED"
depends on HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 86b2b5785fd2..245878158850 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -94,6 +94,7 @@ obj-$(CONFIG_HID_GREENASIA) += hid-gaff.o
obj-$(CONFIG_HID_THRUSTMASTER) += hid-tmff.o
obj-$(CONFIG_HID_TIVO) += hid-tivo.o
obj-$(CONFIG_HID_TOPSEED) += hid-topseed.o
+obj-$(CONFIG_HID_TOPRE) += hid-topre.o
obj-$(CONFIG_HID_TWINHAN) += hid-twinhan.o
obj-$(CONFIG_HID_UCLOGIC) += hid-uclogic.o
obj-$(CONFIG_HID_LED) += hid-led.o
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 1f641870d860..3ae9f6489f8f 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -1001,6 +1001,9 @@
#define USB_DEVICE_ID_TIVO_SLIDE 0x1201
#define USB_DEVICE_ID_TIVO_SLIDE_PRO 0x1203
+#define USB_VENDOR_ID_TOPRE 0x0853
+#define USB_DEVICE_ID_TOPRE_REALFORCE_R2_108 0x0148
+
#define USB_VENDOR_ID_TOPSEED 0x0766
#define USB_DEVICE_ID_TOPSEED_CYBERLINK 0x0204
diff --git a/drivers/hid/hid-topre.c b/drivers/hid/hid-topre.c
new file mode 100644
index 000000000000..88a91cdad5f8
--- /dev/null
+++ b/drivers/hid/hid-topre.c
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * HID driver for Topre REALFORCE Keyboards
+ *
+ * Copyright (c) 2022 Harry Stern <harry@harrystern.net>
+ *
+ * Based on the hid-macally driver
+ */
+
+#include <linux/hid.h>
+#include <linux/module.h>
+
+#include "hid-ids.h"
+
+MODULE_AUTHOR("Harry Stern <harry@harrystern.net>");
+MODULE_DESCRIPTION("REALFORCE R2 Keyboard driver");
+MODULE_LICENSE("GPL");
+
+/*
+ * Fix the REALFORCE R2's non-boot interface's report descriptor to match the
+ * events it's actually sending. It claims to send array events but is instead
+ * sending variable events.
+ */
+static __u8 *topre_report_fixup(struct hid_device *hdev, __u8 *rdesc,
+ unsigned int *rsize)
+{
+ if (*rsize >= 119 && rdesc[69] == 0x29 && rdesc[70] == 0xe7 &&
+ rdesc[71] == 0x81 && rdesc[72] == 0x00) {
+ hid_info(hdev,
+ "fixing up Topre REALFORCE keyboard report descriptor\n");
+ rdesc[72] = 0x02;
+ }
+ return rdesc;
+}
+
+static const struct hid_device_id topre_id_table[] = {
+ { HID_USB_DEVICE(USB_VENDOR_ID_TOPRE,
+ USB_DEVICE_ID_TOPRE_REALFORCE_R2_108) },
+ { }
+};
+MODULE_DEVICE_TABLE(hid, topre_id_table);
+
+static struct hid_driver topre_driver = {
+ .name = "topre",
+ .id_table = topre_id_table,
+ .report_fixup = topre_report_fixup,
+};
+
+module_hid_driver(topre_driver);
--
2.35.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH AUTOSEL 4.9 04/10] HID: roccat: Fix use-after-free in roccat_read()
[not found] <20221013002802.1896096-1-sashal@kernel.org>
2022-10-13 0:27 ` [PATCH AUTOSEL 4.9 03/10] hid: topre: Add driver fixing report descriptor Sasha Levin
@ 2022-10-13 0:27 ` Sasha Levin
1 sibling, 0 replies; 3+ messages in thread
From: Sasha Levin @ 2022-10-13 0:27 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Hyunwoo Kim, Jiri Kosina, Sasha Levin, erazor_de, jikos,
benjamin.tissoires, linux-input
From: Hyunwoo Kim <imv4bel@gmail.com>
[ Upstream commit cacdb14b1c8d3804a3a7d31773bc7569837b71a4 ]
roccat_report_event() is responsible for registering
roccat-related reports in struct roccat_device.
int roccat_report_event(int minor, u8 const *data)
{
struct roccat_device *device;
struct roccat_reader *reader;
struct roccat_report *report;
uint8_t *new_value;
device = devices[minor];
new_value = kmemdup(data, device->report_size, GFP_ATOMIC);
if (!new_value)
return -ENOMEM;
report = &device->cbuf[device->cbuf_end];
/* passing NULL is safe */
kfree(report->value);
...
The registered report is stored in the struct roccat_device member
"struct roccat_report cbuf[ROCCAT_CBUF_SIZE];".
If more reports are received than the "ROCCAT_CBUF_SIZE" value,
kfree() the saved report from cbuf[0] and allocates a new reprot.
Since there is no lock when this kfree() is performed,
kfree() can be performed even while reading the saved report.
static ssize_t roccat_read(struct file *file, char __user *buffer,
size_t count, loff_t *ppos)
{
struct roccat_reader *reader = file->private_data;
struct roccat_device *device = reader->device;
struct roccat_report *report;
ssize_t retval = 0, len;
DECLARE_WAITQUEUE(wait, current);
mutex_lock(&device->cbuf_lock);
...
report = &device->cbuf[reader->cbuf_start];
/*
* If report is larger than requested amount of data, rest of report
* is lost!
*/
len = device->report_size > count ? count : device->report_size;
if (copy_to_user(buffer, report->value, len)) {
retval = -EFAULT;
goto exit_unlock;
}
...
The roccat_read() function receives the device->cbuf report and
delivers it to the user through copy_to_user().
If the N+ROCCAT_CBUF_SIZE th report is received while copying of
the Nth report->value is in progress, the pointer that copy_to_user()
is working on is kfree()ed and UAF read may occur. (race condition)
Since the device node of this driver does not set separate permissions,
this is not a security vulnerability, but because it is used for
requesting screen display of profile or dpi settings,
a user using the roccat device can apply udev to this device node or
There is a possibility to use it by giving.
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-roccat.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/hid/hid-roccat.c b/drivers/hid/hid-roccat.c
index 76d06cf87b2a..36eee9663962 100644
--- a/drivers/hid/hid-roccat.c
+++ b/drivers/hid/hid-roccat.c
@@ -260,6 +260,8 @@ int roccat_report_event(int minor, u8 const *data)
if (!new_value)
return -ENOMEM;
+ mutex_lock(&device->cbuf_lock);
+
report = &device->cbuf[device->cbuf_end];
/* passing NULL is safe */
@@ -279,6 +281,8 @@ int roccat_report_event(int minor, u8 const *data)
reader->cbuf_start = (reader->cbuf_start + 1) % ROCCAT_CBUF_SIZE;
}
+ mutex_unlock(&device->cbuf_lock);
+
wake_up_interruptible(&device->wait);
return 0;
}
--
2.35.1
^ permalink raw reply related [flat|nested] 3+ messages in thread