[PATCH v2] HID: pidff: Fix integer overflow in pidff_rescale

[PATCH v2] HID: pidff: Fix integer overflow in pidff_rescale

[Tomasz Pakuła]

Rescaling values close to the max (U16_MAX) temporarily creates values
that exceed the s32 range. This caused value overflow in case when, for
example, a periodic effect phase was higer than 180 degrees. In turn,
rescale function could return values outised of the logical range of the
HID field.

Fix by using 64 bit signed integer to store the value during calculation
but still return only 32 bit integer.

Closes: https://github.com/JacKeTUs/universal-pidff/issues/116
Fixes: 224ee88fe395 ("Input: add force feedback driver for PID devices")
Cc: <stable@vger.kernel.org>
Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
---
For inclusion in 7.1 RC period.

v2: use div_s64() instead of plain division

 drivers/hid/usbhid/hid-pidff.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index aee8a4443305..c45f182d0448 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -11,6 +11,7 @@
 #include "hid-pidff.h"
 #include <linux/hid.h>
 #include <linux/input.h>
+#include <linux/math64.h>
 #include <linux/minmax.h>
 #include <linux/slab.h>
 #include <linux/stringify.h>
@@ -326,8 +327,10 @@ static s32 pidff_clamp(s32 i, struct hid_field *field)
  */
 static int pidff_rescale(int i, int max, struct hid_field *field)
 {
-	return i * (field->logical_maximum - field->logical_minimum) / max +
-	       field->logical_minimum;
+	/* 64 bits needed for big values during rescale */
+	s64 result = field->logical_maximum - field->logical_minimum;
+
+	return div_s64(result * i, max) + field->logical_minimum;
 }
 
 /*
-- 
2.54.0

Re: [PATCH v2] HID: pidff: Fix integer overflow in pidff_rescale

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The subtraction `field->logical_maximum - field->logical_minimum` is evaluated in 32-bit arithmetic, allowing it to overflow before being promoted to `s64`.
- [High] The adjacent function `pidff_rescale_signed()` suffers from the exact same integer overflow vulnerability but was completely missed by the patch.
- [High] This problem wasn't introduced by this patch, but missing bounds check on `unit_exponent` in `pidff_rescale_time()` allows a kernel stall via a malicious HID descriptor.
--

commit d365813aa395ac1ce1eeffd147aa3beb65aba60b
Author: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
HID: pidff: Fix integer overflow in pidff_rescale

This commit attempts to fix an integer overflow in pidff_rescale by using a
64-bit signed integer for calculations. It addresses an issue where rescaling
values close to the maximum could temporarily exceed the 32-bit signed
integer range.

Closes: https://github.com/JacKeTUs/universal-pidff/issues/116

> diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
> index aee8a44433059..c45f182d04480 100644
> --- a/drivers/hid/usbhid/hid-pidff.c
> +++ b/drivers/hid/usbhid/hid-pidff.c

[ ... ]

> @@ -326,8 +327,10 @@ static s32 pidff_clamp(s32 i, struct hid_field *field)
>   */
>  static int pidff_rescale(int i, int max, struct hid_field *field)
>  {
> -	return i * (field->logical_maximum - field->logical_minimum) / max +
> -	       field->logical_minimum;
> +	/* 64 bits needed for big values during rescale */
> +	s64 result = field->logical_maximum - field->logical_minimum;

Since both logical_maximum and logical_minimum are 32-bit signed integers,
does the subtraction field->logical_maximum - field->logical_minimum evaluate
in 32-bit arithmetic and potentially overflow before being promoted to s64?

If an untrusted USB device provides a descriptor with extreme limits,
this difference could exceed the capacity of a 32-bit signed integer and wrap
around to a negative value. Should at least one operand be explicitly cast
to s64 before the subtraction?

> +
> +	return div_s64(result * i, max) + field->logical_minimum;
>  }
>  
>  /*

This isn't a regression introduced by this patch, but looking at the
adjacent pidff_rescale_signed() function:

drivers/hid/usbhid/hid-pidff.c:pidff_rescale_signed() {
...
	if (i > 0)
		return i * field->logical_maximum / S16_MAX;
...
}

Does the multiplication i * field->logical_maximum suffer from a similar
overflow?

Since both operands fit into an int, they evaluate in 32-bit signed
arithmetic. If a device advertises a logical_maximum greater than 65535,
could this multiplication silently overflow yielding a wrapped-around
negative value before the division takes place?

Also not introduced by this patch, but looking at pidff_rescale_time():

drivers/hid/usbhid/hid-pidff.c:pidff_rescale_time() {
...
	int exponent = field->unit_exponent;

	pr_debug("time field exponent: %d\n", exponent);
	for (; exponent < FF_TIME_EXPONENT; exponent++)
		scaled_time *= 10;
	for (; exponent > FF_TIME_EXPONENT; exponent--)
		scaled_time /= 10;
...
}

Are there any bounds checks on field->unit_exponent before it is used
in these loops?

The field->unit_exponent is parsed from untrusted USB device descriptors.
If a USB device provides a large 32-bit exponent value, could
this execute up to 2.14 billion iterations doing multiplications or
divisions without any early break checks?

Could this trigger a kernel stall or soft lockup when a user uploads a
force-feedback effect?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260510122352.1161826-1-tomasz.pakula.oficjalny@gmail.com?part=1

Re: [PATCH v2] HID: pidff: Fix integer overflow in pidff_rescale

[Jiri Kosina]

On Sun, 10 May 2026, Tomasz Pakuła wrote:

> Rescaling values close to the max (U16_MAX) temporarily creates values
> that exceed the s32 range. This caused value overflow in case when, for
> example, a periodic effect phase was higer than 180 degrees. In turn,
> rescale function could return values outised of the logical range of the
> HID field.
> 
> Fix by using 64 bit signed integer to store the value during calculation
> but still return only 32 bit integer.
> 
> Closes: https://github.com/JacKeTUs/universal-pidff/issues/116
> Fixes: 224ee88fe395 ("Input: add force feedback driver for PID devices")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs