Thread (1 message) flat view 1 message, 1 author, 2d ago
HOTtoday REVIEWED: 1 (0M)

1 review trailer.

[PATCH AUTOSEL 6.18-6.1] net: hsr: require valid EOT supervision TLV

From: Sasha Levin <sashal@kernel.org>
Date: 2026-08-31 13:49:45
Also in: linux-patches, lkml, stable
Subsystem: hsr network protocol, networking [general], the rest · Maintainers: "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Linus Torvalds 

From: Luka Gejak <redacted>

[ Upstream commit 46d111a3ef3b5972804dcdce0833767143a12192 ]

Supervision frames are only valid if terminated with a zero-length EOT
TLV. The current check fails to reject non-EOT entries as the terminal
TLV, potentially allowing malformed supervision traffic.

Fix this by strictly requiring the terminal TLV to be HSR_TLV_EOT with
a length of zero.

Signed-off-by: Luka Gejak <redacted>
Reviewed-by: Fernando Fernandez Mancera <redacted>
Link: https://patch.msgid.link/20260523130420.62144-1-luka.gejak@linux.dev
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Phase 1: Commit Message Forensics

### Step 1.1: Subject Line
**Record:** `[net: hsr]` `[require]` — HSR supervision-frame validation:
strictly require a zero-length EOT TLV at the end of the TLV chain.

### Step 1.2: Tags
**Record:**
- **Signed-off-by:** Luka Gejak `[off-list ref]` (author)
- **Reviewed-by:** Fernando Fernandez Mancera `[off-list ref]`
  (subsystem reviewer)
- **Link:**
  https://patch.msgid.link/20260523130420.62144-1-luka.gejak@linux.dev
- **Signed-off-by:** Jakub Kicinski `[off-list ref]` (net maintainer)
- No Fixes:, Reported-by:, Tested-by:, Acked-by:, or Cc: stable tags
- Notable: Reviewed by SUSE HSR contributor; no syzbot or user bug
  report

### Step 1.3: Body Analysis
**Record:**
- **Bug:** Supervision frames must end with a zero-length EOT TLV per
  IEC 62439-3. The existing check only rejects EOT TLVs with non-zero
  length; it does not reject non-EOT TLVs as the terminal entry.
- **Symptom:** Malformed supervision traffic can be accepted as valid.
- **Root cause:** Inverted conditional logic — accepts any terminal TLV
  that is not `(EOT && length != 0)`.
- **Version info:** None in message.

### Step 1.4: Hidden Bug Fix Detection
**Record:** Explicit protocol-validation bug fix, not disguised cleanup.
The inverted `&&` vs `||`/`!=` is a classic logic error.

---

## Phase 2: Diff Analysis

### Step 2.1: Inventory
**Record:**
- **Files:** `net/hsr/hsr_forward.c` (+1 / -1)
- **Function:** `is_supervision_frame()`
- **Scope:** Single-file, single-line surgical fix

### Step 2.2: Code Flow Change
**Record:**
- **Before:** Reject only if `type == HSR_TLV_EOT && length != 0`. All
  other terminal TLVs (including non-EOT with length 0 or non-zero) are
  accepted.
- **After:** Reject unless `type == HSR_TLV_EOT && length == 0`. Only a
  proper EOT terminator is accepted.
- **Path:** Receive path in `is_supervision_frame()`, called for every
  HSR/PRP frame.

### Step 2.3: Bug Mechanism
**Record:**
- **Category:** Logic / protocol correctness
- **Mechanism:** De Morgan inversion. Comment says “end of tlvs must
  follow at the end,” but old code only filtered malformed EOT entries,
  not non-EOT terminal TLVs.

### Step 2.4: Fix Quality
**Record:** Obviously correct, minimal, no API changes. Regression risk
is very low — only makes validation stricter (rejects more malformed
frames). No deadlock or locking changes.

---

## Phase 3: Git History Investigation

### Step 3.1: Blame
**Record:** Buggy EOT check introduced in `eafaa88b3eb7` (“net: hsr: Add
support for redbox supervision frames”, Oct 2021). Present in this tree
since that commit. `eafaa88b3eb7` is an ancestor of HEAD.

### Step 3.2: Fixes: Tag
**Record:** N/A — no Fixes: tag in commit message.

### Step 3.3: Related File History
**Record:** Related stable commits on `stable/linux-6.18.y`:
- `fbd0662f9c9a6` — “net: hsr: fix potential OOB access in supervision
  frame handling” (same author, same day, already in this tree)
- `eafaa88b3eb7` — introduced the buggy EOT check
- `51dd4ee037222`, `295de650d3aaf` — earlier supervision parsing fixes

### Step 3.4: Author Context
**Record:** Luka Gejak has multiple stable backports in this tree
(`fbd0662f9c9a6`, `1fe371a34e801`, rtw88 fixes). Active HSR contributor.

### Step 3.5: Dependencies
**Record:** Standalone. Originally part of a larger series (v1–v3), but
from v4 onward it is a standalone 1/2 or single patch. No structural
prerequisites. Applies cleanly on top of current tree (`git apply
--check` passed).

---

## Phase 4: Mailing List and External Research

### Step 4.1: Original Discussion
**Record:** `b4 dig -c 46d111a3ef3b5` found thread at
https://patch.msgid.link/20260523130420.62144-1-luka.gejak@linux.dev.
Series evolved v1–v7; committed version is v7 (latest). No NAKs found in
saved mbox. No explicit stable nomination in thread.

### Step 4.2: Reviewers
**Record:** `b4 dig -w`: CC'd netdev maintainers (Miller, Dumazet,
Kicinski, Abeni, Horman) and Felix Maurer (HSR maintainer). Reviewed-by
from Fernando Fernandez Mancera (SUSE).

### Step 4.3: Bug Report
**Record:** No external bug report, syzbot, or Bugzilla link. Author-
identified logic bug.

### Step 4.4: Series Context
**Record:** v1–v3 bundled with “serialize seq_blocks merge”; v4+ split
out as standalone EOT fix. No other series patches required.

### Step 4.5: Stable List
**Record:** No stable-list discussion found. Companion OOB fix
(`fbd0662f9c9a6`) was already backported to this tree.

---

## Phase 5: Code Semantic Analysis

### Step 5.1: Key Functions
**Record:** `is_supervision_frame()` modified.

### Step 5.2: Callers
**Record:** Called from `fill_frame_info()` (line 691), which is called
from `hsr_forward_skb()` (line 739). `hsr_forward_skb()` is invoked
from:
- `hsr_slave.c` — slave port receive
- `hsr_device.c` — master/interlink receive

Every HSR/PRP received frame goes through this path.

### Step 5.3: Callees / Downstream Effects
**Record:** When `is_supervision_frame()` returns true:
- `hsr_get_node()` runs with `is_sup=true` (affects node DB, SAN info)
- `hsr_handle_sup_frame()` called on master for non-proxy supervision
  (node merging)
- Supervision-specific forwarding: dropped on interlink, special path ID
  (0xf) for HSRv0
- `prp_check_lsdu_size()` uses `is_supervision` flag

### Step 5.4: Reachability
**Record:** Reachable from network receive on any HSR/PRP-configured
interface. Attacker on the HSR/PRP segment can send crafted frames.
HSR/PRP is config-specific (`CONFIG_HSR`), not universal.

### Step 5.5: Similar Patterns
**Record:** No similar inverted EOT check elsewhere in `net/hsr/`.
`is_proxy_supervision_frame()` does not perform EOT validation
(different purpose).

---

## Phase 6: Cross-Reference Against Local Tree

### Step 6.1: Buggy Code Present?
**Record:** YES. Local tree is **v6.18.44** (`git describe HEAD`). Buggy
code at lines 113–115 of `net/hsr/hsr_forward.c`:

```113:115:net/hsr/hsr_forward.c
        if (hsr_sup_tlv->HSR_TLV_type == HSR_TLV_EOT &&
            hsr_sup_tlv->HSR_TLV_length != 0)
                return false;
Bug present since `eafaa88b3eb7` (2021). Fix commit `46d111a3ef3b5` is
NOT in this tree.

### Step 6.2: Backport Complications
**Record:** Clean apply expected. OOB fix (`fbd0662f9c9a6`) already
changed `pskb_may_pull()` offsets but left the EOT check unchanged. `git
format-patch -1 46d111a3ef3b5 --stdout | git apply --check` succeeded.

### Step 6.3: Related Fixes Already Present?
**Record:** OOB fix `fbd0662f9c9a6` is in tree (companion fix, same
author/day). EOT logic fix is not.

---

## Phase 7: Subsystem Context

### Step 7.1: Subsystem Criticality
**Record:** `net/hsr` — HSR/PRP redundancy protocol for
industrial/utility networks. **IMPORTANT** for that niche;
**PERIPHERAL** globally (requires `CONFIG_HSR`).

### Step 7.2: Activity
**Record:** Active development in 6.18.y (VLAN support, OOB fix, memory
leak fix, RedBox support).

---

## Phase 8: Impact and Risk Assessment

### Step 8.1: Who Is Affected
**Record:** Users running HSR/PRP (`CONFIG_HSR`) — industrial
automation, power grid, substation networking.

### Step 8.2: Trigger Conditions
**Record:** Malformed supervision frame on the HSR/PRP network with a
non-EOT terminal TLV (any type other than EOT=0 with length 0). Attacker
or faulty device on the L2 segment. Config-specific, not every kernel
user.

### Step 8.3: Failure Mode Severity
**Record:** Malformed frames accepted as valid supervision → incorrect
node DB updates, node merging via `hsr_handle_sup_frame()`, altered
forwarding/drop behavior. **Severity: MEDIUM-HIGH** for HSR deployments
(protocol state corruption / redundancy disruption), but not a kernel
crash, OOB, or UAF. No memory-safety issue.

### Step 8.4: Risk-Benefit
**Record:**
- **Benefit:** Correct IEC 62439-3 validation; completes supervision
  hardening alongside already-backported OOB fix; protects node DB
  integrity.
- **Risk:** Very low — one-line logic inversion, only rejects more bad
  frames.
- **Ratio:** Favorable for HSR users; low global risk.

---

## Phase 9: Final Synthesis

### Step 9.1: Evidence Summary

**FOR backport:**
- Clear logic bug since 2021 (inverted conditional)
- Real protocol validation failure with node DB / forwarding impact
- 1-line, obviously correct, applies cleanly
- Reviewed by HSR contributor and net maintainer
- Companion OOB fix from same author already in v6.18.44
- Standalone, no dependencies

**AGAINST backport:**
- No crash, syzbot report, or user bug report
- Niche subsystem (`CONFIG_HSR`)
- Impact is protocol/state correctness, not memory safety
- No explicit stable nomination

**Unresolved:** No field reports quantifying real-world exploitation.

### Step 9.2: Stable Rules Checklist
1. Obviously correct and tested? **PASS** — logic inversion is self-
   evident; Reviewed-by present; no runtime test reported.
2. Fixes a real bug? **PASS** — inverted conditional since
   `eafaa88b3eb7`.
3. Important issue? **PASS (moderate)** — HSR node DB / redundancy state
   corruption for industrial users, not kernel panic.
4. Small and contained? **PASS** — 1 line, 1 file.
5. No new features/APIs? **PASS**.
6. Can apply to local tree? **PASS** — verified with `git apply
   --check`.

### Step 9.3: Exception Categories
**Record:** None (not device ID, quirk, DT, build, or docs).

### Step 9.4: Decision Rationale

This is a longstanding logic inversion in HSR supervision-frame
validation. Since 2021, any terminal TLV that is not “EOT with non-zero
length” has been accepted, violating IEC 62439-3 and allowing malformed
supervision traffic to drive node DB updates and supervision-specific
forwarding. The fix is a one-line De Morgan correction with negligible
regression risk.

The companion OOB fix from the same author on the same day is already in
v6.18.44; leaving this EOT validation gap incomplete is inconsistent.
For HSR/PRP deployments — used in safety-critical industrial
infrastructure — protocol validation that protects node state integrity
is appropriate stable material even without a crash report.

---

## Verification

- [Phase 1] Parsed subject, tags, body from commit `46d111a3ef3b5`
- [Phase 1] No syzbot/Reported-by; Reviewed-by: Fernando Fernandez
  Mancera
- [Phase 2] Diff: 1 line in `is_supervision_frame()`, logic inversion
  confirmed
- [Phase 2] Read current buggy code at lines 113–115 of
  `net/hsr/hsr_forward.c`
- [Phase 3] `git show eafaa88b3eb7`: buggy EOT check introduced Oct 2021
- [Phase 3] `git merge-base --is-ancestor eafaa88b3eb7 HEAD`: bug-
  introducing commit in tree
- [Phase 3] `git merge-base --is-ancestor 46d111a3ef3b5 HEAD`: fix NOT
  in tree
- [Phase 3] `git log stable/linux-6.18.y -- net/hsr/hsr_forward.c`:
  related OOB fix `fbd0662f9c9a6` present
- [Phase 3] `git merge-base --is-ancestor fbd0662f9c9a6 HEAD`: OOB fix
  in tree
- [Phase 4] `b4 dig -c 46d111a3ef3b5`: lore thread found, v7 is latest
- [Phase 4] `b4 dig -a`: series v1–v7; standalone from v4 onward
- [Phase 4] `b4 dig -w`: netdev maintainers CC'd
- [Phase 4] Mbox grep: Reviewed-by confirmed; no stable nomination or
  NAK
- [Phase 5] `grep is_supervision_frame`: caller chain `fill_frame_info`
  → `hsr_forward_skb` → slave/master receive
- [Phase 5] Read `hsr_handle_sup_frame()`, `hsr_get_node()`,
  `hsr_forward_do()` for downstream impact
- [Phase 6] `git describe HEAD`: v6.18.44
- [Phase 6] Buggy code confirmed present in stable release commit
  `1efe5d048a391`
- [Phase 6] `git format-patch -1 46d111a3ef3b5 --stdout | git apply
  --check`: applies cleanly
- [Phase 8] Assessed severity as protocol state corruption for HSR
  users, not kernel memory safety

**YES**The background checks finished and match what the analysis
already used:

- **Mainline search** found commit `46d111a3ef3b5` (“net: hsr: require
  valid EOT supervision TLV”) on master/linux-next.
- **Author history on stable/linux-6.18.y** shows the companion OOB fix
  (`fbd0662f9c9a6`) is already in v6.18.44, while this EOT logic fix is
  not.

Verdict for this tree (v6.18.44): **YES** — one-line protocol validation
fix, applies cleanly, and completes the supervision-frame hardening
started by the already-backported OOB patch.

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

diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
index 299de290ddaa5..901eb85c112d1 100644
--- a/net/hsr/hsr_forward.c
+++ b/net/hsr/hsr_forward.c
@@ -110,7 +110,7 @@ static bool is_supervision_frame(struct hsr_priv *hsr, struct sk_buff *skb)
 	}
 
 	/* end of tlvs must follow at the end */
-	if (hsr_sup_tlv->HSR_TLV_type == HSR_TLV_EOT &&
+	if (hsr_sup_tlv->HSR_TLV_type != HSR_TLV_EOT ||
 	    hsr_sup_tlv->HSR_TLV_length != 0)
 		return false;
 
-- 
2.53.0
Other recent patches touching these files
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help