From: Arnaud Lecomte contact@arnaud-lcm.com
[ Upstream commit b56cc41a3ae7323aa3c6165f93c32e020538b6d2 ]
As reported by syzbot, mcp2221_raw_event lacked validation of incoming I2C read data sizes, risking buffer overflows in mcp->rxbuf during multi-part transfers. As highlighted in the DS20005565B spec, p44, we have: "The number of read-back data bytes to follow in this packet: from 0 to a maximum of 60 bytes of read-back bytes." This patch enforces we don't exceed this limit.
Reported-by: syzbot+52c1a7d3e5b361ccd346@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=52c1a7d3e5b361ccd346 Tested-by: syzbot+52c1a7d3e5b361ccd346@syzkaller.appspotmail.com Signed-off-by: Arnaud Lecomte contact@arnaud-lcm.com Link: https://patch.msgid.link/20250726220931.7126-1-contact@arnaud-lcm.com Signed-off-by: Benjamin Tissoires bentiss@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
**Backport Status: YES**
This commit should definitely be backported to stable kernel trees. Here's my extensive analysis:
## Security Impact Analysis
This is a **critical security vulnerability** - a buffer overflow in the HID MCP2221 driver that was discovered and reported by syzbot. The vulnerability occurs in `mcp2221_raw_event()` at line 820 (post-patch) where the driver performs:
```c memcpy(&buf[mcp->rxbuf_idx], &data[4], data[3]); ```
Without the fix, `data[3]` (which comes from untrusted USB HID input) is used directly as the copy length without validation. According to the hardware specification (DS20005565B, page 44), this value should never exceed 60 bytes, but the driver wasn't enforcing this limit.
## Vulnerability Details
1. **Attack Vector**: A malicious USB device or compromised MCP2221 device could send crafted HID reports with `data[3]` > 60, causing: - Buffer overflow in `mcp->rxbuf` - Potential arbitrary memory corruption - Possible privilege escalation or code execution
2. **The Fix**: The patch adds critical bounds checking: ```c if (!mcp->rxbuf || mcp->rxbuf_idx < 0 || data[3] > 60) { mcp->status = -EINVAL; break; } ```
This ensures: - `mcp->rxbuf` is not NULL - `mcp->rxbuf_idx` is not negative (preventing underflow) - `data[3]` doesn't exceed the hardware's maximum of 60 bytes
## Stable Backport Criteria Assessment
✅ **Fixes a real bug affecting users**: Yes - security vulnerability with potential for system compromise ✅ **Small and contained fix**: Yes - only 4 lines added, single validation check ✅ **No major side effects**: The fix only adds validation, doesn't change functionality ✅ **No architectural changes**: Simple bounds checking addition ✅ **Critical subsystem impact**: HID subsystem, but localized to one driver ✅ **Already marked for stable**: The commit shows `[ Upstream commit b56cc41a3ae7323aa3c6165f93c32e020538b6d2 ]` indicating it's already been selected ✅ **Follows stable rules**: Critical security fix with minimal regression risk ✅ **Tested by syzbot**: The fix was validated by the same fuzzer that found the issue
## Additional Context
- The vulnerability was found through systematic fuzzing (syzbot), indicating it's reachable through normal USB HID operations - The MCP2221 is a USB-to-I2C/UART converter chip commonly used in embedded systems and development boards - Without this fix, any system with an MCP2221 device (or emulated device) is vulnerable to memory corruption attacks - The fix is already upstream (commit b56cc41a3ae7323aa3c6165f93c32e020538b6d2) and has been tested
This is a textbook example of what should be backported to stable: a simple, well-tested security fix that prevents a serious vulnerability without introducing new features or complexity.
drivers/hid/hid-mcp2221.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c index 6c0ac14f11a6a..2cfc8e1a2912d 100644 --- a/drivers/hid/hid-mcp2221.c +++ b/drivers/hid/hid-mcp2221.c @@ -816,6 +816,10 @@ static int mcp2221_raw_event(struct hid_device *hdev, } if (data[2] == MCP2221_I2C_READ_COMPL || data[2] == MCP2221_I2C_READ_PARTIAL) { + if (!mcp->rxbuf || mcp->rxbuf_idx < 0 || data[3] > 60) { + mcp->status = -EINVAL; + break; + } buf = mcp->rxbuf; memcpy(&buf[mcp->rxbuf_idx], &data[4], data[3]); mcp->rxbuf_idx = mcp->rxbuf_idx + data[3];