diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig index e0e3fbac226d..1193442d008a 100644 --- a/drivers/hid/Kconfig +++ b/drivers/hid/Kconfig @@ -1080,6 +1080,15 @@ config HID_ROCCAT Say Y here if you have a Roccat mouse or keyboard and want support for its special functionalities. +config HID_ROCCAT_KONE_KUNIT_TEST + bool "KUnit tests for the Roccat Kone driver" if !KUNIT_ALL_TESTS + depends on HID_ROCCAT=y && KUNIT=y + default KUNIT_ALL_TESTS + help + Enable the KUnit regression tests for the Roccat Kone driver, + covering bounds checking of device-supplied profile indices. + If unsure, say N. + config HID_SAITEK tristate "Saitek (Mad Catz) non-fully HID-compliant devices" help diff --git a/drivers/hid/hid-roccat-kone.c b/drivers/hid/hid-roccat-kone.c index 58654cf78f0d..3dae9eaa0b6f 100644 --- a/drivers/hid/hid-roccat-kone.c +++ b/drivers/hid/hid-roccat-kone.c @@ -36,6 +36,8 @@ static uint profile_numbers[5] = {0, 1, 2, 3, 4}; static void kone_profile_activated(struct kone_device *kone, uint new_profile) { + if (new_profile < 1 || new_profile > ARRAY_SIZE(kone->profiles)) + new_profile = 1; kone->actual_profile = new_profile; kone->actual_dpi = kone->profiles[new_profile - 1].startup_dpi; } @@ -793,8 +795,10 @@ static void kone_keep_values_up_to_date(struct kone_device *kone, { switch (event->event) { case kone_mouse_event_switch_profile: - kone->actual_dpi = kone->profiles[event->value - 1]. - startup_dpi; + if (event->value >= 1 && + event->value <= ARRAY_SIZE(kone->profiles)) + kone->actual_dpi = + kone->profiles[event->value - 1].startup_dpi; fallthrough; case kone_mouse_event_osd_profile: kone->actual_profile = event->value; @@ -915,3 +919,60 @@ module_exit(kone_exit); MODULE_AUTHOR("Stefan Achatz"); MODULE_DESCRIPTION("USB Roccat Kone driver"); MODULE_LICENSE("GPL v2"); + +#if IS_ENABLED(CONFIG_HID_ROCCAT_KONE_KUNIT_TEST) +#include + +/* + * Regression test for the out-of-bounds read in + * kone_keep_values_up_to_date(): a malicious USB device sends a + * "switch profile" HID event (event == kone_mouse_event_switch_profile) + * with an attacker-chosen value in 0..255, which is used unbounded as + * profiles[value - 1]. On an unpatched kernel the attack case triggers a + * KASAN slab-out-of-bounds read; the fix must leave actual_dpi unchanged. + */ +static void kone_profile_index_oob_test(struct kunit *test) +{ + struct kone_device *kone; + struct kone_mouse_event ev = {}; + /* + * Allocate only up to the end of profiles[] so that any index past + * the 5-element array is IMMEDIATELY out of bounds and lands in the + * KASAN redzone (a far over-read would hit unrelated valid memory and + * escape KASAN). + */ + size_t sz = offsetof(struct kone_device, profiles) + + sizeof(kone->profiles); + + kone = kunit_kzalloc(test, sz, GFP_KERNEL); + KUNIT_ASSERT_NOT_NULL(test, kone); + kone->profiles[0].startup_dpi = 0x42; + + /* benign control: a valid in-range value drives the SAME path and + * must succeed (proves the trigger reaches the real code). + */ + ev.event = kone_mouse_event_switch_profile; + ev.value = 1; + kone_keep_values_up_to_date(kone, &ev); + KUNIT_EXPECT_EQ(test, kone->actual_dpi, 0x42); + + /* 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 */ diff --git a/drivers/hid/hid-roccat.c b/drivers/hid/hid-roccat.c index d6fff53d4ee7..4f15eb951039 100644 --- a/drivers/hid/hid-roccat.c +++ b/drivers/hid/hid-roccat.c @@ -70,6 +70,15 @@ static struct roccat_device *devices[ROCCAT_MAX_DEVICES]; /* protects modifications of devices array */ static DEFINE_MUTEX(devices_lock); +static void roccat_free_device(struct roccat_device *device) +{ + int i; + + for (i = 0; i < ROCCAT_CBUF_SIZE; i++) + kfree(device->cbuf[i].value); + kfree(device); +} + static ssize_t roccat_read(struct file *file, char __user *buffer, size_t count, loff_t *ppos) { @@ -226,7 +235,7 @@ static int roccat_release(struct inode *inode, struct file *file) hid_hw_power(device->hid, PM_HINT_NORMAL); hid_hw_close(device->hid); } else { - kfree(device); + roccat_free_device(device); } } @@ -374,7 +383,7 @@ void roccat_disconnect(int minor) hid_hw_close(device->hid); wake_up_interruptible(&device->wait); } else { - kfree(device); + roccat_free_device(device); } } EXPORT_SYMBOL_GPL(roccat_disconnect);