Re: [PATCH v3 6/8] HID: wacom: cleanup usage of 'driver_data'
From: sashiko-bot@kernel.org
Date: 2026-07-27 15:11:45
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Missing lower-bound check on the enum index allows an attacker to bypass bounds checking and trigger an out-of-bounds read by injecting a large value via the `new_id` sysfs interface. --
--- Patch [6]: [PATCH v3 6/8] HID: wacom: cleanup usage of 'driver_data' ---commit b219fca8b4be38408fca90649c9b3228cd0bfa8c Author: Pawel Zalewski (The Capable Hub) [off-list ref] HID: wacom: cleanup usage of 'driver_data' This commit changes the driver_data field in the HID device ID table from a direct pointer to an integer index. This index is then used to look up the appropriate wacom_features struct from a static array. This change aims to make it safer to inject new device IDs from userspace.
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c index 0eafa483b7f75..834519751a5cf 100644 --- a/drivers/hid/wacom_sys.c +++ b/drivers/hid/wacom_sys.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -2843,7 +2847,15 @@ static int wacom_probe(struct hid_device *hdev, wacom->hdev = hdev; wacom_wac = &wacom->wacom_wac; - wacom_wac->features = *((struct wacom_features *)id->driver_data); + + const struct wacom_features *id_features = get_wacom_features(id->driver_data); + + if (!id_features) { + hid_err(wacom->hdev, "could not get wacom id_features.\n"); + return -EINVAL; + } + wacom_wac->features = *id_features;
[Severity: High] If a large value is injected here via the new_id sysfs interface, it gets passed as id->driver_data to get_wacom_features(). Could the returned id_features pointer be out of bounds, causing a memory access violation when dereferenced during this assignment in wacom_probe()?
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c index da1f0ea85625d..d0a864d525098 100644 --- a/drivers/hid/wacom_wac.c +++ b/drivers/hid/wacom_wac.c
[ ... ]
+const struct wacom_features *get_wacom_features(enum wacom_features_table_index index)
+{
+ if (index >= NUM_OF_WACOM_FEATURES_IN_TABLE)
+ return NULL;
+
+ return wacom_features_table[index];
+}[Severity: High] Since the enum constants fit within a signed 32-bit integer, the compiler typically implements the enum wacom_features_table_index as a signed int. If id->driver_data is a large enough value, it could be implicitly downcast to a negative signed integer when passed to get_wacom_features(). A negative value would bypass the upper-bound check here since a negative number is less than NUM_OF_WACOM_FEATURES_IN_TABLE. Does this missing lower-bound check allow get_wacom_features() to return an invalid pointer from before the wacom_features_table array? Would it be safer to check if index is less than zero, or change the parameter to an unsigned type? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260727-mod-devicetable-hid_device_id-v3-0-980896ca283e@thegoodpenguin.co.uk?part=6