Re: [PATCH 2/2] HID: roccat: add KUnit test for kone profile-index bounds

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] A Use-After-Free vulnerability occurs because the `kone_device` structure is freed while hardware interrupts are still active, leaving a dangling pointer in `hid_drvdata`.
- [High] Uninitialized kernel memory is copied to the persistent `kone->settings` structure on a failed USB control message, leading to a heap information leak via sysfs.
--

commit e8aee21a8be54aaf776b4d76c32eeaefd255b5aa
Author: Michael Bommarito [off-list ref]

HID: roccat: add KUnit test for kone profile-index bounds

This commit adds a KUnit test for the Roccat Kone driver to verify bounds
checking on profile indices. It simulates a crafted switch-profile event to
ensure the driver does not read out-of-bounds.

[ ... ]

[ ... ]

+	/* attack: value == ARRAY_SIZE(profiles) + 1 reads profiles[5], one
+	 * element past the array end -> KASAN slab-out-of-bounds read on an
+	 * unpatched kernel. The fix must reject it (actual_dpi unchanged).
+	 */
+	ev.value = ARRAY_SIZE(kone->profiles) + 1;
+	kone_keep_values_up_to_date(kone, &ev);
+	KUNIT_EXPECT_EQ(test, kone->actual_dpi, 0x42);
+}
+
+static struct kunit_case kone_test_cases[] = {
+	KUNIT_CASE(kone_profile_index_oob_test),
+	{}
+};
+
+static struct kunit_suite kone_test_suite = {
+	.name = "hid-roccat-kone",
+	.test_cases = kone_test_cases,
+};
+kunit_test_suite(kone_test_suite);
+#endif /* CONFIG_HID_ROCCAT_KONE_KUNIT_TEST */

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing I noticed a
potential use-after-free vulnerability in the driver initialization and
teardown paths.

In kone_probe(), hid_hw_start() is called before kone_init_specials(),
starting the hardware and enabling USB interrupts. If initialization fails,
the error path frees the driver data but does not clear the pointer:

drivers/hid/hid-roccat-kone.c:kone_init_specials() {
...
exit_free:
	kfree(kone);
	return retval;
}

Between kfree() and hid_hw_stop(), an incoming USB interrupt will trigger
kone_raw_event(), which dereferences the dangling pointer:

drivers/hid/hid-roccat-kone.c:kone_raw_event() {
...
	struct kone_device *kone = hid_get_drvdata(hdev);
...
}

The same window exists in the teardown path, where the driver state is freed
before stopping the hardware:

drivers/hid/hid-roccat-kone.c:kone_remove() {
...
	kone_remove_specials(hdev);
	hid_hw_stop(hdev);
}

Is it possible for an attacker to trigger this use-after-free by
disconnecting the device or simulating a device error while hardware
interrupts are active?

[Severity: High]
This is a pre-existing issue, but there appears to be a potential heap
information leak when copying uninitialized memory after a failed USB
transfer.

In kone_receive(), a bounce buffer is allocated with kmalloc() and
unconditionally copied to the target even if the USB transfer fails:

drivers/hid/hid-roccat-kone.c:kone_receive() {
	buf = kmalloc(size, GFP_KERNEL);
...
	retval = usb_control_msg(usb_dev, usb_rcvctrlpipe(usb_dev, 0), ...
...
	memcpy(data, buf, size);
	kfree(buf);
	return retval;
}

When called via kone_get_settings(), the uninitialized kernel heap memory
is copied to the persistent settings structure. If this occurs during a
sysfs operation where the error is handled, the corrupted structure remains
intact:

drivers/hid/hid-roccat-kone.c:kone_sysfs_set_tcu() {
...
	retval = kone_get_settings(usb_dev, &kone->settings);
	if (retval)
		goto exit_no_settings;
...
}

A subsequent read of the settings sysfs attribute exposes this memory
directly to userspace:

drivers/hid/hid-roccat-kone.c:kone_sysfs_read_settings() {
...
	memcpy(buf, ((char const *)&kone->settings) + off, count);
...
}

Could a malicious USB device intentionally fail the transfer to exploit
this leak? Should we check the return value of usb_control_msg() before
calling memcpy() in kone_receive()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618030036.1880139-1-michael.bommarito@gmail.com?part=2