Re: [PATCH] HID: tmff: Use 64-bit arithmetic for force feedback scaling

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Structural race condition leading to NULL pointer dereference in Force Feedback initialization.
--

--- Patch [1]: [PATCH] HID: tmff: Use 64-bit arithmetic for force feedback scaling ---
commit b7654294b005de01aa2d8e34c8846bfff883d73f
Author: Linmao Li <lilinmao@kylinos.cn>

    HID: tmff: Use 64-bit arithmetic for force feedback scaling

This commit fixes an integer overflow in force feedback scaling calculations
by using 64-bit intermediate variables. It updates both tmff_scale_u16() and
tmff_scale_s8() helpers to use div_s64() to prevent incorrect values when
processing logical minimum and maximums from HID report descriptors.

> diff --git a/drivers/hid/hid-tmff.c b/drivers/hid/hid-tmff.c
> index 423f395d01ac5..319e7b6703846 100644
> --- a/drivers/hid/hid-tmff.c
> +++ b/drivers/hid/hid-tmff.c
> @@ -17,6 +17,7 @@
>  
>  #include <linux/hid.h>
>  #include <linux/input.h>
> +#include <linux/math64.h>

[Severity: High]
This isn't a bug introduced by this patch, but does the force feedback
initialization in this driver have a structural race condition?

Looking at tm_probe(), hid_hw_start() fully registers the input device and
exposes the event node to userspace before tmff_init() configures force
feedback:

drivers/hid/hid-tmff.c:tm_probe() {
    ...
	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_FF);
	if (ret) {
		hid_err(hdev, "hw start failed\n");
		goto err;
	}

	tmff_init(hdev, (void *)id->driver_data);
    ...
}

Inside tmff_init(), capability bits are set and the FF structures are
initialized:

drivers/hid/hid-tmff.c:tmff_init() {
    ...
				for (i = 0; ff_bits[i] >= 0; i++)
					set_bit(ff_bits[i], input_dev->ffbit);

				break;
    ...
	error = input_ff_create_memless(input_dev, tmff, tmff_play);
    ...
}

If a local user monitors for device creation, they might immediately open the
event node and issue an EVIOCSFF ioctl.

This would execute input_ff_upload(), which checks that EV_FF and the effect
bits are set.  Since tmff_init() has already set the capabilities, could this
unconditionally call an uninitialized ff->upload pointer before
input_ff_create_memless() finishes setting it up?

Would it be possible to encounter a NULL pointer dereference here?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710102314.190094-1-lilinmao@kylinos.cn?part=1