public inbox for netfilter-devel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH AUTOSEL 6.19-5.10] netfilter: nf_conntrack: Add allow_clash to generic protocol handler
       [not found] <20260214212452.782265-1-sashal@kernel.org>
@ 2026-02-14 21:22 ` Sasha Levin
  2026-02-14 21:23 ` [PATCH AUTOSEL 6.19-5.10] netfilter: xt_tcpmss: check remaining length before reading optlen Sasha Levin
  1 sibling, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2026-02-14 21:22 UTC (permalink / raw)
  To: patches, stable
  Cc: Yuto Hamaguchi, Florian Westphal, Sasha Levin, pablo,
	netfilter-devel, coreteam

From: Yuto Hamaguchi <Hamaguchi.Yuto@da.MitsubishiElectric.co.jp>

[ Upstream commit 8a49fc8d8a3e83dc51ec05bcd4007bdea3c56eec ]

The upstream commit, 71d8c47fc653711c41bc3282e5b0e605b3727956
 ("netfilter: conntrack: introduce clash resolution on insertion race"),
sets allow_clash=true in the UDP/UDPLITE protocol handler
but does not set it in the generic protocol handler.

As a result, packets composed of connectionless protocols at each layer,
such as UDP over IP-in-IP, still drop packets due to conflicts during conntrack insertion.

To resolve this, this patch sets allow_clash in the nf_conntrack_l4proto_generic.

Signed-off-by: Yuto Hamaguchi <Hamaguchi.Yuto@da.MitsubishiElectric.co.jp>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

The current file does **not** yet have `allow_clash = true` — this
confirms the patch hasn't been applied yet and is needed.

### 7. STABILITY AND TRUST INDICATORS

- **Signed-off by Florian Westphal** — the netfilter maintainer,
  indicating proper review
- The fix is trivially verifiable: it mirrors what was already done for
  UDP/UDPLITE/GRE
- The author (Yuto Hamaguchi from Mitsubishi Electric) clearly
  encountered this in a real-world deployment scenario

### Summary

| Criterion | Assessment |
|-----------|-----------|
| **Fixes a real bug** | YES — packets dropped for connectionless
protocol stacks |
| **Obviously correct** | YES — mirrors existing pattern for
UDP/UDPLITE/GRE |
| **Small and contained** | YES — single line addition |
| **No new features** | Correct — just extends existing clash resolution
to generic handler |
| **Risk of regression** | Very low — same mechanism already proven for
UDP |
| **User impact** | Moderate-high — affects any setup with
connectionless tunnels + conntrack |
| **Dependencies** | Requires 71d8c47fc653 which is already in mainline
|

This is a textbook stable backport candidate: a one-line fix that
addresses a real user-visible bug (packet drops) with essentially zero
regression risk, fixing an oversight in an earlier commit that only
partially applied clash resolution to all relevant protocol handlers.

**YES**

 net/netfilter/nf_conntrack_proto_generic.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/netfilter/nf_conntrack_proto_generic.c b/net/netfilter/nf_conntrack_proto_generic.c
index e831637bc8ca8..cb260eb3d012c 100644
--- a/net/netfilter/nf_conntrack_proto_generic.c
+++ b/net/netfilter/nf_conntrack_proto_generic.c
@@ -67,6 +67,7 @@ void nf_conntrack_generic_init_net(struct net *net)
 const struct nf_conntrack_l4proto nf_conntrack_l4proto_generic =
 {
 	.l4proto		= 255,
+	.allow_clash            = true,
 #ifdef CONFIG_NF_CONNTRACK_TIMEOUT
 	.ctnl_timeout		= {
 		.nlattr_to_obj	= generic_timeout_nlattr_to_obj,
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* [PATCH AUTOSEL 6.19-5.10] netfilter: xt_tcpmss: check remaining length before reading optlen
       [not found] <20260214212452.782265-1-sashal@kernel.org>
  2026-02-14 21:22 ` [PATCH AUTOSEL 6.19-5.10] netfilter: nf_conntrack: Add allow_clash to generic protocol handler Sasha Levin
@ 2026-02-14 21:23 ` Sasha Levin
  1 sibling, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2026-02-14 21:23 UTC (permalink / raw)
  To: patches, stable
  Cc: Florian Westphal, sungzii, Sasha Levin, pablo, netfilter-devel,
	coreteam

From: Florian Westphal <fw@strlen.de>

[ Upstream commit 735ee8582da3d239eb0c7a53adca61b79fb228b3 ]

Quoting reporter:
  In net/netfilter/xt_tcpmss.c (lines 53-68), the TCP option parser reads
 op[i+1] directly without validating the remaining option length.

  If the last byte of the option field is not EOL/NOP (0/1), the code attempts
  to index op[i+1]. In the case where i + 1 == optlen, this causes an
  out-of-bounds read, accessing memory past the optlen boundary
  (either reading beyond the stack buffer _opt or the
  following payload).

Reported-by: sungzii <sungzii@pm.me>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Analysis of netfilter: xt_tcpmss: check remaining length before
reading optlen

### 1. COMMIT MESSAGE ANALYSIS

The commit message is clear and detailed. The reporter (sungzii)
explains the exact bug mechanism:
- In the TCP option parser in `xt_tcpmss.c`, when iterating through TCP
  options, the code reads `op[i+1]` without first checking that `i+1` is
  within bounds.
- If the last byte of the option field is not EOL/NOP (value 0 or 1),
  the code falls through to the `else` branch and reads `op[i+1]`, which
  is an **out-of-bounds read** when `i + 1 == optlen`.

Key indicators:
- **"Reported-by: sungzii"** - a user found and reported this bug
- **Out-of-bounds read** - a real memory safety bug
- **Netfilter subsystem** - network-facing, security-sensitive code that
  processes packets

### 2. CODE CHANGE ANALYSIS

The fix is a single-line change:

```c
- if (op[i] < 2)
+               if (op[i] < 2 || i == optlen - 1)
```

**What was wrong:** The TCP option parsing loop iterates through option
bytes. Options with values 0 (EOL) and 1 (NOP) are single-byte. All
other options use a TLV (type-length-value) format where `op[i+1]`
contains the length. The code checks `if (op[i] < 2)` to handle single-
byte options, and otherwise reads `op[i+1]` for the length. But if `i`
is the last valid index (`i == optlen - 1`), and the option byte is >=
2, the code reads `op[i+1]` which is **past the end of the buffer**.

**The fix:** Before reading `op[i+1]`, the code now also checks if `i`
is at the last byte (`i == optlen - 1`). If so, it treats it as a
single-byte increment (just `i++`), which will terminate the loop on the
next iteration since `i` will then equal `optlen`.

This is:
- **Obviously correct** - if there's only one byte left, we can't read a
  2-byte option header
- **Minimal** - one condition added to an existing check
- **Safe** - the worst case is we skip a malformed trailing option byte,
  which is the right behavior

### 3. CLASSIFICATION

- **Bug type:** Out-of-bounds read (memory safety)
- **Security relevance:** HIGH - this is in the netfilter packet
  processing path. An attacker could craft TCP packets with malformed
  options to trigger the OOB read. This could leak kernel memory
  contents or trigger a crash depending on the memory layout.
- **Category:** Bounds validation fix

### 4. SCOPE AND RISK ASSESSMENT

- **Lines changed:** 1 line modified
- **Files changed:** 1 file (`net/netfilter/xt_tcpmss.c`)
- **Risk of regression:** Extremely low. The added condition only
  activates when `i == optlen - 1`, meaning the last byte of the
  options. In that case, incrementing by 1 exits the loop. This cannot
  break valid TCP option parsing because valid multi-byte options always
  have at least 2 bytes.
- **Subsystem:** Netfilter - core networking/firewall code used by
  virtually all Linux deployments

### 5. USER IMPACT

- **Who is affected:** Anyone using the `tcpmss` iptables/nftables match
  rule, which is common in firewall configurations
- **Trigger:** Receiving a TCP packet with malformed options (last
  option byte >= 2 with no room for the length byte)
- **Severity:** Out-of-bounds read in packet processing - potential
  information leak or crash
- **Exploitability:** Could be triggered remotely by sending crafted TCP
  packets

### 6. STABILITY INDICATORS

- Author is Florian Westphal, a well-known netfilter maintainer
- The fix is trivial and obviously correct
- The code being fixed has existed for a very long time (the `xt_tcpmss`
  module is ancient)

### 7. DEPENDENCY CHECK

- No dependencies on other commits
- The affected code exists in all stable trees (this is long-standing
  code)
- The patch applies cleanly as a standalone fix

### CONCLUSION

This is a textbook stable backport candidate:
- **Fixes a real bug:** Out-of-bounds read in TCP option parsing
- **Security-sensitive:** In the netfilter packet processing path,
  remotely triggerable
- **Minimal and surgical:** One condition added to one line
- **Zero regression risk:** The additional check is strictly correct
- **No new features:** Pure bug fix
- **Author is subsystem maintainer:** Florian Westphal maintains
  netfilter
- **Affects all stable trees:** The vulnerable code is ancient

**YES**

 net/netfilter/xt_tcpmss.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/xt_tcpmss.c b/net/netfilter/xt_tcpmss.c
index 37704ab017992..0d32d4841cb32 100644
--- a/net/netfilter/xt_tcpmss.c
+++ b/net/netfilter/xt_tcpmss.c
@@ -61,7 +61,7 @@ tcpmss_mt(const struct sk_buff *skb, struct xt_action_param *par)
 			return (mssval >= info->mss_min &&
 				mssval <= info->mss_max) ^ info->invert;
 		}
-		if (op[i] < 2)
+		if (op[i] < 2 || i == optlen - 1)
 			i++;
 		else
 			i += op[i+1] ? : 1;
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-02-14 21:27 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260214212452.782265-1-sashal@kernel.org>
2026-02-14 21:22 ` [PATCH AUTOSEL 6.19-5.10] netfilter: nf_conntrack: Add allow_clash to generic protocol handler Sasha Levin
2026-02-14 21:23 ` [PATCH AUTOSEL 6.19-5.10] netfilter: xt_tcpmss: check remaining length before reading optlen Sasha Levin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox