Re: [PATCH] HID: wacom: validate battery range before scaling
From: sashiko-bot@kernel.org
Date: 2026-09-15 04:20:24
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] hid: wacom: battery updates and initialization skipped on out-of-range values
--
commit 2de6d44ab126bd3e2386ea0695c8fb70e0ec0e11
Author: Aldo Ariel Panzardo [off-list ref]
HID: wacom: validate battery range before scaling
Battery events divide by the difference between the logical maximum
and minimum without validating the range. A malformed HID descriptor
with equal limits can therefore trigger a divide-by-zero exception.
Move percentage conversion to a helper that widens the arithmetic,
rejects non-positive ranges and out-of-range results, and accounts for
a nonzero logical minimum.
quoted hunk ↗ jump to hunk
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c index 8feb8027be95e..1d6a72320a6e1 100644 --- a/drivers/hid/wacom_wac.c +++ b/drivers/hid/wacom_wac.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1952,6 +1953,24 @@ static void wacom_wac_battery_usage_mapping(struct hid_device *hdev, return; } +static bool wacom_wac_battery_percentage(struct hid_field *field, __s32 value, + __s32 *percentage) +{ + s64 range = (s64)field->logical_maximum - field->logical_minimum; + s64 result; + + if (range <= 0) + return false; + + result = div64_s64(((s64)value - field->logical_minimum) * 100, + range); + if (result < 0 || result > 100) + return false;
[Severity: Medium] Will this aggressively drop events when a hardware ADC reports a raw value slightly out of bounds (such as when fully charged or discharged)? Should this clamp the percentage to [0, 100] instead of returning false so we don't drop the event entirely?
quoted hunk ↗ jump to hunk
+ + *percentage = result; + return true; +} + static void wacom_wac_battery_event(struct hid_device *hdev, struct hid_field *field, struct hid_usage *usage, __s32 value) {@@ -1965,7 +1984,8 @@ static void wacom_wac_battery_event(struct hid_device *hdev, struct hid_field *f wacom_wac->hid_data.bat_status = POWER_SUPPLY_STATUS_UNKNOWN; } else { - value = value * 100 / (field->logical_maximum - field->logical_minimum); + if (!wacom_wac_battery_percentage(field, value, &value)) + return;
[Severity: Medium] If the very first report is slightly out of bounds, could this early return in wacom_wac_battery_event() skip setting the WACOM_QUIRK_BATTERY quirk below, preventing the battery power supply from being registered at all?
quoted hunk ↗ jump to hunk
wacom_wac->hid_data.battery_capacity = value; wacom_wac->hid_data.bat_connected = 1; wacom_wac->hid_data.bat_status = WACOM_POWER_SUPPLY_STATUS_AUTO;@@ -1973,7 +1993,8 @@ static void wacom_wac_battery_event(struct hid_device *hdev, struct hid_field *f wacom_wac->features.quirks |= WACOM_QUIRK_BATTERY; break; case WACOM_HID_WD_BATTERY_LEVEL: - value = value * 100 / (field->logical_maximum - field->logical_minimum); + if (!wacom_wac_battery_percentage(field, value, &value)) + return; wacom_wac->hid_data.battery_capacity = value; wacom_wac->hid_data.bat_connected = 1; wacom_wac->hid_data.bat_status = WACOM_POWER_SUPPLY_STATUS_AUTO;
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260915041248.2432629-1-qwe.aldo@gmail.com?part=1