Thread (2 messages) 2 messages, 2 authors, 19d ago

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

From: sashiko-bot@kernel.org
Date: 2026-07-10 10:35:55

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 [off-list ref]

    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.
quoted hunk ↗ jump to hunk
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help