Thread (1 message) flat view 1 message, 1 author, 2026-02-14
STALE173d

[PATCH AUTOSEL 6.19-6.1] net: mctp-i2c: fix duplicate reception of old data

From: Sasha Levin <sashal@kernel.org>
Date: 2026-02-14 21:25:09
Also in: linux-patches, stable
Subsystem: management component transport protocol (mctp), networking drivers, the rest · Maintainers: Jeremy Kerr, Matt Johnston, Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds

From: Jian Zhang <redacted>

[ Upstream commit ae4744e173fadd092c43eda4ca92dcb74645225a ]

The MCTP I2C slave callback did not handle I2C_SLAVE_READ_REQUESTED
events. As a result, i2c read event will trigger repeated reception of
old data, reset rx_pos when a read request is received.

Signed-off-by: Jian Zhang <redacted>
Link: https://patch.msgid.link/20260108101829.1140448-1-zhangjian.3032@bytedance.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Analysis of net: mctp-i2c: fix duplicate reception of old data

### 1. Commit Message Analysis

The commit message clearly describes a **bug fix**: the MCTP I2C slave
callback was not handling `I2C_SLAVE_READ_REQUESTED` events, which
caused **duplicate reception of old data** when an I2C read event
occurred. The fix resets `rx_pos` when a read request is received.

Keywords: "fix duplicate reception of old data" — this is an explicit
bug fix.

### 2. Code Change Analysis

The patch makes two changes:

**Change 1: Handle `I2C_SLAVE_READ_REQUESTED` in the switch statement**
case I2C_SLAVE_READ_REQUESTED:
    midev->rx_pos = 0;
    break;
This adds handling for a previously unhandled I2C slave event. When a
read request comes in, `rx_pos` is reset to 0, preventing stale data
from being re-processed. Without this, the `rx_pos` would retain its old
value from a previous transaction, and when `I2C_SLAVE_STOP` fires,
`mctp_i2c_recv()` would process stale data in the buffer — resulting in
duplicate/ghost packet reception.

**Change 2: Early return in `mctp_i2c_recv()` when `rx_pos == 0`**
if (midev->rx_pos == 0)
    return 0;
This is a defensive guard: if `rx_pos` was reset to 0 (by the new
READ_REQUESTED handler), `mctp_i2c_recv()` should do nothing since
there's no valid write data to process. Without this check, the function
would proceed with invalid state (`rx_pos == 0`), potentially causing
incorrect length calculations or accessing uninitialized buffer data.

### 3. Bug Mechanism

The I2C slave callback handles events during I2C bus transactions. The
MCTP-over-I2C protocol involves the bus master writing MCTP packets.
However, the I2C bus can also generate `I2C_SLAVE_READ_REQUESTED` events
(when the master reads from this slave). Without handling this event:

1. `rx_pos` retains its value from a previous write transaction
2. When `I2C_SLAVE_STOP` fires after the read, `mctp_i2c_recv()`
   processes the stale buffer
3. This causes **duplicate reception of old MCTP packets** — a data
   integrity/correctness bug

### 4. Classification

- **Bug fix**: Yes — fixes incorrect behavior (duplicate packet
  reception)
- **New feature**: No — this handles an existing I2C event that was
  previously ignored
- **Scope**: Very small — adds 5 lines of code across two locations in
  one file

### 5. Stable Criteria Assessment

| Criterion | Assessment |
|-----------|------------|
| Obviously correct and tested | Yes — simple, logical fix; accepted by
net maintainer Jakub Kicinski |
| Fixes a real bug | Yes — duplicate reception of stale MCTP data |
| Important issue | Moderate — data correctness issue in networking
stack |
| Small and contained | Yes — 5 lines added in a single file |
| No new features | Correct — just handles a missing event case |
| Applies cleanly | Likely — small, localized change |

### 6. Risk Assessment

**Risk: Very low**
- The change is minimal and purely additive (no existing code modified)
- Adding a case to a switch statement with a simple assignment is very
  safe
- The early return guard in `mctp_i2c_recv()` is a standard defensive
  check
- Only affects MCTP-over-I2C users — very narrow blast radius

**Benefit: Moderate to High for affected users**
- MCTP (Management Component Transport Protocol) over I2C is used in
  server/BMC management
- Duplicate packet reception causes incorrect behavior in management
  stacks
- Without this fix, any I2C read event corrupts the MCTP receive path

### 7. Dependencies

No dependencies on other commits. The fix is self-contained and modifies
only the existing `mctp_i2c_slave_cb()` function and `mctp_i2c_recv()`
function.

### 8. Conclusion

This is a clean, small, obviously correct bug fix that addresses a real
data corruption/correctness issue (duplicate reception of stale data) in
the MCTP I2C driver. It meets all stable kernel criteria: it's small,
contained, fixes a real bug, introduces no new features, and has very
low regression risk.

**YES**

 drivers/net/mctp/mctp-i2c.c | 6 ++++++
 1 file changed, 6 insertions(+)
diff --git a/drivers/net/mctp/mctp-i2c.c b/drivers/net/mctp/mctp-i2c.c
index f782d93f826ef..ecda1cc36391c 100644
--- a/drivers/net/mctp/mctp-i2c.c
+++ b/drivers/net/mctp/mctp-i2c.c
@@ -242,6 +242,9 @@ static int mctp_i2c_slave_cb(struct i2c_client *client,
 		return 0;
 
 	switch (event) {
+	case I2C_SLAVE_READ_REQUESTED:
+		midev->rx_pos = 0;
+		break;
 	case I2C_SLAVE_WRITE_RECEIVED:
 		if (midev->rx_pos < MCTP_I2C_BUFSZ) {
 			midev->rx_buffer[midev->rx_pos] = *val;
@@ -279,6 +282,9 @@ static int mctp_i2c_recv(struct mctp_i2c_dev *midev)
 	size_t recvlen;
 	int status;
 
+	if (midev->rx_pos == 0)
+		return 0;
+
 	/* + 1 for the PEC */
 	if (midev->rx_pos < MCTP_I2C_MINLEN + 1) {
 		ndev->stats.rx_length_errors++;
-- 
2.51.0
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help