* [PATCH AUTOSEL 7.0-5.10] HID: magicmouse: Prevent out-of-bounds (OOB) read during DOUBLE_REPORT_ID
2026-05-20 11:18 [PATCH AUTOSEL 7.0-6.12] HID: logitech-hidpp: Add support for newer Bluetooth keyboards Sasha Levin
@ 2026-05-20 11:18 ` Sasha Levin
2026-05-20 11:41 ` sashiko-bot
2026-05-20 11:18 ` [PATCH AUTOSEL 7.0-5.10] HID: mcp2221: fix OOB write in mcp2221_raw_event() Sasha Levin
` (5 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Sasha Levin @ 2026-05-20 11:18 UTC (permalink / raw)
To: patches, stable
Cc: Lee Jones, Günther Noack, Jiri Kosina, Sasha Levin, jikos,
bentiss, linux-input, linux-kernel
From: Lee Jones <lee@kernel.org>
[ Upstream commit d93ba918a185aca2594da63e92fdc5495b559c0f ]
It is currently possible for a malicious or misconfigured USB device to
cause an out-of-bounds (OOB) read when submitting reports using
DOUBLE_REPORT_ID by specifying a large report length and providing a
smaller one.
Let's prevent that by comparing the specified report length with the
actual size of the data read in from userspace. If the actual data
length ends up being smaller than specified, we'll politely warn the
user and prevent any further processing.
Signed-off-by: Lee Jones <lee@kernel.org>
Reviewed-by: Günther Noack <gnoack@google.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Phase 1: Commit Message Forensics
Step 1.1 Record: Subsystem `HID: magicmouse`; action verb `Prevent`;
claimed intent is preventing an out-of-bounds read while parsing
`DOUBLE_REPORT_ID`.
Step 1.2 Record: Tags present: `Signed-off-by: Lee Jones
<lee@kernel.org>`, `Reviewed-by: Günther Noack <gnoack@google.com>`,
`Signed-off-by: Jiri Kosina <jkosina@suse.com>`. No `Fixes:`, `Reported-
by:`, `Tested-by:`, `Link:`, or `Cc: stable@vger.kernel.org` tag was
present in the supplied commit text.
Step 1.3 Record: The body describes a malformed HID report where
`data[1]` advertises a larger embedded report length than the actual
received buffer. Symptom/failure mode: OOB read during recursive
parsing. No affected kernel versions are named. Root cause:
`magicmouse_raw_event()` trusted the embedded length before checking
that the buffer actually contained that many bytes.
Step 1.4 Record: This is not a hidden cleanup; it is an explicit memory-
safety bug fix.
## Phase 2: Diff Analysis
Step 2.1 Record: One file changed: `drivers/hid/hid-magicmouse.c`, 16
insertions, no removals. Modified function: `magicmouse_raw_event()`.
Scope: single-file surgical fix.
Step 2.2 Record: First hunk adds `if (size < 1) return 0;` before
`switch (data[0])`, preventing zero-sized recursive calls from
dereferencing `data[0]`. Second hunk adds `size < 2` before reading
`data[1]`, then validates `size >= data[1] + 2` before splitting the
double report into recursive calls.
Step 2.3 Record: Bug category is memory safety / bounds validation.
Broken mechanism: the `DOUBLE_REPORT_ID` case used `data[1]` to choose
recursive buffer pointers and sizes without proving the first embedded
report existed in the received buffer.
Step 2.4 Record: The fix is obviously correct by inspection: it checks
the exact fields before use and follows existing invalid-report behavior
of returning `0`. Regression risk is very low; only malformed undersized
reports are rejected. Minor concern: the new `hid_warn()` string lacks a
trailing newline, but that is logging style, not functional risk.
## Phase 3: Git History Investigation
Step 3.1 Record: `git blame` shows `magicmouse_raw_event()` dates back
to `128537cea464` from 2010, while the `DOUBLE_REPORT_ID` recursive
parsing lines were introduced by `a462230e16acc` (`HID: magicmouse:
enable Magic Trackpad support`). `git describe --contains` places that
introduction at `v2.6.37-rc1~144^2~3^5~2`.
Step 3.2 Record: No `Fixes:` tag is present, so there was no tagged
introducing commit to follow. Blame independently identifies the
relevant old code.
Step 3.3 Record: Recent `drivers/hid/hid-magicmouse.c` history contains
other fixes such as missing input crash prevention and report-fixup leak
fixes, but no prerequisite for this bounds check was identified. This
patch is standalone.
Step 3.4 Record: `git log --author='Lee Jones' -- drivers/hid` shows Lee
Jones has multiple recent HID fixes, including OOB/UAF-related HID work.
The patch was reviewed by Günther Noack and applied by HID maintainer
Jiri Kosina in the mailing-list thread.
Step 3.5 Record: No dependency was found. The patch only uses existing
`hid_warn()` and local variables already present in
`magicmouse_raw_event()`.
## Phase 4: Mailing List And External Research
Step 4.1 Record: No commit hash was supplied and the commit object was
not found in local `git log`, so `b4 dig -c <commit>` was not
applicable. I used `b4 mbox` with the v2 Message-ID instead. Lore URL:
`https://lore.kernel.org/all/20260416131655.2279756-1-lee@kernel.org/`.
Series revisions found: v1 and v2.
Step 4.2 Record: The v2 headers show Lee sent it to Jiri Kosina,
Benjamin Tissoires, `linux-input@vger.kernel.org`, `linux-
kernel@vger.kernel.org`, and Cc’d Günther Noack. Jiri later replied
“Applied”.
Step 4.3 Record: No `Reported-by` or external bug-report link was
present. The thread itself contains the bug explanation.
Step 4.4 Record: v1 added only the `size < data[1] + 2` check. Günther
Noack objected that `size == data[1] + 2` still caused a zero-sized
second recursive call which then dereferenced `data[0]`. v2 added the
top-level `size < 1` and `DOUBLE_REPORT_ID` `size < 2` checks; Günther
replied “This looks correct now” and gave `Reviewed-by`.
Step 4.5 Record: Web search for stable-specific discussion did not find
a stable nomination or a reason not to backport.
## Phase 5: Code Semantic Analysis
Step 5.1 Record: Modified function: `magicmouse_raw_event()`.
Step 5.2 Record: Callers are HID core through the driver `.raw_event`
callback and recursive self-calls in the `DOUBLE_REPORT_ID` case. `hid-
core.c` calls `hdrv->raw_event(hid, report, data, size)` before generic
report processing.
Step 5.3 Record: Key callees in the affected path are recursive
`magicmouse_raw_event()` calls, `magicmouse_emit_touch()`, input event
reporting helpers, and `hid_warn()`. The new checks run before recursive
pointer arithmetic.
Step 5.4 Record: Reachability is verified through `hid_input_report()`,
including `usbhid` interrupt/control paths and `uhid` input paths. A
matched or emulated HID device can feed raw reports to this parser.
Unprivileged triggering was not verified and is not needed for the
decision.
Step 5.5 Record: Search in `drivers/hid` found this `DOUBLE_REPORT_ID`
split pattern only in `hid-magicmouse.c`.
## Phase 6: Stable Tree Analysis
Step 6.1 Record: The vulnerable pattern exists in `v6.6`, `v6.1`,
`v5.15`, `v5.10`, `v5.4`, and `v4.19` by `git grep`. The relevant code
was introduced around `v2.6.37-rc1`.
Step 6.2 Record: Expected backport difficulty is low: the same block and
function exist across checked stable baselines, though line offsets
differ. Exact clean application to every stable branch was not tested.
Step 6.3 Record: Local history searches for `DOUBLE_REPORT_ID` and `out-
of-bounds.*magicmouse` found no prior equivalent fix in this checkout.
## Phase 7: Subsystem Context
Step 7.1 Record: Subsystem is HID input driver support for Apple Magic
Mouse/Trackpad, under `drivers/hid`. Criticality: peripheral/driver-
specific, but memory-safety relevant.
Step 7.2 Record: HID is actively maintained; recent `drivers/hid`
history shows multiple 2026 fixes including OOB, UAF, and NULL-deref
fixes.
## Phase 8: Impact And Risk
Step 8.1 Record: Affected users are systems with `CONFIG_HID_MAGICMOUSE`
and matching Apple Magic Trackpad/Magic Mouse style HID devices or
emulated/spoofed devices reaching this driver.
Step 8.2 Record: Trigger requires a malformed `DOUBLE_REPORT_ID` report
where the embedded length does not fit the actual received size, or
where the second recursive call would be zero-sized. This is not a
normal-path trigger, but it is reachable via raw HID reports.
Step 8.3 Record: Failure mode is OOB read in kernel HID parsing.
Severity: HIGH because it is memory safety in a device-input path and
can be triggered by malformed device input; exact crash or disclosure
behavior was not independently reproduced.
Step 8.4 Record: Benefit is high for affected systems because it blocks
a concrete OOB read. Risk is very low: 16 inserted validation lines,
single function, no ABI/API change, no locking change.
## Phase 9: Final Synthesis
Step 9.1 Record: Evidence for backporting: real OOB read, old code
present in many stable baselines, small surgical validation, reviewed
after v1 caught an incomplete fix, applied by HID maintainer. Evidence
against: no `Tested-by`, no explicit stable nomination, exact clean
apply to every tree not tested. Unresolved: exact upstream commit hash
unavailable locally; exact unprivileged triggerability unverified; exact
USB-vs-Bluetooth trigger wording not independently verified.
Step 9.2 Record: Stable rules: obviously correct: yes, by code
inspection and review; tested: no explicit `Tested-by`. Fixes real bug:
yes, OOB read. Important issue: yes, memory safety. Small and contained:
yes, one function, 16 insertions. No new features/APIs: yes. Can apply
to stable: likely low difficulty; exact apply unverified.
Step 9.3 Record: No exception category applies; this is a normal memory-
safety bug fix, not a device ID, quirk, DT, build, or documentation
patch.
Step 9.4 Record: The technical merit strongly favors backporting. The
fix is narrow, old affected code is present across stable trees, and the
mailing-list review specifically corrected the subtle recursive zero-
size case.
## Verification
- Phase 1: Parsed supplied commit message and tags.
- Phase 2: Compared supplied diff with local `drivers/hid/hid-
magicmouse.c` context.
- Phase 3: Ran `git blame -L 386,496 -- drivers/hid/hid-magicmouse.c`;
identified `a462230e16acc` as the `DOUBLE_REPORT_ID` recursive parsing
introduction.
- Phase 3: Ran `git describe --contains --match 'v[0-9]*'
a462230e16acc`; found `v2.6.37-rc1~144^2~3^5~2`.
- Phase 4: Used `b4 mbox 20260416131655.2279756-1-lee@kernel.org`; found
v2 thread, reviewer approval, and Jiri’s “Applied” reply.
- Phase 4: Used `b4 mbox ad-S8Riah-f6mEWm@google.com`; found v1 review
pointing out the remaining zero-sized recursive-call bug.
- Phase 5: Used `rg` and read `hid-core.c`; verified `.raw_event`
dispatch and `hid_input_report()` reachability.
- Phase 6: Used `git grep` on `v6.6`, `v6.1`, `v5.15`, `v5.10`, `v5.4`,
and `v4.19`; confirmed vulnerable code exists.
- Phase 7: Checked `drivers/hid/Kconfig`; confirmed
`CONFIG_HID_MAGICMOUSE`.
- Phase 8: Severity assessment is based on verified OOB read mechanics;
exact crash/leak manifestation is unverified.
- UNVERIFIED: Exact commit hash in local repository; exact clean apply
to every stable branch; unprivileged triggerability.
**YES**
drivers/hid/hid-magicmouse.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index 9eadf3252d0dc..f53839484c861 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -390,6 +390,10 @@ static int magicmouse_raw_event(struct hid_device *hdev,
struct input_dev *input = msc->input;
int x = 0, y = 0, ii, clicks = 0, npoints;
+ /* Protect against zero sized recursive calls from DOUBLE_REPORT_ID */
+ if (size < 1)
+ return 0;
+
switch (data[0]) {
case TRACKPAD_REPORT_ID:
case TRACKPAD2_BT_REPORT_ID:
@@ -490,6 +494,18 @@ static int magicmouse_raw_event(struct hid_device *hdev,
/* Sometimes the trackpad sends two touch reports in one
* packet.
*/
+
+ /* Ensure that we have at least 2 elements (report type and size) */
+ if (size < 2)
+ return 0;
+
+ if (size < data[1] + 2) {
+ hid_warn(hdev,
+ "received report length (%d) was smaller than specified (%d)",
+ size, data[1] + 2);
+ return 0;
+ }
+
magicmouse_raw_event(hdev, report, data + 2, data[1]);
magicmouse_raw_event(hdev, report, data + 2 + data[1],
size - 2 - data[1]);
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH AUTOSEL 7.0-5.10] HID: magicmouse: Prevent out-of-bounds (OOB) read during DOUBLE_REPORT_ID
2026-05-20 11:18 ` [PATCH AUTOSEL 7.0-5.10] HID: magicmouse: Prevent out-of-bounds (OOB) read during DOUBLE_REPORT_ID Sasha Levin
@ 2026-05-20 11:41 ` sashiko-bot
0 siblings, 0 replies; 12+ messages in thread
From: sashiko-bot @ 2026-05-20 11:41 UTC (permalink / raw)
To: Sasha Levin; +Cc: dmitry.torokhov, linux-input
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [Medium] The newly added warning for mismatched report lengths is not rate-limited, allowing an attacker to continuously spam the kernel log.
- [Low] Missing trailing newline in the newly added `hid_warn` format string.
Pre-existing issues:
- [Critical] A NULL pointer dereference can occur if a raw event is received before the device is fully initialized or if initialization fails to create an input device.
- [High] A malicious USB device can cause a kernel stack overflow by sending a crafted HID report with deeply nested DOUBLE_REPORT_ID reports.
--
commit 722425cdf7389e2fa0f87560f3507f1c80daa8b7
Author: Lee Jones <lee@kernel.org>
HID: magicmouse: Prevent out-of-bounds (OOB) read during DOUBLE_REPORT_ID
This commit adds checks to prevent out-of-bounds reads in the magicmouse
driver when processing DOUBLE_REPORT_ID reports. It verifies the report
size against the specified length and adds protection against zero-sized
recursive calls.
> diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
> index 9eadf3252d0dc..f53839484c861 100644
> --- a/drivers/hid/hid-magicmouse.c
> +++ b/drivers/hid/hid-magicmouse.c
> @@ -390,6 +390,10 @@ static int magicmouse_raw_event(struct hid_device *hdev,
> struct input_dev *input = msc->input;
[Severity: Critical]
This is a pre-existing issue, but can this lead to a NULL pointer
dereference?
If a device provides a crafted report descriptor without any input fields,
hidinput_connect() will not call .input_mapping, leaving msc->input as NULL.
During magicmouse_probe(), for some devices the probe returns 0 early without
verifying msc->input. If raw IN reports are then streamed, this function will
blindly dereference the NULL input pointer later when evaluating
input->id.product.
> int x = 0, y = 0, ii, clicks = 0, npoints;
>
> + /* Protect against zero sized recursive calls from DOUBLE_REPORT_ID */
> + if (size < 1)
> + return 0;
> +
[ ... ]
> @@ -490,6 +494,18 @@ static int magicmouse_raw_event(struct hid_device *hdev,
> /* Sometimes the trackpad sends two touch reports in one
> * packet.
> */
> +
> + /* Ensure that we have at least 2 elements (report type and size) */
> + if (size < 2)
> + return 0;
> +
> + if (size < data[1] + 2) {
> + hid_warn(hdev,
> + "received report length (%d) was smaller than specified (%d)",
[Severity: Low]
Does this log message need a trailing newline? It looks like the missing
newline will prevent the buffer from flushing immediately, causing subsequent
log messages to append to this line.
[Severity: Medium]
Could this warning be abused to spam the kernel log? A malicious or
misconfigured device could rapidly stream malformed reports, flooding dmesg.
Would it be safer to use hid_warn_ratelimited() here?
> + size, data[1] + 2);
> + return 0;
> + }
> +
> magicmouse_raw_event(hdev, report, data + 2, data[1]);
> magicmouse_raw_event(hdev, report, data + 2 + data[1],
> size - 2 - data[1]);
[Severity: High]
This is a pre-existing issue, but does this recursion safely handle a payload
entirely filled with DOUBLE_REPORT_ID markers and data[1] set to 0?
Because size is only decremented by 2 in each recursive step:
magicmouse_raw_event(..., size - 2 - data[1]);
A large report of 4096 bytes could trigger up to 2048 nested calls. Since
tail-call optimization is not guaranteed in the kernel, couldn't this exhaust
the kernel stack and trigger a panic?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260520111944.3424570-9-sashal@kernel.org?part=1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH AUTOSEL 7.0-5.10] HID: mcp2221: fix OOB write in mcp2221_raw_event()
2026-05-20 11:18 [PATCH AUTOSEL 7.0-6.12] HID: logitech-hidpp: Add support for newer Bluetooth keyboards Sasha Levin
2026-05-20 11:18 ` [PATCH AUTOSEL 7.0-5.10] HID: magicmouse: Prevent out-of-bounds (OOB) read during DOUBLE_REPORT_ID Sasha Levin
@ 2026-05-20 11:18 ` Sasha Levin
2026-05-20 11:56 ` sashiko-bot
2026-05-20 11:19 ` [PATCH AUTOSEL 7.0-5.10] HID: elan: Add support for ELAN SB974D touchpad Sasha Levin
` (4 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Sasha Levin @ 2026-05-20 11:18 UTC (permalink / raw)
To: patches, stable
Cc: Florian Pradines, Benoît Sevens, Jiri Kosina, Sasha Levin,
gupt21, jikos, bentiss, linux-i2c, linux-input, linux-kernel
From: Florian Pradines <florian.pradines@gmail.com>
[ Upstream commit f097d246677b03db814c5862f368cea341b76a00 ]
mcp2221_raw_event() copies device-supplied data into mcp->rxbuf at
offset rxbuf_idx without checking that the copy fits within the
destination buffer. A device responding with up to 60 bytes to a
small I2C/SMBus read can overflow the buffer.
Add a rxbuf_size field to struct mcp2221, set it alongside rxbuf in
mcp_i2c_smbus_read(), and check rxbuf_idx + data[3] <= rxbuf_size
before the memcpy.
Reported-by: Benoît Sevens <bsevens@google.com>
Signed-off-by: Florian Pradines <florian.pradines@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Phase 1: Commit Message Forensics
Step 1.1 Record: subsystem `HID: mcp2221`; action verb `fix`; claimed
intent: prevent an out-of-bounds write in `mcp2221_raw_event()` when
copying device-supplied I2C read data into `mcp->rxbuf`.
Step 1.2 Record: tags found in upstream commit
`f097d246677b03db814c5862f368cea341b76a00`: `Reported-by: Benoît Sevens
<bsevens@google.com>`, `Signed-off-by: Florian Pradines
<florian.pradines@gmail.com>`, `Signed-off-by: Jiri Kosina
<jkosina@suse.com>`. No `Fixes:`, no `Cc: stable`, no `Link:` in the
commit message.
Step 1.3 Record: bug description is explicit: `mcp2221_raw_event()`
copies `data[3]` bytes into `mcp->rxbuf + rxbuf_idx` without checking
the destination buffer size. Failure mode is kernel memory corruption
via OOB write when a device returns up to 60 bytes for a smaller
requested read.
Step 1.4 Record: not hidden; this is a direct memory-safety fix.
## Phase 2: Diff Analysis
Step 2.1 Record: one file changed, `drivers/hid/hid-mcp2221.c`, 7
insertions. Modified areas: `struct mcp2221`, `mcp_i2c_smbus_read()`,
`mcp2221_raw_event()`. Scope: single-file surgical fix.
Step 2.2 Record: before, the driver tracked `rxbuf` and `rxbuf_idx` but
not the destination size. After, it stores `rxbuf_size` when setting
`rxbuf`, and rejects `rxbuf_idx + data[3] > rxbuf_size` before
`memcpy()`.
Step 2.3 Record: bug category is memory safety, specifically OOB write
from device-controlled length and cumulative receive offset. The earlier
`data[3] > 60` check only capped a single MCP2221 report, not the
destination buffer size.
Step 2.4 Record: fix is minimal and locally correct for the upstream
parent, but direct stable backports need care because older stable
branches still have two manual `mcp_smbus_xfer()` paths that set
`mcp->rxbuf` directly.
## Phase 3: Git History
Step 3.1 Record: `git blame` showed `rxbuf`, `rxbuf_idx`,
`mcp_i2c_smbus_read()`, and the `memcpy()` path originated in
`67a95c21463d0`, first contained in `v5.7-rc1`. The partial-read
handling came from `2682468671aa0`, first contained in `v6.8-rc1`. The
prior per-report `data[3] > 60` guard came from `b56cc41a3ae73`, first
contained in `v6.17-rc4` and backported to stable equivalents.
Step 3.2 Record: no `Fixes:` tag, so no tagged introducing commit to
follow.
Step 3.3 Record: recent related commits include `b56cc41a3ae73` for the
60-byte report cap, `e31b556c0ba21` for read-error cancellation, and
`ab05515757fcb` for using `mcp_i2c_smbus_read()` for block reads.
`ab05515757fcb` is an important context dependency for the exact
upstream diff.
Step 3.4 Record: author Florian Pradines had no other local commits
found in `drivers/hid`; Jiri Kosina, the HID maintainer,
committed/applied the patch.
Step 3.5 Record: upstream parent contains `ab05515757fcb`; stable
branches checked from `5.10.y` through `7.0.y` do not. A stable backport
should either adapt the patch to set `rxbuf_size` in the two remaining
manual `mcp_smbus_xfer()` paths or include an appropriate
prerequisite/backport.
## Phase 4: Mailing List / External Research
Step 4.1 Record: `b4 dig -c f097d246677b03db814c5862f368cea341b76a00`
found the lore thread: `https://patch.msgid.link/20260509094517.2691246-
1-florian.pradines@gmail.com`. `b4 dig -a` showed only v1.
Step 4.2 Record: `b4 dig -w` showed recipients included Florian
Pradines, Rishi Gupta, Jiri Kosina, Benjamin Tissoires, `linux-i2c`, and
`linux-input`.
Step 4.3 Record: reporter Benoît Sevens previously submitted a similar
heap-buffer-overflow patch. Jiri replied that Benoît’s patch was mangled
and that Florian’s patch would be taken with Benoît added as `Reported-
by`. Benoît then noted the missing `mcp_smbus_xfer()` assignments, which
is relevant for older stable bases.
Step 4.4 Record: not part of a multi-patch series. Related patches exist
for report-size validation, but Jiri questioned the “MCP2221 always
sends 64-byte reports” assumption in that separate thread.
Step 4.5 Record: direct lore/stable WebFetch was blocked by Anubis, but
web search found related stable discussion for prior mcp2221 fixes, not
an exact stable request for `f097d246677b`.
## Phase 5: Semantic Analysis
Step 5.1 Record: changed function is `mcp_i2c_smbus_read()`; changed
callback is `mcp2221_raw_event()`.
Step 5.2 Record: callers of `mcp_i2c_smbus_read()` are `mcp_i2c_xfer()`
and `mcp_smbus_xfer()`, wired into `mcp_i2c_algo`. `mcp2221_raw_event()`
is called from HID core before normal raw report processing, via
`hid_input_report()` / `__hid_input_report()`.
Step 5.3 Record: affected callees include `mcp_send_data_req_status()`,
`mcp_send_report()`, `wait_for_completion_timeout()`, and the vulnerable
`memcpy()`.
Step 5.4 Record: reachability is real: USB HID interrupt input reaches
`mcp2221_raw_event()`, and I2C/SMBus transfers reach
`mcp_i2c_smbus_read()` through the registered I2C adapter.
Triggerability by an unprivileged local user was not verified; trigger
by a malicious or buggy MCP2221-side device response is verified by the
code path.
Step 5.5 Record: similar pattern found in stable branches: two manual
`mcp_smbus_xfer()` receive-buffer assignments remain in stable branches
and must be handled in backports.
## Phase 6: Stable Tree Analysis
Step 6.1 Record: the driver was introduced in `v5.7-rc1`; local stable
refs `5.10.y`, `5.15.y`, `6.1.y`, `6.6.y`, `6.12.y`, `6.18.y`, `6.19.y`,
and `7.0.y` contain the vulnerable `rxbuf`/`memcpy()` pattern. `5.4.y`
does not contain the driver.
Step 6.2 Record: `git apply --check` against current
`stable/linux-7.0.y` succeeds syntactically, but semantic backport work
is needed because stable lacks `ab05515757fcb`.
Step 6.3 Record: prior stable branches already contain the earlier
60-byte cap fix or its backported equivalent, but that does not fix
cumulative overflow into smaller destination buffers.
## Phase 7: Subsystem Context
Step 7.1 Record: subsystem is HID driver for the Microchip MCP2221 USB-
to-I2C/SMBus bridge. Criticality: driver-specific, but memory corruption
from USB/HID device input is high severity for affected systems.
Step 7.2 Record: file history shows active recent maintenance and
multiple security/correctness fixes in this driver.
## Phase 8: Impact And Risk
Step 8.1 Record: affected population is users of MCP2221 hardware or
systems exposed to such USB HID devices.
Step 8.2 Record: trigger is an I2C/SMBus read where the MCP2221-side
response reports more bytes than the destination buffer can hold.
Physical/malicious-device trigger is verified; unprivileged local
trigger depends on device-node permissions and was not verified.
Step 8.3 Record: failure mode is OOB write into kernel memory, severity
HIGH to CRITICAL due to memory corruption/crash/security risk.
Step 8.4 Record: benefit is high. Risk is low for the upstream parent
and for correctly adapted stable backports. Risk is medium for blind
cherry-picks into older stable branches because `rxbuf_size` would not
be set in all stable receive-buffer paths.
## Phase 9: Final Synthesis
Evidence for backporting: real OOB write, explicit reporter, small
patch, maintainer-applied, affects stable-supported code since `5.10.y`,
fixes a memory-corruption class bug.
Evidence against or concerns: no `Fixes:`/`Cc: stable`; exact upstream
diff assumes `ab05515757fcb`, absent from checked stable branches; blind
cherry-pick is not sufficient for older stable trees.
Stable rules checklist: obviously correct on its upstream base: yes.
Fixes a real user-affecting bug: yes. Important issue: yes, kernel
memory corruption. Small and contained: yes, 7 insertions. No new
feature/API: yes. Applies to stable: syntactically yes on `7.0.y`, but
older stable needs a branch-specific backport adjustment.
Exception category: none; this is a memory-safety fix, not a device
ID/quirk/doc/build exception.
## Verification
- Phase 1: fetched and inspected upstream commit
`f097d246677b03db814c5862f368cea341b76a00`.
- Phase 2: verified exact diff is 7 insertions in `drivers/hid/hid-
mcp2221.c`.
- Phase 3: ran `git blame`, `git describe --contains`, and targeted `git
log` without `--all`; identified `67a95c21463d0`, `2682468671aa0`,
`b56cc41a3ae73`, and `ab05515757fcb`.
- Phase 4: ran `b4 dig -c`, `b4 dig -a`, `b4 dig -w`, and fetched mboxes
with `b4 mbox`; verified Jiri’s “Applied, thanks” and Benoît’s earlier
report/backport-relevant concern.
- Phase 5: used code search and file reads to verify HID and I2C/SMBus
call paths.
- Phase 6: used `git grep` against stable refs to verify vulnerable
patterns in `5.10.y` through `7.0.y`, and absence from `5.4.y`.
- Phase 6: verified exact patch syntactically applies to current
`stable/linux-7.0.y`, but `ab05515757fcb` is not an ancestor of
checked stable refs.
- Phase 8: severity is based on verified device-controlled `memcpy()`
into kernel buffer without destination-size check.
- UNVERIFIED: no runtime reproducer was executed; no build test was run;
unprivileged local triggerability was not verified.
This should be backported, but stable maintainers should not blindly
cherry-pick it into pre-`ab05515757fcb` trees without adding
`rxbuf_size` initialization to every remaining `mcp->rxbuf` assignment.
**YES**
drivers/hid/hid-mcp2221.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c
index ef3b5c77c38e3..badd4958bc12c 100644
--- a/drivers/hid/hid-mcp2221.c
+++ b/drivers/hid/hid-mcp2221.c
@@ -121,6 +121,7 @@ struct mcp2221 {
u8 *rxbuf;
u8 txbuf[64];
int rxbuf_idx;
+ int rxbuf_size;
int status;
u8 cur_i2c_clk_div;
struct gpio_chip *gc;
@@ -323,12 +324,14 @@ static int mcp_i2c_smbus_read(struct mcp2221 *mcp,
mcp->txbuf[3] = (u8)(msg->addr << 1);
total_len = msg->len;
mcp->rxbuf = msg->buf;
+ mcp->rxbuf_size = msg->len;
} else {
mcp->txbuf[1] = smbus_len;
mcp->txbuf[2] = 0;
mcp->txbuf[3] = (u8)(smbus_addr << 1);
total_len = smbus_len;
mcp->rxbuf = smbus_buf;
+ mcp->rxbuf_size = smbus_len;
}
ret = mcp_send_data_req_status(mcp, mcp->txbuf, 4);
@@ -912,6 +915,10 @@ static int mcp2221_raw_event(struct hid_device *hdev,
mcp->status = -EINVAL;
break;
}
+ if (mcp->rxbuf_idx + data[3] > mcp->rxbuf_size) {
+ mcp->status = -EINVAL;
+ break;
+ }
buf = mcp->rxbuf;
memcpy(&buf[mcp->rxbuf_idx], &data[4], data[3]);
mcp->rxbuf_idx = mcp->rxbuf_idx + data[3];
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH AUTOSEL 7.0-5.10] HID: mcp2221: fix OOB write in mcp2221_raw_event()
2026-05-20 11:18 ` [PATCH AUTOSEL 7.0-5.10] HID: mcp2221: fix OOB write in mcp2221_raw_event() Sasha Levin
@ 2026-05-20 11:56 ` sashiko-bot
0 siblings, 0 replies; 12+ messages in thread
From: sashiko-bot @ 2026-05-20 11:56 UTC (permalink / raw)
To: Sasha Levin; +Cc: dmitry.torokhov, linux-input
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [High] The patch introduces `mcp->rxbuf_size` to prevent Out-of-Bounds (OOB) writes but fails to initialize it in `mcp_smbus_xfer()` for block operations.
Pre-existing issues:
- [High] The `mcp->rxbuf` pointer is never cleared after an I2C/SMBus transfer completes or times out, leaving a dangling pointer that can be exploited.
- [High] The `mcp2221_raw_event()` handler ignores the `size` parameter provided by the HID subsystem, leading to out-of-bounds reads on the kernel heap.
- [High] A data race exists between the I2C transfer context and the HID raw event context, which accesses `rxbuf` state variables without synchronization.
--
commit 161d8d13687380796daedd1f35b98269619cd589
Author: Florian Pradines <florian.pradines@gmail.com>
HID: mcp2221: fix OOB write in mcp2221_raw_event()
mcp2221_raw_event() copies device-supplied data into mcp->rxbuf at offset
rxbuf_idx without verifying that the copy fits within the destination buffer.
A device responding with up to 60 bytes to a small I2C/SMBus read can overflow
this buffer. This patch adds an rxbuf_size field to struct mcp2221 and checks
rxbuf_idx + data[3] <= rxbuf_size before performing the copy.
> diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c
> index ef3b5c77c38e3..badd4958bc12c 100644
> --- a/drivers/hid/hid-mcp2221.c
> +++ b/drivers/hid/hid-mcp2221.c
[ ... ]
> @@ -323,12 +324,14 @@ static int mcp_i2c_smbus_read(struct mcp2221 *mcp,
> mcp->txbuf[3] = (u8)(msg->addr << 1);
> total_len = msg->len;
> mcp->rxbuf = msg->buf;
> + mcp->rxbuf_size = msg->len;
> } else {
> mcp->txbuf[1] = smbus_len;
> mcp->txbuf[2] = 0;
> mcp->txbuf[3] = (u8)(smbus_addr << 1);
> total_len = smbus_len;
> mcp->rxbuf = smbus_buf;
> + mcp->rxbuf_size = smbus_len;
[Severity: High]
Does this overlook updating mcp->rxbuf_size for block operations in
mcp_smbus_xfer()?
In mcp_smbus_xfer(), mcp->rxbuf is manually assigned for block reads without
updating mcp->rxbuf_size:
drivers/hid/hid-mcp2221.c:mcp_smbus_xfer() {
...
case I2C_SMBUS_BLOCK_DATA:
if (read_write == I2C_SMBUS_READ) {
...
mcp->rxbuf_idx = 0;
mcp->rxbuf = data->block;
...
}
If rxbuf_size is 0, the new bounds check in mcp2221_raw_event() will always
fail, breaking legitimate SMBus block reads. If rxbuf_size holds a large stale
value from a prior transfer, the check might pass and still allow an overflow
on the data->block stack buffer.
> }
>
> ret = mcp_send_data_req_status(mcp, mcp->txbuf, 4);
[ ... ]
> @@ -912,6 +915,10 @@ static int mcp2221_raw_event(struct hid_device *hdev,
> mcp->status = -EINVAL;
> break;
> }
> + if (mcp->rxbuf_idx + data[3] > mcp->rxbuf_size) {
> + mcp->status = -EINVAL;
> + break;
> + }
[Severity: High]
This is a pre-existing issue, but does mcp2221_raw_event() ignore the size
parameter provided by the HID subsystem?
The driver unconditionally accesses data bytes like data[3], and later copies
data[3] bytes starting from &data[4], without verifying the actual length of
the received HID report buffer. If a malicious device provides a small report
size, could reading or copying these bytes result in out-of-bounds reads on
the kernel heap?
[Severity: High]
This isn't a bug introduced by this patch, but is there a data race between
the I2C transfer context and the HID raw event context?
While mcp_i2c_smbus_read() and mcp_smbus_xfer() update mcp->rxbuf,
mcp->rxbuf_size, and mcp->rxbuf_idx while holding mcp->lock,
mcp2221_raw_event() runs asynchronously in the HID input report context and
accesses these variables without acquiring the lock.
If a stray or malicious USB report arrives concurrently with a new I2C read
request, could the state become desynchronized and lead to an out-of-bounds
write?
> buf = mcp->rxbuf;
[Severity: High]
This is a pre-existing issue, but is the mcp->rxbuf pointer left dangling
after a transfer completes or times out?
During transfers, mcp->rxbuf points to a caller-provided buffer which is often
allocated on the kernel stack. When a transfer times out, the stack frame is
popped, but mcp->rxbuf still points to the now-invalid stack address.
If a delayed or spurious USB report arrives later, could mcp2221_raw_event()
copy device-controlled data into this stale pointer and corrupt the kernel
stack?
> memcpy(&buf[mcp->rxbuf_idx], &data[4], data[3]);
> mcp->rxbuf_idx = mcp->rxbuf_idx + data[3];
> mcp->status = 0;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260520111944.3424570-25-sashal@kernel.org?part=1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH AUTOSEL 7.0-5.10] HID: elan: Add support for ELAN SB974D touchpad
2026-05-20 11:18 [PATCH AUTOSEL 7.0-6.12] HID: logitech-hidpp: Add support for newer Bluetooth keyboards Sasha Levin
2026-05-20 11:18 ` [PATCH AUTOSEL 7.0-5.10] HID: magicmouse: Prevent out-of-bounds (OOB) read during DOUBLE_REPORT_ID Sasha Levin
2026-05-20 11:18 ` [PATCH AUTOSEL 7.0-5.10] HID: mcp2221: fix OOB write in mcp2221_raw_event() Sasha Levin
@ 2026-05-20 11:19 ` Sasha Levin
2026-05-20 12:24 ` sashiko-bot
2026-05-20 11:19 ` [PATCH AUTOSEL 7.0-6.12] HID: i2c-hid: add reset quirk for BLTP7853 touchpad Sasha Levin
` (3 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Sasha Levin @ 2026-05-20 11:19 UTC (permalink / raw)
To: patches, stable
Cc: Damien Dejean, Kornel Dulęba, Jiri Kosina, Sasha Levin,
jikos, bentiss, linux-input, linux-kernel
From: Damien Dejean <damiendejean@google.com>
[ Upstream commit 55ce1858848132ed074fe907f00b5ce1ccab0ce1 ]
Elan SB974D touchpad uses ELAN_MT_I2C format to send HID reports. Add an
entry to match for the device and parse its vendor specific format.
Signed-off-by: Damien Dejean <damiendejean@google.com>
Signed-off-by: Kornel Dulęba <korneld@google.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Phase 1: Commit Message Forensics
Step 1.1 Record: subsystem `HID: elan`; action verb `Add support`;
intent is to add ELAN SB974D touchpad matching so the existing ELAN HID
driver parses its vendor-specific I2C multitouch reports.
Step 1.2 Record: tags are:
- `Signed-off-by: Damien Dejean <damiendejean@google.com>`
- `Signed-off-by: Kornel Dulęba <korneld@google.com>`
- `Signed-off-by: Jiri Kosina <jkosina@suse.com>`
No `Fixes:`, `Reported-by:`, `Tested-by:`, `Reviewed-by:`, `Acked-by:`,
`Cc: stable`, or `Link:` tags are present.
Step 1.3 Record: the body says SB974D uses `ELAN_MT_I2C` report format
and needs an ID table entry so `hid-elan` handles and parses that
format. Symptom is unsupported or generic handling of this touchpad, not
a crash. No affected-version statement.
Step 1.4 Record: this is not a hidden memory/race/crash fix. It is a
hardware enablement/device-ID addition to an existing driver, which is a
stable exception category.
## Phase 2: Diff Analysis
Step 2.1 Record: two files changed, 2 insertions total:
- `drivers/hid/hid-elan.c`: adds one `HID_I2C_DEVICE()` entry to
`elan_devices`.
- `drivers/hid/hid-ids.h`: adds `USB_DEVICE_ID_SB974D 0x0400`.
Modified data structure: `elan_devices[]`. Scope: single-driver,
surgical ID addition.
Step 2.2 Record: before, ELAN I2C product `0x0400` did not match `hid-
elan`; after, BUS_I2C vendor `0x04f3` product `0x0400` matches `hid-
elan` and uses existing probe/raw-event paths.
Step 2.3 Record: bug category is hardware support/device ID addition. No
new parser, locking, memory management, or API change. The fix works by
routing this specific I2C HID device to already-existing `ELAN_MT_I2C`
handling.
Step 2.4 Record: fix quality is high: two-line addition, localized,
mirrors the existing Toshiba I2C ELAN entry, and has very low regression
risk. Main possible risk is binding product `0x0400` to `hid-elan`, but
the commit message and external fwupd discussion verify `04f3:0400` is
an ELAN touchpad PID.
## Phase 3: Git History Investigation
Step 3.1 Record: `git blame` shows `elan_devices[]` was introduced by
`9a6a4193d65b` in v4.17-rc1 range; the existing I2C Toshiba entry was
introduced by `e7ad3dc9f4a2` in v4.19-rc1 range. This means the required
driver and I2C-format support are old and present in long-term stable
lines.
Step 3.2 Record: no `Fixes:` tag, so no introducing bug commit to
follow.
Step 3.3 Record: recent history shows this commit is standalone; only
later related file change found was unrelated `HID: i2c-hid: add reset
quirk for BLTP7853 touchpad`.
Step 3.4 Record: author Damien Dejean has only this HID commit in the
checked `origin/master` HID history. Jiri Kosina committed/applied it;
`MAINTAINERS` lists Jiri Kosina and Benjamin Tissoires as HID core
maintainers.
Step 3.5 Record: no prerequisite commits found beyond the existing `hid-
elan` driver and I2C parser, both already present in checked stable
branches.
## Phase 4: Mailing List And External Research
Step 4.1 Record: `b4 dig -c 55ce185884813` found the original patch at `
https://patch.msgid.link/20260414133858.3992799-1-
damiendejean@google.com`. `b4 dig -a` showed only v1, no later
revisions.
Step 4.2 Record: `b4 dig -w` showed recipients included Jiri Kosina,
Benjamin Tissoires, `linux-input@vger.kernel.org`, `linux-
kernel@vger.kernel.org`, Damien Dejean, and Kornel Dulęba.
Step 4.3 Record: no `Reported-by` or bug-report `Link:`. The lore thread
mbox contains Jiri Kosina’s “Applied to hid.git#for-7.1/upstream-fixes,
thanks.” No NAKs or concerns in the saved thread.
Step 4.4 Record: no multi-patch series; standalone patch.
Step 4.5 Record: lore WebFetch hit Anubis, but `b4` successfully fetched
the mbox. Stable branch grep found no existing `SB974D` backport in
`stable/linux-7.0.y` or `stable/linux-6.19.y`.
## Phase 5: Code Semantic Analysis
Step 5.1 Record: modified table is `elan_devices[]`; functional paths
affected are existing callbacks `elan_probe`, `elan_input_mapping`,
`elan_input_configured`, and `elan_raw_event`.
Step 5.2 Record: `hid_match_device()` matches `hdrv->id_table`;
`hid_device_probe()` calls the matched driver’s `.probe`;
`hid_input_report()` calls `.raw_event`. Thus the new ID makes this
hardware reach existing `hid-elan` probe and report parsing.
Step 5.3 Record: relevant callees include `hid_parse`, `hid_hw_start`,
`elan_start_multitouch`, and for I2C reports `elan_i2c_report_input`.
Step 5.4 Record: reachability is hardware enumeration and HID input
report delivery. Users of the SB974D touchpad hit this during device
probe and normal touch input.
Step 5.5 Record: similar pattern exists in the same driver for
`USB_DEVICE_ID_TOSHIBA_CLICK_L9W`, also using `HID_I2C_DEVICE()` and the
existing `ELAN_MT_I2C` path.
## Phase 6: Stable Tree Analysis
Step 6.1 Record: checked stable branches `5.10.y`, `5.15.y`, `6.1.y`,
`6.6.y`, `6.12.y`, `6.18.y`, `6.19.y`, and `7.0.y`; all contain
`CONFIG_HID_ELAN`, `drivers/hid/hid-elan.c`, and the existing
`TOSHIBA_CLICK_L9W` I2C ELAN entry.
Step 6.2 Record: `git apply --check` succeeded on current `7.0.y`,
`6.19.y`, and representative older stable worktrees `5.10.y`, `5.15.y`,
`6.1.y`, `6.6.y`, `6.12.y`, `6.18.y`.
Step 6.3 Record: no related `SB974D` fix already present in checked
stable branches.
## Phase 7: Subsystem Context
Step 7.1 Record: subsystem is HID input driver support. Criticality is
driver-specific/peripheral, affecting users of this ELAN SB974D touchpad
hardware.
Step 7.2 Record: HID is actively maintained; this patch was applied
through `hid.git#for-7.1/upstream-fixes` by HID maintainer Jiri Kosina.
## Phase 8: Impact And Risk
Step 8.1 Record: affected population is hardware-specific: systems with
ELAN SB974D I2C touchpad and `CONFIG_HID_ELAN`.
Step 8.2 Record: trigger is device enumeration and normal touchpad input
on that hardware. No evidence found that unprivileged users can trigger
a security issue; this is hardware functionality.
Step 8.3 Record: failure mode without the patch is lack of proper `hid-
elan` handling for this touchpad’s vendor-specific `ELAN_MT_I2C` report
format. Severity is medium for affected hardware because touchpad
functionality is user-visible, but not a crash/corruption/security
issue.
Step 8.4 Record: benefit is high for affected hardware and low globally.
Risk is very low: two lines, no behavior change for unrelated device
IDs, no API change, and clean apply to checked stable branches.
## Phase 9: Final Synthesis
Evidence for backporting: this is exactly the stable exception for
adding a new device ID to an existing driver; the driver and parser
already exist in stable; the patch is tiny, cleanly applies, and was
accepted by the HID maintainer. Evidence against: it is hardware
enablement rather than a conventional critical bug fix, and there is no
user bug report or `Tested-by` tag in the commit.
Stable rules:
1. Obviously correct and tested? Code is obviously correct by
inspection; explicit testing tag absent.
2. Fixes a real user-visible issue? Yes for affected SB974D systems:
proper touchpad driver binding/report parsing.
3. Important issue? Medium hardware functionality issue, not
crash/corruption/security.
4. Small and contained? Yes, 2 insertions in HID.
5. No new APIs/features? No API; hardware ID addition only.
6. Can apply to stable? Yes, verified across representative active
stable branches.
Exception category: new device ID added to an existing driver. This is
stable-suitable.
## Verification
- Phase 1: `git show --format=fuller 55ce185884813` verified subject,
body, author, committer, and tags.
- Phase 2: `git show --stat` and full diff verified exactly 2 insertions
in `hid-elan.c` and `hid-ids.h`.
- Phase 3: `git blame` verified table history; `git describe --contains`
placed original driver support around v4.17-rc1 and I2C support around
v4.19-rc1.
- Phase 3: `git log --oneline` checks found no dependent SB974D series
and only this author’s HID commit in the checked range.
- Phase 4: `b4 dig`, `b4 dig -a`, and `b4 dig -w` verified original lore
submission, single v1, recipients, and maintainer application.
- Phase 4: WebFetch to lore/stable was blocked by Anubis; the saved `b4`
mbox was used instead.
- Phase 5: `ReadFile` and `rg` verified `elan_probe`, `elan_raw_event`,
`elan_i2c_report_input`, `hid_match_device`, and `hid_input_report`
call paths.
- Phase 6: `git grep` verified relevant stable branches contain
`HID_ELAN` and the existing ELAN I2C entry.
- Phase 6: `git apply --check` verified clean application to `7.0.y`,
`6.19.y`, `6.18.y`, `6.12.y`, `6.6.y`, `6.1.y`, `5.15.y`, and
`5.10.y`.
- Phase 7: `MAINTAINERS` verified Jiri Kosina and Benjamin Tissoires as
HID maintainers.
- Phase 8: no sanitizer report, crash report, or security impact was
found; decision is driven by the verified device-ID stable exception.
**YES**
drivers/hid/hid-elan.c | 1 +
drivers/hid/hid-ids.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/hid/hid-elan.c b/drivers/hid/hid-elan.c
index 76d93fc48f6a2..0190ad567ce4d 100644
--- a/drivers/hid/hid-elan.c
+++ b/drivers/hid/hid-elan.c
@@ -513,6 +513,7 @@ static const struct hid_device_id elan_devices[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_ELAN, USB_DEVICE_ID_HP_X2_10_COVER),
.driver_data = ELAN_HAS_LED },
{ HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, USB_DEVICE_ID_TOSHIBA_CLICK_L9W) },
+ { HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, USB_DEVICE_ID_SB974D) },
{ }
};
MODULE_DEVICE_TABLE(hid, elan_devices);
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index c1e4a6ce96317..c371721826dcc 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -455,6 +455,7 @@
#define USB_DEVICE_ID_EDIFIER_QR30 0xa101 /* EDIFIER Hal0 2.0 SE */
#define USB_VENDOR_ID_ELAN 0x04f3
+#define USB_DEVICE_ID_SB974D 0x0400
#define USB_DEVICE_ID_TOSHIBA_CLICK_L9W 0x0401
#define USB_DEVICE_ID_HP_X2 0x074d
#define USB_DEVICE_ID_HP_X2_10_COVER 0x0755
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH AUTOSEL 7.0-5.10] HID: elan: Add support for ELAN SB974D touchpad
2026-05-20 11:19 ` [PATCH AUTOSEL 7.0-5.10] HID: elan: Add support for ELAN SB974D touchpad Sasha Levin
@ 2026-05-20 12:24 ` sashiko-bot
0 siblings, 0 replies; 12+ messages in thread
From: sashiko-bot @ 2026-05-20 12:24 UTC (permalink / raw)
To: Sasha Levin; +Cc: dmitry.torokhov, linux-input
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] The driver fails to restore the device to multitouch mode after a system suspend/resume cycle.
- [High] In USB mode, if more than two fingers are active, the driver assigns identical coordinates to all fingers after the first one.
--
commit d5ae02b47586e24db0bca9c6a7b552b9fc7ef6e4
Author: Damien Dejean <damiendejean@google.com>
HID: elan: Add support for ELAN SB974D touchpad
This commit adds support for the ELAN SB974D touchpad by registering its
device ID in the supported devices list so the driver can parse its
vendor-specific formats.
> diff --git a/drivers/hid/hid-elan.c b/drivers/hid/hid-elan.c
> index 76d93fc48f6a2..0190ad567ce4d 100644
> --- a/drivers/hid/hid-elan.c
> +++ b/drivers/hid/hid-elan.c
> @@ -513,6 +513,7 @@ static const struct hid_device_id elan_devices[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_ELAN, USB_DEVICE_ID_HP_X2_10_COVER),
> .driver_data = ELAN_HAS_LED },
> { HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, USB_DEVICE_ID_TOSHIBA_CLICK_L9W) },
> + { HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, USB_DEVICE_ID_SB974D) },
[Severity: High]
This is a pre-existing issue, but does the driver fail to restore the device
to multitouch mode after a system suspend/resume cycle?
The ELAN touchpads require a vendor-specific initialization command
(elan_start_multitouch) to disable mouse emulation and enable multitouch
mode. This sequence is currently only executed during the initial elan_probe.
The hid_driver struct for elan_driver does not provide resume or reset_resume
callbacks. When the system suspends and the device loses power (both in USB
and I2C modes), it reverts to its default mouse emulation mode.
Upon resume, the core HID drivers call hid_driver_reset_resume(), which acts
as a no-op since the callback is missing and returns success without rebinding.
Does this leave the touchpad permanently in a degraded state after a
suspend/resume cycle?
[Severity: High]
This is a pre-existing issue, but in USB mode, if more than two fingers are
active, does the driver assign identical coordinates to all fingers after the
first one?
Looking at elan_usb_report_input(), when parsing the ELAN_MT_SECOND_FINGER
packet, the driver iterates over ELAN_MAX_FINGERS to dispatch slot updates
based on the active touch bits in prev_report[2].
For the first active bit found, it correctly uses coordinates from
prev_report + 3. For all subsequent active bits, the else branch executes:
elan_report_mt_slot(drvdata, data + 1, i);
Because data + 1 is passed directly each time without incrementing any offset,
are the exact same coordinates and pressure values assigned to all fingers
after the first?
> { }
> };
> MODULE_DEVICE_TABLE(hid, elan_devices);
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260520111944.3424570-28-sashal@kernel.org?part=1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH AUTOSEL 7.0-6.12] HID: i2c-hid: add reset quirk for BLTP7853 touchpad
2026-05-20 11:18 [PATCH AUTOSEL 7.0-6.12] HID: logitech-hidpp: Add support for newer Bluetooth keyboards Sasha Levin
` (2 preceding siblings ...)
2026-05-20 11:19 ` [PATCH AUTOSEL 7.0-5.10] HID: elan: Add support for ELAN SB974D touchpad Sasha Levin
@ 2026-05-20 11:19 ` Sasha Levin
2026-05-20 11:19 ` [PATCH AUTOSEL 7.0-6.1] HID: google: hammer: stop hardware on devres action failure Sasha Levin
` (2 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Sasha Levin @ 2026-05-20 11:19 UTC (permalink / raw)
To: patches, stable
Cc: Xu Rao, Jiri Kosina, Sasha Levin, jikos, bentiss, linux-input,
linux-kernel
From: Xu Rao <raoxu@uniontech.com>
[ Upstream commit a991aa5e89365ba1959fae6847fd288125b209e5 ]
The BLTP7853 I2C HID touchpad may fail to probe after reboot or
reprobe because reset completion is not signalled to the host. The
driver then waits for the reset-complete interrupt until it times out
and the device probe fails:
i2c_hid i2c-BLTP7853:00: failed to reset device.
i2c_hid i2c-BLTP7853:00: can't add hid device: -61
i2c_hid: probe of i2c-BLTP7853:00 failed with error -61
Add I2C_HID_QUIRK_NO_IRQ_AFTER_RESET for the device so i2c-hid does
not wait for a reset interrupt that may never arrive.
Signed-off-by: Xu Rao <raoxu@uniontech.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Phase Walkthrough
### Phase 1: Commit Message Forensics
Record: subsystem `HID: i2c-hid`; action verb `add`; intent is to add a
reset-handling quirk for the BLTP7853 touchpad.
Record: tags present are `Signed-off-by: Xu Rao <raoxu@uniontech.com>`
and `Signed-off-by: Jiri Kosina <jkosina@suse.com>`. No `Fixes:`,
`Reported-by:`, `Tested-by:`, `Reviewed-by:`, `Acked-by:`, `Link:`, or
`Cc: stable@vger.kernel.org` tags are present in the supplied message.
Record: the commit message describes a real probe failure: the device
may not signal reset completion after reboot or reprobe, the driver
waits for reset completion, and probe fails with `-61` / `-ENODATA`. The
root cause given is missing reset-complete IRQ from the device.
Record: this is not hidden as cleanup; it is explicitly a hardware
workaround for a probe failure.
### Phase 2: Diff Analysis
Record: files changed are `drivers/hid/hid-ids.h` with 3 additions and
`drivers/hid/i2c-hid/i2c-hid-core.c` with 2 additions. Total scope: 5
added lines, no removals. No function body is changed; the modified
object is the private `i2c_hid_quirks[]` table.
Record: before the patch, vendor/product `0x36b6/0xc001` had no private
i2c-hid quirk entry, so the reset path used normal reset-ack waiting.
After the patch, `i2c_hid_lookup_quirk()` can match the BLTP7853 IDs and
set `I2C_HID_QUIRK_NO_IRQ_AFTER_RESET`.
Record: in current code, `I2C_HID_QUIRK_NO_IRQ_AFTER_RESET` makes
`i2c_hid_finish_hwreset()` sleep 100 ms and clear
`I2C_HID_RESET_PENDING` instead of waiting for a reset IRQ. In older
stable code, the same quirk skips the 5 second wait path that returns
`-ENODATA`.
Record: bug category is hardware workaround / quirk. Specific mechanism:
a device that does not raise the expected reset-complete interrupt is
handled through an existing no-IRQ-after-reset path.
Record: fix quality is high: tiny table-only change, uses existing
infrastructure, no new API, no locking change, no cross-subsystem
behavior. Regression risk is very low and limited to devices reporting
exactly the new vendor/product pair.
### Phase 3: Git History Investigation
Record: exact candidate subject was not found in local `git log` on
`master`, `all-next`, `fixes-next`, `input-next`, `pending-7.0`, or
`for-greg/7.0-200`; no candidate commit hash was available to blame the
newly added lines.
Record: existing no-IRQ reset quirk support was introduced by
`402946a8ef71e` (`HID: i2c-hid: Add no-irq-after-reset quirk for
0911:5288 device`), contained since `v4.15-rc1`. Related Voyo quirk
`fc6a31b007393` shows the same failure pattern: repeated `failed to
reset device` and final `can't add hid device: -61`.
Record: no `Fixes:` tag is present, so Step 3.2 is not applicable.
Record: recent history contains normal HID/i2c-hid quirk and fix
activity. Relevant related changes include `7bcf9ebb50f2a` (`Turn
missing reset ack into a warning`) and older device-specific no-IRQ
quirks. No prerequisite was identified beyond existing
`I2C_HID_QUIRK_NO_IRQ_AFTER_RESET` support.
Record: local `git log --author="Xu Rao" -- drivers/hid` found no HID
commits on `master`. `MAINTAINERS` confirms Jiri Kosina is an HID core
maintainer, and he signed off the supplied commit.
### Phase 4: Mailing List And External Research
Record: candidate `b4 dig` could not be performed because the exact
candidate commit hash is unavailable and the subject is not present in
local reachable history.
Record: `b4 dig -c 402946a8ef71e` found the original related no-IRQ-
after-reset quirk submission at
`https://patch.msgid.link/20171107122800.23196-1-hdegoede@redhat.com`;
`b4 dig -a` showed v1 and v2; `b4 dig -w` showed HID maintainers and
`linux-input@vger.kernel.org` were included.
Record: `b4 dig -c 7bcf9ebb50f2a` found the reset-ack warning patch as
`[PATCH v3 5/7]`.
Record: web searches for the exact BLTP7853 reset-quirk subject and
stable-list discussion found no direct candidate thread. Searches did
find related BLTP7853 touchpad reports, but not independent confirmation
of this exact `0x36b6/0xc001` reset failure.
### Phase 5: Code Semantic Analysis
Record: modified key object is `i2c_hid_quirks[]`; relevant functions
are `i2c_hid_lookup_quirk()`, `__i2c_hid_core_probe()`,
`i2c_hid_finish_hwreset()`, and older-branch `i2c_hid_command()`
behavior.
Record: callers verified: `i2c_hid_core_probe()` is called from ACPI,
OF, ELAN, and Goodix i2c-hid probe drivers. `__i2c_hid_core_probe()`
reads HID descriptor vendor/product IDs and calls
`i2c_hid_lookup_quirk()`.
Record: call chain is device enumeration/probe: platform driver probe ->
`i2c_hid_core_probe()` -> `__i2c_hid_core_probe()` ->
`i2c_hid_lookup_quirk()` -> HID registration -> reset during HID parse.
This is reachable during boot, reboot, and reprobe for affected
hardware.
Record: similar patterns exist for HANTICK, ITE/Voyo, and Raydium
devices already using `I2C_HID_QUIRK_NO_IRQ_AFTER_RESET`.
### Phase 6: Stable Tree Analysis
Record: `I2C_HID_QUIRK_NO_IRQ_AFTER_RESET` exists in active checked
stable branches `5.10.y`, `5.15.y`, `6.1.y`, `6.6.y`, `6.12.y`,
`6.18.y`, `6.19.y`, and `7.0.y`.
Record: BLTP-specific IDs from this patch are not present in those
checked branches or `master`.
Record: `5.10.y`, `5.15.y`, `6.1.y`, and `6.6.y` still have the reset-
ack timeout returning `-ENODATA`, matching the commit-message failure.
`6.12.y` and newer checked branches contain `7bcf9ebb50f2a`, where
missing reset ack is downgraded to a warning, so the benefit there is
mostly avoiding a delay/warning rather than fixing probe failure.
Record: patch applies cleanly to current `stable/linux-7.0.y` when
checked with `git apply --check`. Older stable trees may need minor
context adjustment due to file layout differences, but the required
quirk mechanism exists.
### Phase 7: Subsystem And Maintainer Context
Record: subsystem is HID/i2c-hid under `drivers/hid/`, criticality
`IMPORTANT` for users of affected laptop touchpad hardware.
Record: subsystem is actively maintained; `MAINTAINERS` lists HID core
as maintained by Jiri Kosina and Benjamin Tissoires.
### Phase 8: Impact And Risk
Record: affected population is hardware-specific: systems with an I2C
HID device reporting the BLTP7853 IDs added by this patch.
Record: trigger condition is probe/reprobe after reboot or reprobe, as
described by the commit message and matched to the older stable reset
path.
Record: failure severity is HIGH for older stable branches because the
touchpad can fail to bind at all. For newer branches with
`7bcf9ebb50f2a`, severity is LOW-MEDIUM because generic handling avoids
probe failure but still waits/logs.
Record: benefit is high for older stable users with this hardware; risk
is very low because the change is a narrow hardware quirk table entry.
### Phase 9: Final Synthesis
Evidence for backporting: real device probe failure, existing quirk
mechanism, tiny hardware-specific change, no new API, maintainer sign-
off, and older stable branches demonstrably still have the failing reset
timeout behavior.
Evidence against backporting: no direct candidate lore thread or
external bug report could be verified; exact hardware ID `0x36b6/0xc001`
was only verified from the supplied patch, not an external hardware
database. Newer stable branches already have generic missing-reset-ack
handling, reducing urgency there.
Stable rules: obviously correct by inspection, fixes a real hardware
probe failure on older stable branches, small and contained, no new
feature/API, and falls under the accepted hardware quirk exception.
Backport difficulty should be clean or minor.
## Verification
- [Phase 1] Parsed supplied message and tags: only Xu Rao and Jiri
Kosina Signed-off-by tags present.
- [Phase 2] Read `drivers/hid/i2c-hid/i2c-hid-core.c`: verified quirk
table and reset handling.
- [Phase 3] Searched local histories without `--all`: exact candidate
not found; related no-IRQ quirk commits found.
- [Phase 3] Inspected `402946a8ef71e`, `fc6a31b007393`, and
`7bcf9ebb50f2a`: confirmed same reset-ack quirk mechanism and later
generic warning behavior.
- [Phase 4] Ran `b4 dig` for related commits; candidate `b4 dig` was not
possible without a candidate hash.
- [Phase 5] Traced callers with `rg` and read ACPI/OF probe files:
confirmed probe-time reachability.
- [Phase 6] Checked active stable branches: quirk support exists in all
checked branches; old reset failure path exists through `6.6.y`;
`6.12.y+` has generic warning handling.
- [Phase 6] Ran `git apply --check` against current `7.0.y`: patch shape
applies cleanly.
- UNVERIFIED: original candidate lore discussion, independent bug
report, and exact external hardware database confirmation for
`0x36b6/0xc001`.
**YES**
drivers/hid/hid-ids.h | 3 +++
drivers/hid/i2c-hid/i2c-hid-core.c | 2 ++
2 files changed, 5 insertions(+)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index c371721826dcc..559040e47f3c7 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -277,6 +277,9 @@
#define USB_VENDOR_ID_BIGBEN 0x146b
#define USB_DEVICE_ID_BIGBEN_PS3OFMINIPAD 0x0902
+#define I2C_VENDOR_ID_BLTP 0x36b6
+#define I2C_PRODUCT_ID_BLTP7853 0xc001
+
#define USB_VENDOR_ID_BTC 0x046e
#define USB_DEVICE_ID_BTC_EMPREX_REMOTE 0x5578
#define USB_DEVICE_ID_BTC_EMPREX_REMOTE_2 0x5577
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 5a183af3d5c6a..baff2728603ec 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -149,6 +149,8 @@ static const struct i2c_hid_quirks {
I2C_HID_QUIRK_BOGUS_IRQ },
{ I2C_VENDOR_ID_GOODIX, I2C_DEVICE_ID_GOODIX_0D42,
I2C_HID_QUIRK_DELAY_WAKEUP_AFTER_RESUME },
+ { I2C_VENDOR_ID_BLTP, I2C_PRODUCT_ID_BLTP7853,
+ I2C_HID_QUIRK_NO_IRQ_AFTER_RESET },
{ 0, 0 }
};
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH AUTOSEL 7.0-6.1] HID: google: hammer: stop hardware on devres action failure
2026-05-20 11:18 [PATCH AUTOSEL 7.0-6.12] HID: logitech-hidpp: Add support for newer Bluetooth keyboards Sasha Levin
` (3 preceding siblings ...)
2026-05-20 11:19 ` [PATCH AUTOSEL 7.0-6.12] HID: i2c-hid: add reset quirk for BLTP7853 touchpad Sasha Levin
@ 2026-05-20 11:19 ` Sasha Levin
2026-05-20 11:19 ` [PATCH AUTOSEL 7.0-6.6] HID: sony: add missing size validation for SMK-Link remotes Sasha Levin
2026-05-20 11:19 ` [PATCH AUTOSEL 7.0-5.15] HID: ft260: validate i2c input report length Sasha Levin
6 siblings, 0 replies; 12+ messages in thread
From: Sasha Levin @ 2026-05-20 11:19 UTC (permalink / raw)
To: patches, stable
Cc: Myeonghun Pak, Jiri Kosina, Sasha Levin, jikos, bentiss,
linux-input, linux-kernel
From: Myeonghun Pak <mhun512@gmail.com>
[ Upstream commit b08665fe80fab0956e64741c07d9bbcec635c34d ]
hammer_probe() starts the HID hardware before registering the devres
action that stops it. If devm_add_action() fails, probe returns an
error with the hardware still started because the cleanup action was
never registered and the driver's remove callback is not called after a
failed probe.
Use devm_add_action_or_reset() so the stop action runs immediately on
registration failure while preserving the existing devres-managed cleanup
path for later probe failures and remove.
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Phase Walkthrough
### Phase 1: Commit Message Forensics
- Step 1.1 Record: subsystem `HID: google: hammer`; action verb `stop`;
intent is to stop HID hardware if devres action registration fails.
- Step 1.2 Record: tags in committed
`b08665fe80fab0956e64741c07d9bbcec635c34d`: `Signed-off-by: Myeonghun
Pak <mhun512@gmail.com>` and `Signed-off-by: Jiri Kosina
<jkosina@suse.com>`. No `Fixes:`, `Reported-by:`, `Tested-by:`,
`Reviewed-by:`, `Acked-by:`, `Link:`, or `Cc: stable`.
- Step 1.3 Record: bug is an error-path cleanup failure.
`hammer_probe()` calls `hid_hw_start()` before registering
`hammer_stop` with devres; if `devm_add_action()` returns `-ENOMEM`,
probe fails and no registered cleanup action calls `hid_hw_stop()`.
- Step 1.4 Record: yes, this is a hidden bug fix: a one-line
devres/error-path cleanup fix preventing started HID hardware from
being left active after failed probe.
### Phase 2: Diff Analysis
- Step 2.1 Record: one file, `drivers/hid/hid-google-hammer.c`; 1
insertion, 1 deletion; modified function `hammer_probe()`; single-file
surgical fix.
- Step 2.2 Record: before, failure of `devm_add_action()` returned
directly after `hid_hw_start()` succeeded. After,
`devm_add_action_or_reset()` runs `hammer_stop(hdev)` immediately if
action registration fails.
- Step 2.3 Record: bug category is error-path resource cleanup. The
broken state is missing `hid_hw_stop()` after successful
`hid_hw_start()` when devres action allocation fails.
- Step 2.4 Record: fix quality is high: it uses the kernel helper whose
inline definition calls the action on registration failure. No
unrelated changes, no public API change, and no normal-path behavior
change when registration succeeds.
### Phase 3: Git History Investigation
- Step 3.1 Record: blame shows `devm_add_action(&hdev->dev, hammer_stop,
hdev)` came from `d950db3f80a801` (`HID: google: switch to devm when
registering keyboard backlight LED`), first contained by `v5.18-rc1`.
- Step 3.2 Record: no `Fixes:` tag. I followed the blamed introducing
commit instead; `d950db3f80a801` added `hammer_stop()` and moved stop
cleanup into devres.
- Step 3.3 Record: recent file history shows related Google HID
cleanup/device-ID/vivaldi work, but no intermediate fix for this exact
missed cleanup.
- Step 3.4 Record: candidate author has only this HID commit in
`origin/master` in the checked history. Jiri Kosina is listed in
`MAINTAINERS` for HID core and applied/signed the commit.
- Step 3.5 Record: no dependency found. The replacement helper exists in
affected stable branches checked.
### Phase 4: Mailing List And External Research
- Step 4.1 Record: `b4 dig -c b08665fe80fab` found the original patch at
`https://patch.msgid.link/20260424125043.52639-1-
pakmyeonghun@bagmyeonghun-ui-MacBookPro.local`. `b4 dig -a` found only
v1.
- Step 4.2 Record: `b4 dig -w` showed the original recipients were
Myeonghun Pak, Jiri Kosina, Benjamin Tissoires, `linux-
input@vger.kernel.org`, and `linux-kernel@vger.kernel.org`.
- Step 4.3 Record: no separate bug report or `Reported-by` tag was
present.
- Step 4.4 Record: thread had 3 messages; Jiri replied, “Makes sense,
thanks for catching it. Applied.” No NAKs or objections found in the
fetched mbox.
- Step 4.5 Record: direct lore stable web search was blocked by Anubis;
local stable branch log search did not find this commit already
present.
### Phase 5: Code Semantic Analysis
- Step 5.1 Record: modified function is `hammer_probe()`.
- Step 5.2 Record: `hammer_probe()` is registered as `.probe` in
`hammer_driver`; HID core reaches it via `hid_device_probe()` ->
`__hid_device_probe()` -> `hdrv->probe(hdev, id)`.
- Step 5.3 Record: key callees are `hid_parse()`, `hid_hw_start()`,
`devm_add_action_or_reset()`, `hid_hw_open()`, and
`hammer_register_leds()`. `hid_hw_start()` documentation says
`hid_hw_stop()` must be called after success.
- Step 5.4 Record: path is reachable during HID device probe for Google
Hammer-family USB IDs in the driver table. Trigger for the bug is
allocation failure inside `devm_add_action()` after hardware start.
- Step 5.5 Record: no other `hammer_stop` pattern found. Nearby HID
drivers commonly call `hid_hw_stop()` on probe-error paths, supporting
that this cleanup is expected.
### Phase 6: Stable Tree Analysis
- Step 6.1 Record: buggy code exists in `stable/linux-6.1.y`, `6.6.y`,
`6.12.y`, `6.15.y`, `6.16.y`, `6.17.y`, `6.18.y`, and `6.19.y`; not in
`stable/linux-5.15.y`.
- Step 6.2 Record: patch applied cleanly with `git apply --check` on
temporary worktrees for `6.1.y`, `6.6.y`, `6.12.y`, and `6.19.y`.
- Step 6.3 Record: no related stable fix with the same subject was found
in checked stable branch logs.
### Phase 7: Subsystem Context
- Step 7.1 Record: subsystem is HID driver code, specifically Google
Hammer HID devices; criticality is driver-specific/peripheral.
- Step 7.2 Record: HID is actively maintained; recent `drivers/hid`
history includes multiple fixes and quirks, and the candidate was
merged through the HID tree.
### Phase 8: Impact And Risk
- Step 8.1 Record: affected users are users of Google Hammer-family HID
hardware on stable kernels containing `d950db3f80a801`.
- Step 8.2 Record: trigger is uncommon but real: memory/devres
allocation failure during device probe after `hid_hw_start()`
succeeds. I did not verify unprivileged syscall reachability.
- Step 8.3 Record: failure mode is leaked/active HID hardware state
after failed probe, not a verified crash or data corruption. Severity
is medium, with higher concern under memory pressure because cleanup
is explicitly required by HID core semantics.
- Step 8.4 Record: benefit is moderate for affected hardware and error
recovery correctness; risk is very low because the successful path is
unchanged and the failure path calls the already intended cleanup
function.
### Phase 9: Final Synthesis
- Step 9.1 Record: evidence for backporting: real cleanup bug, affected
maintained stable trees, one-line obviously correct fix, uses existing
helper, applies cleanly, maintainer accepted it. Evidence against: no
reported user crash, rare trigger, driver-specific impact. Unresolved:
no runtime test report or concrete field failure report found.
- Step 9.2 Record: stable rules: obviously correct yes; fixes real bug
yes; important enough as resource/hardware-state leak on failed probe
yes, though not crash-class; small and contained yes; no new
feature/API yes; applies to stable yes for checked branches.
- Step 9.3 Record: no automatic exception category; this is not a device
ID, quirk, DT, build, or documentation patch.
- Step 9.4 Record: risk-benefit favors backporting because the fix is a
minimal correction to a verified cleanup bug in stable-present code.
## Verification
- Phase 1: inspected `git show --stat --no-ext-diff b08665fe80fab`;
confirmed subject, body, tags, and 1-line diff.
- Phase 2: inspected full candidate diff; confirmed only
`devm_add_action()` -> `devm_add_action_or_reset()` in
`hammer_probe()`.
- Phase 3: ran blame on changed area; confirmed introducing commit
`d950db3f80a801`, first contained by `v5.18-rc1`.
- Phase 3: inspected `d950db3f80a801`; confirmed it added devres-managed
`hammer_stop()`.
- Phase 4: ran `b4 dig -c`, `-a`, and `-w`; confirmed original patch
URL, v1 only, maintainers/lists included.
- Phase 4: fetched full thread with `b4 mbox`; confirmed Jiri Kosina
replied that the patch made sense and was applied.
- Phase 5: read HID core and devres code; confirmed `hid_hw_start()`
requires `hid_hw_stop()`, and `devm_add_action_or_reset()` invokes the
action on failure.
- Phase 6: checked stable branches with `git grep`; confirmed buggy line
in `6.1.y` through `6.19.y`, not `5.15.y`.
- Phase 6: ran `git apply --check` in temporary worktrees for `6.1.y`,
`6.6.y`, `6.12.y`, and `6.19.y`; all passed.
- UNVERIFIED: no separate user bug report, crash report, or runtime test
result was found.
**YES**
drivers/hid/hid-google-hammer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-google-hammer.c b/drivers/hid/hid-google-hammer.c
index 1af477e58480b..c99c3c0d442e1 100644
--- a/drivers/hid/hid-google-hammer.c
+++ b/drivers/hid/hid-google-hammer.c
@@ -496,7 +496,7 @@ static int hammer_probe(struct hid_device *hdev,
if (error)
return error;
- error = devm_add_action(&hdev->dev, hammer_stop, hdev);
+ error = devm_add_action_or_reset(&hdev->dev, hammer_stop, hdev);
if (error)
return error;
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH AUTOSEL 7.0-6.6] HID: sony: add missing size validation for SMK-Link remotes
2026-05-20 11:18 [PATCH AUTOSEL 7.0-6.12] HID: logitech-hidpp: Add support for newer Bluetooth keyboards Sasha Levin
` (4 preceding siblings ...)
2026-05-20 11:19 ` [PATCH AUTOSEL 7.0-6.1] HID: google: hammer: stop hardware on devres action failure Sasha Levin
@ 2026-05-20 11:19 ` Sasha Levin
2026-05-20 11:19 ` [PATCH AUTOSEL 7.0-5.15] HID: ft260: validate i2c input report length Sasha Levin
6 siblings, 0 replies; 12+ messages in thread
From: Sasha Levin @ 2026-05-20 11:19 UTC (permalink / raw)
To: patches, stable
Cc: Rosalie Wanders, Jiri Kosina, Sasha Levin, jikos, bentiss,
linux-input, linux-kernel
From: Rosalie Wanders <rosalie@mailbox.org>
[ Upstream commit a4170b63eda999d20ad6dc39ddc3ce5c1ac619e6 ]
This commit adds the missing size validation for SMK-Link remotes in
sony_raw_event(), this prevents a malicious device from allowing
hid-sony to read out of bounds of the provided buffer.
I do not own these devices so the size check only forces that the buffer
is large enough for nsg_mrxu_parse_report().
Signed-off-by: Rosalie Wanders <rosalie@mailbox.org>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Phase Walkthrough
Phase 1, commit message forensics. Record: subsystem `HID: sony`; action
`add`; intent is missing size validation for SMK-Link remotes. Tags
present: `Signed-off-by: Rosalie Wanders`, `Signed-off-by: Jiri Kosina`;
no `Fixes:`, `Reported-by:`, `Tested-by:`, `Reviewed-by:`, `Acked-by:`,
`Link:`, or `Cc: stable`. Body says a malicious device can make `hid-
sony` read beyond the provided buffer in `sony_raw_event()`. This is a
direct memory-safety bug, not a hidden cleanup.
Phase 2, diff analysis. Record: one file, `drivers/hid/hid-sony.c`, 2
insertions and 3 deletions; only `sony_raw_event()` changes. Before: any
`NSG_MRXU_REMOTE` report with `rd[0] == 0x02` called
`nsg_mrxu_parse_report()`. After: it only does so when `size >= 12`.
`nsg_mrxu_parse_report()` reads through `rd[11]`, including `rd[offset +
10]` with `offset = 1`, so the minimum size is verified by code. Bug
category: bounds check / out-of-bounds read. Fix quality: surgical and
obviously correct; the only behavioral change is rejecting malformed
short reports before parsing.
Phase 3, git history. Record: upstream commit is
`a4170b63eda999d20ad6dc39ddc3ce5c1ac619e6`, authored by Rosalie Wanders
and committed by HID maintainer Jiri Kosina. Blame shows the NSG-MRxU
parser and unchecked call were introduced by `b7289cb1cf99d` (`HID:
sony: Add touchpad support for NSG-MR5U and NSG-MR7U remotes`), first
described by `git describe` as `v4.17-rc1~118^2~2^2`. No `Fixes:` tag to
follow. Recent history shows an adjacent independent HID Sony size-
validation fix for Rock Band 3 Pro instruments, but no dependency for
this patch. Author has multiple recent `hid-sony` commits.
Phase 4, mailing list and external research. Record: `b4 dig -c
a4170b63eda99` found the exact lore submission at
`https://patch.msgid.link/20260412010806.7997-2-rosalie@mailbox.org`.
`b4 dig -a` showed only v1, no later revision. `b4 dig -w` showed Jiri
Kosina, Benjamin Tissoires, `linux-input`, and `linux-kernel` were
included. Full thread mbox shows Jiri replied: “Applied to
hid.git#for-7.1/upstream-fixes, thanks.” No NAKs, objections, explicit
stable request, or reviewer-suggested changes found. Direct WebFetch to
lore was blocked by Anubis, but `b4` fetched the thread successfully.
Phase 5, semantic analysis. Record: key functions are `sony_raw_event()`
and `nsg_mrxu_parse_report()`. `sony_raw_event()` is registered as the
HID driver `.raw_event` callback. HID core calls `.raw_event` from
`__hid_input_report()` before `hid_report_raw_event()`’s later report-
size validation, so driver-local parsing must validate `size` itself.
Lower HID layers such as USB HID call `hid_input_report()` with device-
provided lengths. `nsg_mrxu_parse_report()` is only called from
`sony_raw_event()` in this file. Similar pattern found: adjacent
upstream commit `12bd440b66ed8` adds a size check before another `rd[]`
access in the same function.
Phase 6, stable tree analysis. Record: the buggy NSG code exists in
checked stable branches `4.19.y`, `5.4.y`, `5.10.y`, `5.15.y`, `6.1.y`,
`6.6.y`, `6.12.y`, `6.18.y`, `6.19.y`, and `7.0.y`; `git merge-base
--is-ancestor b7289cb1cf99d4 <branch>` confirmed the introducing commit
is in each. Exact patch apply-check succeeded on `7.0.y`; older branches
failed exact context due file drift, but the required backport is a
trivial one-condition change at the same unchecked call site. No
existing stable branch log entry for this exact SMK-Link fix was found
in the checked branches.
Phase 7, subsystem context. Record: subsystem is HID input driver code,
specifically `drivers/hid/hid-sony.c`; criticality is driver-specific
but security-relevant for systems using or exposed to spoofed SMK-
Link/Sony HID devices. `MAINTAINERS` lists HID core as maintained by
Jiri Kosina and Benjamin Tissoires; Jiri committed and applied the
patch. The file has moderate recent activity, including HID Sony fixes
and device support work.
Phase 8, impact and risk. Record: affected users are `CONFIG_HID_SONY`
systems with SMK-Link NSG-MR5U/NSG-MR7U remote support, or systems that
accept a malicious/spoofed matching HID device. Trigger is a report with
ID byte `0x02` and length below 12. Failure mode is an out-of-bounds
read from the supplied report buffer; severity HIGH because it is
device-triggerable kernel memory-safety behavior. Benefit is high for
affected systems; risk is very low because valid reports of length >= 12
keep identical behavior and malformed short reports stop before unsafe
parsing.
Phase 9, synthesis. Evidence for backporting: real OOB read, malicious-
device trigger described in commit body, parser byte accesses verify the
needed bound, bug exists since v4.17-era code and is present in many
stable branches, fix is a one-line functional bounds check, maintainer
applied it to HID upstream fixes. Evidence against: no explicit `Tested-
by`, author says they do not own the hardware, and older stable trees
need a small context backport. Unresolved: no public crash report or
syzbot report found; no hardware test evidence found.
## Stable Rules
1. Obviously correct and tested: obviously correct by code inspection;
no explicit `Tested-by` and no hardware ownership, but maintainer
accepted it as an upstream fix.
2. Fixes a real bug: yes, verified unchecked parser reads up to
`rd[11]`.
3. Important issue: yes, device-triggerable kernel out-of-bounds read.
4. Small and contained: yes, one function in one driver.
5. No new features/APIs: yes.
6. Can apply to stable: yes for `7.0.y` directly; older stable branches
need trivial context adjustment.
No exception category is needed; this is a normal memory-safety fix.
## Verification
- Phase 1: parsed `git show a4170b63eda99`; confirmed tags and absence
of `Fixes:`, reports, review, stable Cc.
- Phase 2: inspected diff and `nsg_mrxu_parse_report()`; confirmed
`rd[11]` maximum access and added `size >= 12`.
- Phase 3: ran `git blame`, `git log -S`, `git show b7289cb1cf99d4`, and
author history checks.
- Phase 4: ran `b4 dig -c`, `-a`, `-w`, `b4 am`, and full `b4 mbox`;
confirmed v1-only thread and maintainer apply reply.
- Phase 5: used `rg` and `ReadFile` on HID core and `hid-sony`;
confirmed `.raw_event` call path and lower-layer `hid_input_report()`
entry.
- Phase 6: checked stable branch code, ancestry of `b7289cb1cf99d4`,
exact apply-checks, and stable logs for this subject.
- Phase 7: checked `MAINTAINERS` for HID maintainers and subsystem
ownership.
- Phase 8: mapped trigger and severity from verified parser accesses and
HID input path.
- Unverified: actual hardware testing on SMK-Link remotes; exact clean
application to older stable trees without a tiny backport adjustment.
**YES**
drivers/hid/hid-sony.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index a89af14e4acc6..02baaf84e9792 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -1035,10 +1035,9 @@ static int sony_raw_event(struct hid_device *hdev, struct hid_report *report,
sixaxis_parse_report(sc, rd, size);
} else if ((sc->quirks & MOTION_CONTROLLER_BT) && rd[0] == 0x01 && size == 49) {
sixaxis_parse_report(sc, rd, size);
- } else if ((sc->quirks & NAVIGATION_CONTROLLER) && rd[0] == 0x01 &&
- size == 49) {
+ } else if ((sc->quirks & NAVIGATION_CONTROLLER) && rd[0] == 0x01 && size == 49) {
sixaxis_parse_report(sc, rd, size);
- } else if ((sc->quirks & NSG_MRXU_REMOTE) && rd[0] == 0x02) {
+ } else if ((sc->quirks & NSG_MRXU_REMOTE) && rd[0] == 0x02 && size >= 12) {
nsg_mrxu_parse_report(sc, rd, size);
return 1;
} else if ((sc->quirks & RB4_GUITAR_PS4_USB) && rd[0] == 0x01 && size == 64) {
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH AUTOSEL 7.0-5.15] HID: ft260: validate i2c input report length
2026-05-20 11:18 [PATCH AUTOSEL 7.0-6.12] HID: logitech-hidpp: Add support for newer Bluetooth keyboards Sasha Levin
` (5 preceding siblings ...)
2026-05-20 11:19 ` [PATCH AUTOSEL 7.0-6.6] HID: sony: add missing size validation for SMK-Link remotes Sasha Levin
@ 2026-05-20 11:19 ` Sasha Levin
2026-05-20 11:57 ` sashiko-bot
6 siblings, 1 reply; 12+ messages in thread
From: Sasha Levin @ 2026-05-20 11:19 UTC (permalink / raw)
To: patches, stable
Cc: Michael Zaidman, Sebastián Josué Alba Vives,
Jiri Kosina, Sasha Levin, jikos, bentiss, linux-i2c, linux-input,
linux-kernel
From: Michael Zaidman <michael.zaidman@gmail.com>
[ Upstream commit 80c4bbb2b38513e9c3d84805fa61a0ee16d79c45 ]
Add two checks to ft260_raw_event() to prevent out-of-bounds reads
from malicious or malfunctioning devices:
First, reject reports shorter than the 2-byte header (report ID +
length fields). Without this, even accessing xfer->length on a
1-byte report is an OOB read.
Second, validate xfer->length against the actual data capacity of
the received HID report. Each I2C data report ID (0xD0 through
0xDE) defines a different report size in the HID descriptor, so the
available payload varies per report. A corrupted length field could
cause memcpy to read beyond the report buffer.
Reported-by: Sebastián Josué Alba Vives <sebasjosue84@gmail.com>
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Decision Summary
This should be backported. It is a small, single-driver HID fix for a
real out-of-bounds read in `ft260_raw_event()`, reachable from FT260 USB
HID input reports before HID core report-size validation runs. The
benefit is high because the failure mode is memory-safety/security
relevant; the regression risk is low because valid reports are unchanged
and only malformed reports are rejected.
## Phase Walkthrough
### Phase 1: Commit Message Forensics
Record: subsystem `HID: ft260`; action verb `validate`; intent is to
validate I2C input report length.
Record: tags in commit `80c4bbb2b38513e9c3d84805fa61a0ee16d79c45`:
`Reported-by: Sebastián Josué Alba Vives <sebasjosue84@gmail.com>`,
`Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>`, `Signed-
off-by: Jiri Kosina <jkosina@suse.com>`. No `Fixes:`, `Link:`, `Cc:
stable`, `Tested-by`, `Reviewed-by`, or `Acked-by` in the committed
message.
Record: bug description is explicit: malicious or malfunctioning
FT260-compatible HID devices can send too-short reports or corrupted
length fields, causing OOB reads. Symptom/failure mode is OOB heap read
before/inside `memcpy()`. No affected version listed in commit body.
Record: this is not a hidden fix; it is directly described as a memory-
safety fix.
### Phase 2: Diff Analysis
Record: one file changed, `drivers/hid/hid-ft260.c`, `14` insertions and
`2` deletions. Only `ft260_raw_event()` is modified. Scope is single-
file surgical.
Record: before, `ft260_raw_event()` cast `data` to `struct
ft260_i2c_input_report` and accessed `xfer->report`/`xfer->length`
without checking `size`; then `memcpy()` used `xfer->length` after
checking only destination capacity. After, it rejects reports shorter
than the 2-byte header and rejects `xfer->length > size - offsetof(...,
data)` before copying.
Record: bug category is memory safety: OOB read on truncated input and
OOB source read on corrupted length. No locking, API, or feature
behavior changes.
Record: fix quality is strong: minimal, obvious bounds checks
immediately before the unsafe accesses/copy. Regression risk is low;
only malformed reports are rejected. The use of actual `size` avoids the
earlier reviewed problem of rejecting valid short FT260 report IDs.
### Phase 3: Git History Investigation
Record: `git blame` shows the vulnerable header access and `memcpy()`
came from `6a82582d9fa4` (`HID: ft260: add usb hid to i2c host bridge
driver`, first contained around `v5.13-rc1`). The destination-buffer
check came from `b7121e3c04440` (`HID: ft260: skip unexpected HID input
reports`, first contained around `v6.2-rc1`).
Record: no `Fixes:` tag, so Step 3.2 is not applicable.
Record: recent file history shows related FT260 fixes and enhancements,
including `b7121e3c04440`, but no prerequisite needed for the new
source-length/header validation. For older `5.15.y`/`6.1.y`, the exact
patch context does not apply because `b7121e3c04440` is absent, but the
fix itself is still conceptually standalone.
Record: author Michael Zaidman is listed in `MAINTAINERS` as FT260
maintainer and has a long FT260 commit history. Jiri Kosina committed
the patch and is listed as HID core maintainer.
### Phase 4: Mailing List And External Research
Record: `b4 dig -c 80c4bbb2b3851` found the matching submission at `http
s://patch.msgid.link/20260411062437.279838-1-michael.zaidman@gmail.com`.
Record: `b4 dig -a` showed prior versions/discussion. The original
reporter’s v1/v2 were Cc’d to stable and had a too-broad `size < 64`
approach. Review identified that valid FT260 reports can be shorter,
leading to Michael’s final per-driver fix using the header size and
actual report size.
Record: `b4 dig -w` showed Michael Zaidman, Jiri Kosina, Benjamin
Tissoires, linux-input, linux-kernel, and the reporter on the final
submission.
Record: saved lore mbox shows Jiri applied the final patch to
`hid.git#for-7.1/upstream-fixes`. The thread also contains Michael’s
hardware test note: tested on FT260 with I2C-attached EEPROM behind
PCA9548 muxes, including small and larger reads.
Record: stable-specific history exists in the earlier reporter
submissions, including `Cc: stable@vger.kernel.org` and explicit
OOB/security rationale. No NAK remained against the final version.
### Phase 5: Code Semantic Analysis
Record: modified function is `ft260_raw_event()`.
Record: callers traced manually: `ft260_raw_event()` is registered as
`.raw_event`; HID core calls `hdrv->raw_event()` in
`__hid_input_report()` before `hid_report_raw_event()` performs report-
size checks. USB HID paths call `hid_input_report()` from
interrupt/control completion paths.
Record: callees/side effects: `ft260_raw_event()` copies HID I2C
response payload into `dev->read_buf`, advances `read_idx`, and
completes the waiting I2C read when enough data arrives.
Record: reachability: normal FT260 I2C reads go through `i2c_adapter`
operations (`ft260_i2c_xfer()` / `ft260_smbus_xfer()` ->
`ft260_i2c_read()`), while the malformed input itself is supplied by the
USB HID device. Userspace reachability depends on system I2C access
permissions; unprivileged trigger was not verified.
Record: similar pattern in this function is unique for the FT260 I2C
input report copy. Related HID OOB fixes exist in recent HID history,
supporting the general risk class but not required for this patch.
### Phase 6: Stable Tree Analysis
Record: `drivers/hid/hid-ft260.c` exists in `stable/linux-5.15.y`,
`6.1.y`, `6.6.y`, `6.12.y`, and current `v7.0.9` checkout; it does not
exist in `4.19.y`, `5.4.y`, or `5.10.y`.
Record: the buggy code pattern exists in `5.15.y`, `6.1.y`, `6.6.y`, and
`6.12.y`; none contain `80c4bbb2b3851`.
Record: patch applies cleanly to `6.6.y`, `6.12.y`, and current `v7.0.9`
using a temporary index. It does not apply unchanged to `5.15.y` or
`6.1.y` because those lack `b7121e3c04440` context, so older branches
need a small manual backport.
Record: no alternate stable fix for this exact validation was found.
### Phase 7: Subsystem And Maintainer Context
Record: subsystem is HID driver for an FTDI USB HID-to-I2C bridge.
Criticality is driver-specific but security-relevant for systems using
that hardware or exposed USB ports.
Record: subsystem/file activity is active; recent HID history includes
several OOB fixes, and FT260 has ongoing maintenance by Michael Zaidman.
### Phase 8: Impact And Risk
Record: affected population is driver/config/hardware-specific: systems
with `CONFIG_HID_FT260` and FT260 or malicious FT260-emulating USB HID
devices.
Record: trigger is a malformed HID input report from the device,
especially during/around I2C read handling. Physical or device-level
control is required; unprivileged local trigger was not verified.
Record: failure mode is OOB heap read, with reporter discussion noting
possible data exposure through the I2C read buffer. Severity: HIGH,
security-relevant memory safety.
Record: benefit is high for affected systems; risk is low because the
patch is small, contained, and only rejects malformed reports.
### Phase 9: Final Synthesis
Evidence for backporting: real OOB read; malicious/malfunctioning device
trigger; HID raw_event is called before HID core size validation;
maintainer-authored final fix; hardware test notes; one function and one
file; no new API or feature; applies cleanly to current 7.0 and newer
stable branches.
Evidence against/backport concerns: older `5.15.y` and `6.1.y` need
manual context adjustment; `4.19.y`, `5.4.y`, and `5.10.y` lack the
driver. No `Fixes:` tag, but that is not a negative signal here.
Stable rules checklist: obviously correct and tested: yes, by code
inspection and submission test notes. Fixes a real bug: yes, OOB reads.
Important issue: yes, memory safety/security. Small and contained: yes,
`14+/2-` in one function. No new features/APIs: yes. Can apply to
stable: yes for 6.6/6.12/7.0; minor backport needed for 5.15/6.1.
Exception categories: none; this is a direct memory-safety bug fix, not
a device ID/quirk/build/doc exception.
## Verification
- [Phase 1] `git show --format=fuller --no-patch 80c4bbb2b3851`:
confirmed subject, message, author/committer, and tags.
- [Phase 2] `git show --function-context 80c4bbb2b3851 --
drivers/hid/hid-ft260.c`: confirmed only `ft260_raw_event()` changes
and exact bounds checks.
- [Phase 3] `git blame -L 1065,1085 80c4bbb2b3851^ -- drivers/hid/hid-
ft260.c`: confirmed vulnerable lines originated in `6a82582d9fa4` and
related destination check in `b7121e3c04440`.
- [Phase 3] `git show` on `6a82582d9fa4` and `b7121e3c04440`: confirmed
driver introduction and related unexpected-report fix.
- [Phase 4] `b4 dig -c`, `-a`, `-w`, and `-m`: found final lore thread,
revisions, recipients, maintainer apply message, and test notes.
- [Phase 4] WebFetch/Spinics: confirmed earlier stable-cc’d reporter
patches and review discussion about avoiding a blanket 64-byte check.
- [Phase 5] Read `drivers/hid/hid-core.c`: confirmed `raw_event` runs
before `hid_report_raw_event()` size validation.
- [Phase 5] Searched FT260 call paths: confirmed registration as
`.raw_event`, USB HID input path, and I2C adapter read paths.
- [Phase 6] `git ls-tree` and `git grep` against stable branches:
confirmed affected code exists in `5.15.y+` branches checked, absent
from older listed branches.
- [Phase 6] Temporary-index `git apply --check`: clean for `6.6.y`,
`6.12.y`, and current `v7.0.9`; fails unchanged for `5.15.y`/`6.1.y`,
requiring minor backport.
- [Phase 7] `MAINTAINERS` search: confirmed Michael Zaidman maintains
FT260 and Jiri/Benjamin maintain HID core.
- UNVERIFIED: whether an unprivileged local user can trigger this on a
given distro; that depends on USB access, I2C device permissions, and
deployment policy.
**YES**
drivers/hid/hid-ft260.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 333341e80b0ec..70e2eedb465af 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -1068,10 +1068,22 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
struct ft260_device *dev = hid_get_drvdata(hdev);
struct ft260_i2c_input_report *xfer = (void *)data;
+ if (size < offsetof(struct ft260_i2c_input_report, data)) {
+ hid_err(hdev, "short report %d\n", size);
+ return -1;
+ }
+
if (xfer->report >= FT260_I2C_REPORT_MIN &&
xfer->report <= FT260_I2C_REPORT_MAX) {
- ft260_dbg("i2c resp: rep %#02x len %d\n", xfer->report,
- xfer->length);
+ ft260_dbg("i2c resp: rep %#02x len %d size %d\n",
+ xfer->report, xfer->length, size);
+
+ if (xfer->length > size -
+ offsetof(struct ft260_i2c_input_report, data)) {
+ hid_err(hdev, "report %#02x: length %d exceeds HID report size\n",
+ xfer->report, xfer->length);
+ return -1;
+ }
if ((dev->read_buf == NULL) ||
(xfer->length > dev->read_len - dev->read_idx)) {
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH AUTOSEL 7.0-5.15] HID: ft260: validate i2c input report length
2026-05-20 11:19 ` [PATCH AUTOSEL 7.0-5.15] HID: ft260: validate i2c input report length Sasha Levin
@ 2026-05-20 11:57 ` sashiko-bot
0 siblings, 0 replies; 12+ messages in thread
From: sashiko-bot @ 2026-05-20 11:57 UTC (permalink / raw)
To: Sasha Levin; +Cc: dmitry.torokhov, linux-input
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Pre-existing issues:
- [High] memcpy reads beyond the bounds of the statically sized array 'data[2]', which will trigger a FORTIFY_SOURCE panic for payloads > 2 bytes.
- [High] A race condition in the probe function leads to a NULL pointer dereference in `ft260_raw_event` if an input report is received early.
- [High] A race condition exists between the HID raw event handler and the I2C read timeout path, leading to a Use-After-Free or kernel stack corruption.
- [High] The probe function incorrectly returns 0 (success) when intentionally rejecting the UART interface, rendering it unusable.
--
commit 4ff190a6df13fab19cc5a93ba01ca6de278d20bd
Author: Michael Zaidman <michael.zaidman@gmail.com>
HID: ft260: validate i2c input report length
This commit adds checks to ft260_raw_event to prevent out-of-bounds reads
from malicious or malfunctioning devices. It rejects reports shorter than the
2-byte header and validates the length field against the actual HID report
capacity.
> diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
> index 333341e80b0ec..70e2eedb465af 100644
> --- a/drivers/hid/hid-ft260.c
> +++ b/drivers/hid/hid-ft260.c
> @@ -1068,10 +1068,22 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
> struct ft260_device *dev = hid_get_drvdata(hdev);
[Severity: High]
This is a pre-existing issue, but can this result in a NULL pointer
dereference during device initialization?
If hid_hw_start() is called in ft260_probe() before hid_set_drvdata() is
initialized, the device might send an input report immediately (e.g., from
a pending interrupt). This would trigger ft260_raw_event(), leaving dev as
NULL and causing a panic when dev->read_buf is accessed below.
> struct ft260_i2c_input_report *xfer = (void *)data;
>
> + if (size < offsetof(struct ft260_i2c_input_report, data)) {
> + hid_err(hdev, "short report %d\n", size);
> + return -1;
> + }
> +
> if (xfer->report >= FT260_I2C_REPORT_MIN &&
> xfer->report <= FT260_I2C_REPORT_MAX) {
> - ft260_dbg("i2c resp: rep %#02x len %d\n", xfer->report,
> - xfer->length);
> + ft260_dbg("i2c resp: rep %#02x len %d size %d\n",
> + xfer->report, xfer->length, size);
> +
> + if (xfer->length > size -
> + offsetof(struct ft260_i2c_input_report, data)) {
> + hid_err(hdev, "report %#02x: length %d exceeds HID report size\n",
> + xfer->report, xfer->length);
> + return -1;
> + }
>
> if ((dev->read_buf == NULL) ||
[Severity: High]
This is a pre-existing issue, but does this check safely prevent a
use-after-free or stack corruption?
In ft260_i2c_read(), the driver blocks on wait_for_completion_timeout(). If a
timeout occurs, it sets dev->read_buf = NULL and returns, destroying the
caller-provided buffer which may be a stack frame.
If ft260_raw_event() checks dev->read_buf != NULL and is preempted while the
timeout path returns, wouldn't the subsequent memcpy() write hardware payload
data into freed memory or a popped stack frame?
> (xfer->length > dev->read_len - dev->read_idx)) {
There is another pre-existing issue right below the diff in ft260_raw_event():
drivers/hid/hid-ft260.c:ft260_raw_event() {
[ ... ]
if ((dev->read_buf == NULL) ||
(xfer->length > dev->read_len - dev->read_idx)) {
hid_err(hdev, "unexpected report %#02x, length %d\n",
xfer->report, xfer->length);
return -1;
}
memcpy(&dev->read_buf[dev->read_idx], &xfer->data,
xfer->length);
[ ... ]
}
[Severity: High]
This isn't a bug introduced by this patch, but will this memcpy() trigger a
FORTIFY_SOURCE panic for payloads larger than 2 bytes?
The ft260_i2c_input_report structure defines the data payload as a fixed-size
array:
struct ft260_i2c_input_report {
u8 report;
u8 length;
u8 data[2];
} __packed;
If xfer->length is greater than 2, the compiler statically knows the source
object size is exactly 2 bytes. Under CONFIG_FORTIFY_SOURCE=y, wouldn't this
trigger a mandatory kernel panic at runtime for a buffer overread? Could the
data field be changed to a flexible array member?
There is also a pre-existing issue in the probe function:
drivers/hid/hid-ft260.c:ft260_probe() {
[ ... ]
ret = ft260_is_interface_enabled(hdev);
if (ret <= 0)
goto err_hid_close;
[ ... ]
err_hid_close:
hid_hw_close(hdev);
err_hid_stop:
hid_hw_stop(hdev);
return ret;
}
[Severity: High]
This isn't a bug introduced by this patch, but does the probe function
incorrectly handle a rejected UART interface?
When ft260_is_interface_enabled() encounters the UART interface, it leaves
the return code as 0. The driver jumps to the error path, stops the hardware,
and returns 0 instead of an error code like -ENODEV.
Returning 0 informs the kernel driver core that hid-ft260 successfully bound
to the device interface. Doesn't this hijack the UART interface and leave it
in a disabled state, permanently preventing generic HID drivers (like
hid-generic) from claiming it?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260520111944.3424570-54-sashal@kernel.org?part=1
^ permalink raw reply [flat|nested] 12+ messages in thread