Re: [PATCH v2] HID: wacom: validate battery range before scaling

[Jason Gerecke]

On Mon, Sep 14, 2026 at 9:28 PM Aldo Ariel Panzardo <qwe.aldo@xxxxxxxxx> wrote:
>
> 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 accounts for a nonzero logical
> minimum.  Clamp the result to [0, 100] instead of rejecting
> out-of-range values, so that real hardware whose ADC occasionally
> reports slightly beyond its declared limits still registers its
> battery and updates capacity normally.
>
> Fixes: 37d160193834 ("HID: wacom: generic: Scale battery capacity measurements to percentages")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@xxxxxxxxx>

I realize this is *very* late, but (similar to the just-reviewed
sequence number patch) I'd like to request a version of this patch
that only addresses the divide-by-zero without introducing variable
widening. It is unlikely that the range of battery values would ever
exceed anything more than a very small fraction of __s32 so the
widening (and associated requirement to use div64_s64) feels very
unnecessary. Checking `if (range <= 0)` on an __s32 range should be
sufficient to catch overflowing and empty ranges. I think the
introduction of clamping is pretty reasonable as well and should be
kept in the v3.

Again, apologies for the delay---hopefully we'll (finally) get this merged soon!

Jason (she/they)
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one  /
(That is to say, eight) to the two,     /
But you can’t take seven from three,    /
So you look at the sixty-fours....

> ---
>
> Changes in v2:
>   - Clamp the computed percentage to [0, 100] instead of returning
>     false, so that slightly out-of-range ADC values do not prevent
>     battery registration (WACOM_QUIRK_BATTERY) or silently drop
>     updates.  Reported by Sashiko AI review.
>
>  drivers/hid/wacom_wac.c | 23 +++++++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index da1f0ea85..XXXXXXX 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -7,6 +7,7 @@
>  #include "wacom.h"
>  #include <linux/input/mt.h>
>  #include <linux/jiffies.h>
> +#include <linux/math64.h>
>
>  /* resolution for penabled devices */
>  #define WACOM_PL_RES           20
> @@ -1937,6 +1938,22 @@ static void wacom_wac_battery_usage_mapping(struct hid_device *hdev,
>         return;
>  }
>
> +static __s32 wacom_wac_battery_percentage(struct hid_field *field, __s32 value)
> +{
> +       s64 range = (s64)field->logical_maximum - field->logical_minimum;
> +       s64 result;
> +
> +       if (range <= 0)
> +               return 0;
> +
> +       result = div64_s64(((s64)value - field->logical_minimum) * 100,
> +                          range);
> +
> +       return clamp_val(result, 0, 100);
> +}
> +
>  static void wacom_wac_battery_event(struct hid_device *hdev, struct hid_field *field,
>                 struct hid_usage *usage, __s32 value)
>  {
> @@ -1950,7 +1967,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);
> +                       value = wacom_wac_battery_percentage(field, value);
>                         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;
> @@ -1958,7 +1976,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);
> +               value = wacom_wac_battery_percentage(field, value);
>                 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;
> --
> 2.43.0
>