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

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

[Aldo Ariel Panzardo]

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.

Fixes: 37d160193834 ("HID: wacom: generic: Scale battery capacity measurements to percentages")
Cc: stable@vger.kernel.org
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
 drivers/hid/wacom_wac.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index da1f0ea85..b93aac8e3 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,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;
+
+	*percentage = result;
+	return true;
+}
+
 static void wacom_wac_battery_event(struct hid_device *hdev, struct hid_field *field,
 		struct hid_usage *usage, __s32 value)
 {
@@ -1950,7 +1969,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;
 			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 +1978,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;
-- 
2.43.0

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

[sashiko-bot]

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 <qwe.aldo@gmail.com>

    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.

> 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

[ ... ]

> @@ -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?

> +
> +	*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?

>  			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

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

[Aldo Ariel Panzardo]

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@vger.kernel.org
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---

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

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@gmail.com> 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@vger.kernel.org
> Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>

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
>

[PATCH v3] HID: wacom: validate battery range before scaling

[Aldo Ariel Panzardo]

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 rejects non-positive
ranges and accounts for a nonzero logical minimum.  Clamp the result
to [0, 100] so that 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@vger.kernel.org
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---

Thanks for the feedback, Jason -- you are right, the s64 stuff was
overkill here.  Much simpler this way.

Changes in v3:
- Drop s64 widening and div64_s64 as requested by Jason Gerecke.
  Use __s32 range with a <= 0 guard instead. Keep the clamping.

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 or silently drop updates.

 drivers/hid/wacom_wac.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 361738b..36bab07 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -1960,6 +1960,16 @@ static void wacom_wac_battery_usage_mapping(struct hid_device *hdev,
 	return;
 }
 
+static __s32 wacom_wac_battery_percentage(struct hid_field *field, __s32 value)
+{
+	__s32 range = field->logical_maximum - field->logical_minimum;
+
+	if (range <= 0)
+		return 0;
+
+	return clamp_val((value - field->logical_minimum) * 100 / range, 0, 100);
+}
+
 static void wacom_wac_battery_event(struct hid_device *hdev, struct hid_field *field,
 		struct hid_usage *usage, __s32 value)
 {
@@ -1973,7 +1983,7 @@ 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;
@@ -1981,7 +1991,7 @@ 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.53.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] HID: wacom: out-of-range battery values dropped in
  wacom_wac_event()
--

commit d917c200b45c05631d5073d9e5d68f13f31acf
Author: Aldo Ariel Panzardo <qwe.aldo@gmail.com>

HID: wacom: validate battery range before scaling

This commit moves battery percentage scaling into a helper that checks for
a non-positive logical range, subtracts the logical minimum, and clamps the
computed percentage to [0, 100].

> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index 361738bb87253..36bab07a72dba 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -1960,6 +1960,16 @@ static void wacom_wac_battery_usage_mapping(struct hid_device *hdev,
>  	return;
>  }
>  
> +static __s32 wacom_wac_battery_percentage(struct hid_field *field, __s32 value)
> +{
> +	__s32 range = field->logical_maximum - field->logical_minimum;
> +
> +	if (range <= 0)
> +		return 0;
> +
> +	return clamp_val((value - field->logical_minimum) * 100 / range, 0, 100);
> +}

[Severity: Medium]
Can an out-of-range battery reading ever reach this clamp_val()?

wacom_wac_battery_percentage() is only called from wacom_wac_battery_event(),
whose sole caller wacom_wac_event() drops any value outside the logical
limits before checking WACOM_BATTERY_USAGE():

drivers/hid/wacom_wac.c:wacom_wac_event() {
    ...
	if (value > field->logical_maximum || value < field->logical_minimum)
		return;

	/* usage tests must precede field tests */
	if (WACOM_BATTERY_USAGE(usage))
		wacom_wac_battery_event(hdev, field, usage, value);
    ...
}

As a result, battery reports outside [field->logical_minimum,
field->logical_maximum] are still discarded in wacom_wac_event() without
setting WACOM_QUIRK_BATTERY or updating battery_capacity, and value is
already within bounds whenever wacom_wac_battery_percentage() runs.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106213526.3393779-1-qwe.aldo@gmail.com?part=1

[PATCH v4] HID: wacom: validate battery range before scaling

[Aldo Ariel Panzardo]

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 rejects non-positive
ranges and accounts for a nonzero logical minimum.  Clamp the result
to [0, 100] as an additional safeguard against out-of-range
percentages.

Fixes: 37d160193834 ("HID: wacom: generic: Scale battery capacity measurements to percentages")
Cc: stable@vger.kernel.org
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---

Changes in v4:
- Reword commit message: describe clamp as a defensive safeguard
  rather than claiming it handles a specific ADC scenario.  The
  caller already filters out-of-range values.  Code unchanged.

Changes in v3:
- Drop s64 widening and div64_s64 as requested by Jason Gerecke.
  Use __s32 range with a <= 0 guard instead. Keep the clamping.

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 or silently drop updates.

 drivers/hid/wacom_wac.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 361738b..36bab07 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -1960,6 +1960,16 @@ static void wacom_wac_battery_usage_mapping(struct hid_device *hdev,
 	return;
 }

+static __s32 wacom_wac_battery_percentage(struct hid_field *field, __s32 value)
+{
+	__s32 range = field->logical_maximum - field->logical_minimum;
+
+	if (range <= 0)
+		return 0;
+
+	return clamp_val((value - field->logical_minimum) * 100 / range, 0, 100);
+}
+
 static void wacom_wac_battery_event(struct hid_device *hdev, struct hid_field *field,
 		struct hid_usage *usage, __s32 value)
 {
@@ -1973,7 +1983,7 @@ 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;
@@ -1981,7 +1991,7 @@ 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.53.0