public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Jian Zhang <zhangjian.3032@bytedance.com>,
	Jakub Kicinski <kuba@kernel.org>, Sasha Levin <sashal@kernel.org>,
	jk@codeconstruct.com.au, matt@codeconstruct.com.au,
	netdev@vger.kernel.org
Subject: [PATCH AUTOSEL 6.19-6.1] net: mctp-i2c: fix duplicate reception of old data
Date: Sat, 14 Feb 2026 16:22:33 -0500	[thread overview]
Message-ID: <20260214212452.782265-8-sashal@kernel.org> (raw)
In-Reply-To: <20260214212452.782265-1-sashal@kernel.org>

From: Jian Zhang <zhangjian.3032@bytedance.com>

[ 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 <zhangjian.3032@bytedance.com>
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**
```c
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`**
```c
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


  parent reply	other threads:[~2026-02-14 21:25 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260214212452.782265-1-sashal@kernel.org>
2026-02-14 21:22 ` [PATCH AUTOSEL 6.19-5.10] myri10ge: avoid uninitialized variable use Sasha Levin
2026-02-14 21:22 ` Sasha Levin [this message]
2026-02-14 21:22 ` [PATCH AUTOSEL 6.19-6.12] net: wwan: mhi: Add network support for Foxconn T99W760 Sasha Levin
2026-02-14 21:22 ` [PATCH AUTOSEL 6.19-5.10] net/rds: Clear reconnect pending bit Sasha Levin
2026-02-14 21:22 ` [PATCH AUTOSEL 6.19-6.12] ipv6: annotate data-races over sysctl.flowlabel_reflect Sasha Levin
2026-02-14 21:22 ` [PATCH AUTOSEL 6.19-5.15] ipv6: exthdrs: annotate data-race over multiple sysctl Sasha Levin
2026-02-14 21:23 ` [PATCH AUTOSEL 6.19-5.10] octeontx2-af: Workaround SQM/PSE stalls by disabling sticky Sasha Levin
2026-02-14 21:23 ` [PATCH AUTOSEL 6.19-5.10] vmw_vsock: bypass false-positive Wnonnull warning with gcc-16 Sasha Levin
2026-02-14 21:23 ` [PATCH AUTOSEL 6.19-5.15] ipv6: annotate data-races in ip6_multipath_hash_{policy,fields}() Sasha Levin
2026-02-14 21:23 ` [PATCH AUTOSEL 6.19-6.6] ipv4: igmp: annotate data-races around idev->mr_maxdelay Sasha Levin
2026-02-14 21:23 ` [PATCH AUTOSEL 6.19-5.10] net/rds: No shortcut out of RDS_CONN_ERROR Sasha Levin
2026-02-14 21:23 ` [PATCH AUTOSEL 6.19-6.18] ipv6: annotate data-races in net/ipv6/route.c Sasha Levin
2026-02-14 21:23 ` [PATCH AUTOSEL 6.19-6.12] bnxt_en: Allow ntuple filters for drops Sasha Levin
2026-02-14 21:23 ` [PATCH AUTOSEL 6.19-6.18] ptp: ptp_vmclock: add 'VMCLOCK' to ACPI device match Sasha Levin
2026-02-14 21:23 ` [PATCH AUTOSEL 6.19-5.10] ipv4: fib: Annotate access to struct fib_alias.fa_state Sasha Levin
2026-02-14 21:23 ` [PATCH AUTOSEL 6.19-6.12] net: sfp: add quirk for Lantech 8330-265D Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260214212452.782265-8-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=jk@codeconstruct.com.au \
    --cc=kuba@kernel.org \
    --cc=matt@codeconstruct.com.au \
    --cc=netdev@vger.kernel.org \
    --cc=patches@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    --cc=zhangjian.3032@bytedance.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox