[PATCH AUTOSEL 6.18-5.10] sctp: Unwind address notifier registration on failure
From: Sasha Levin <sashal@kernel.org>
Date: 2026-08-31 13:37:35
Also in:
linux-patches, linux-sctp, lkml, stable
Subsystem:
networking [general], sctp protocol, the rest · Maintainers:
"David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Marcelo Ricardo Leitner, Xin Long, Linus Torvalds
From: Yuho Choi <redacted> [ Upstream commit c8459ee2fef502d6ef6c063751c33d9ac7943eab ] sctp_v4_add_protocol() and sctp_v6_add_protocol() register their address notifiers before registering the SCTP protocol handlers. If protocol registration fails, the functions return without unregistering the notifiers. Unregister the notifiers on the protocol registration failure paths. Also propagate notifier registration failures instead of ignoring them. Signed-off-by: Yuho Choi <redacted> Link: https://patch.msgid.link/20260608162230.46644-1-dbgh9129@gmail.com 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: `[sctp]` `[Unwind]` — SCTP subsystem; unwind/cleanup on failure in `sctp_v4_add_protocol()` and `sctp_v6_add_protocol()`. **Step 1.2: Tags** Record: - **Signed-off-by:** Yuho Choi `[off-list ref]` (author) - **Link:** https://patch.msgid.link/20260608162230.46644-1-dbgh9129@gmail.com - **Signed-off-by:** Jakub Kicinski `[off-list ref]` (net maintainer merge) - No Fixes:, Reported-by:, Tested-by:, Reviewed-by:, Acked-by:, or Cc: stable tags - Notable: netdev maintainer merge; no syzbot or user bug report **Step 1.3: Body** Record: - **Bug:** Address notifiers are registered before protocol handlers; if `inet_add_protocol()` / `inet6_add_protocol()` fails, notifiers are not unregistered. - **Symptom:** Leaked notifier registrations on SCTP module init failure paths. - **Root cause:** Missing error-path cleanup in `sctp_v4_add_protocol()` and `sctp_v6_add_protocol()`; notifier registration return values ignored. - **Version info:** None in the message. **Step 1.4: Hidden bug fix?** Record: **Yes.** Described as cleanup, but it fixes a real error-path bug: dangling notifier registrations that can outlive a failed SCTP module load and point at freed module text/data. --- ## Phase 2: Diff Analysis **Step 2.1: Inventory** Record: - `net/sctp/protocol.c`: +6/−2 (10 net lines in mainline commit) - `net/sctp/ipv6.c`: +6/−2 - **Functions:** `sctp_v4_add_protocol()`, `sctp_v6_add_protocol()` - **Scope:** Small, two-file, symmetric fix **Step 2.2: Code flow** Record: - **Hunk 1 (`sctp_v4_add_protocol`):** Before — `register_inetaddr_notifier()` return ignored; on `inet_add_protocol()` failure, return `-EAGAIN` with notifier still registered. After — check notifier registration; on protocol registration failure, call `unregister_inetaddr_notifier()`. - **Hunk 2 (`sctp_v6_add_protocol`):** Same pattern with `register_inet6addr_notifier()` / `unregister_inet6addr_notifier()` / `inet6_add_protocol()`. - **Paths affected:** SCTP module init error paths only. **Step 2.3: Bug mechanism** Record: **Category:** Error-path resource leak / dangling notifier registration. - `sctp_inetaddr_notifier` and `sctp_inet6addr_notifier` are static `notifier_block` structures in the SCTP module. - `sctp_init()` calls these functions during `module_init`; on failure it unwinds other resources but does **not** call `sctp_v4_del_protocol()` from `err_add_protocol`, and `err_v6_add_protocol` only calls `sctp_v4_del_protocol()` (not v6 notifier cleanup). - A failed `module_init` unloads the module while a leaked notifier remains on the global inet/inet6 notifier chains → callbacks can run into freed module memory. **Step 2.4: Fix quality** Record: **Obviously correct**, minimal, mirrors the existing `sctp_v4_del_protocol()` / `sctp_v6_del_protocol()` unwind pattern and the mac80211 notifier registration pattern in `ieee80211_register_hw()`. **Regression risk:** Very low; only affects failure paths. --- ## Phase 3: Git History Investigation **Step 3.1: Blame** Record: - `sctp_v4_add_protocol()` notifier pattern: commit `270637abff0cd` (Vlad Yasevich, 2008-03-20) — present since ~2.6 era. - `sctp_v6_add_protocol()` notifier pattern: commit `827bf12236fbaf` (Sridhar Samudrala, 2007-05-04). - Bug has existed in this code for many years. **Step 3.2: Fixes: tag** Record: N/A — no Fixes: tag. **Step 3.3: File history** Record: Recent SCTP changes in this tree are unrelated (netns teardown, UAF fixes, races). No duplicate fix for this notifier issue. Fix commit on master: `c8459ee2fef50` (2026-06-10); **not** in current HEAD (`6.18.44`). **Step 3.4: Author** Record: Yuho Choi — active contributor of similar error-path cleanup fixes (watchdog PM notifier, OF node leaks, etc.). SCTP maintainers (Marcelo Leitner, Xin Long) were CC'd per `b4 dig -w`. **Step 3.5: Dependencies** Record: **Standalone.** No series dependencies; single v1 patch merged to net. `git apply --check` against current tree succeeds. --- ## Phase 4: Mailing List and External Research **Step 4.1: Original discussion** Record: `b4 dig -c c8459ee2fef50` → https://patch.msgid.link/20260608162230.46644-1-dbgh9129@gmail.com `b4 dig -a`: single v1 revision only. Lore fetch blocked by bot protection; could not read thread replies. **Step 4.2: Reviewers** Record: `b4 dig -w` CC'd Marcelo Ricardo Leitner, Xin Long, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, linux- sctp@, netdev@. **Step 4.3: Bug report** Record: No external bug report or syzbot link. **Step 4.4: Related patches** Record: Standalone; not part of a multi-patch series. **Step 4.5: Stable list** Record: Could not search stable@ lore (blocked). No stable nomination verified from available sources. --- ## Phase 5: Code Semantic Analysis **Step 5.1: Key functions** Record: `sctp_v4_add_protocol()`, `sctp_v6_add_protocol()`, callbacks `sctp_inetaddr_event()`, `sctp_inet6addr_event()`. **Step 5.2: Callers** Record: - `sctp_v4_add_protocol()` — called from `sctp_init()` (`module_init`) - `sctp_v6_add_protocol()` — called from `sctp_init()` after v4 succeeds - Not hot-path; module initialization only. **Step 5.3: Callees** Record: `register_inetaddr_notifier()` → `blocking_notifier_chain_register()`; `register_inet6addr_notifier()` → `atomic_notifier_chain_register()`; `inet_add_protocol()` / `inet6_add_protocol()` use `cmpxchg` and return `-1` if the protocol slot is already occupied. **Step 5.4: Reachability** Record: Trigger requires SCTP module init failure after notifier registration — uncommon but possible (e.g. `inet6_add_protocol()` fails after v4 succeeds; `inet_add_protocol()` fails on occupied `IPPROTO_SCTP` slot). After failure, any subsequent IPv4/IPv6 address event can invoke the leaked notifier → **reachable from normal network interface activity**. **Step 5.5: Similar patterns** Record: `net/mac80211/main.c` correctly unwinds notifier registration on failure (`fail_ifa6` → `unregister_inetaddr_notifier`). SCTP lacked the same pattern. --- ## Phase 6: Cross-Reference Against Local Tree **Step 6.1: Buggy code present?** Record: **Yes.** Tree is `v6.18.44` (`stable/linux-6.18.y`, detached HEAD). Current code at `net/sctp/protocol.c:1298-1307` and `net/sctp/ipv6.c:1226-1234` matches the pre-fix state. Fix `c8459ee2fef50` is **not** an ancestor of HEAD. **Step 6.2: Backport complications** Record: **Clean apply** — `git show c8459ee2fef50 | git apply --check` passes with no conflicts. **Step 6.3: Related fixes already present?** Record: None for this notifier unwind issue. --- ## Phase 7: Subsystem Context **Step 7.1: Subsystem** Record: **net/sctp** — networking protocol (IMPORTANT; used in telecom/enterprise, optional `CONFIG_IP_SCTP` module). **Step 7.2: Activity** Record: SCTP in 6.18.y receives active stable fixes (UAF, races, netns teardown); mature subsystem with ongoing maintenance. --- ## Phase 8: Impact and Risk Assessment **Step 8.1: Who is affected** Record: Systems with SCTP built as a module (`CONFIG_IP_SCTP=m`) where module initialization fails after notifier registration but before full protocol registration completes. **Step 8.2: Trigger conditions** Record: - `register_*addr_notifier()` succeeds, then `inet*_add_protocol()` fails (returns `-EAGAIN`) - More plausible v6 path: v4 fully registered, v6 notifier registered, v6 protocol registration fails → `err_v6_add_protocol` cleans v4 but leaves v6 notifier registered - **Likelihood:** Low on typical systems; **not** user-triggerable in normal operation, but possible on init failure/retry scenarios **Step 8.3: Failure mode severity** Record: Leaked notifier on global chain after failed module unload → callback into freed module memory on address events → **kernel oops / use-after-free**. Severity when triggered: **HIGH (CRITICAL)**. Trigger rarity: **LOW**. **Step 8.4: Risk-benefit** Record: - **Benefit:** Prevents dangling notifier / UAF on error paths; enables clean SCTP module reload after init failure - **Risk:** Very low — 16 lines, error-path only, mirrors established kernel patterns - **Ratio:** Favorable for stable despite rare trigger --- ## Phase 9: Final Synthesis **Evidence FOR backport:** - Real bug in long-standing code (since 2007/2008) - Can cause UAF/kernel crash if module init fails and addresses change afterward - Small, obviously correct, applies cleanly to 6.18.44 - Merged to net with maintainer sign-off - Matches established notifier registration cleanup patterns (mac80211, nf_nat_masquerade) **Evidence AGAINST backport:** - No user reports or syzbot reproduction - Trigger path is SCTP module init failure — uncommon in production - Impact limited to systems using SCTP as a module **Stable rules checklist:** 1. Obviously correct and tested? **PASS** — logic is straightforward; no Tested-by, but maintainer-merged 2. Fixes a real bug? **PASS** — notifier leak on error path 3. Important issue? **PASS** — UAF/crash potential (HIGH when triggered) 4. Small and contained? **PASS** — 2 files, ~16 lines 5. No new features/APIs? **PASS** 6. Can apply to local tree? **PASS** — clean apply verified **Exception category:** Error-path cleanup (not device ID/quirk/docs). **Decision rationale for 6.18.y:** The buggy code is present, the fix is minimal and correct, and the failure mode (dangling notifier → UAF on address events after failed module load) is serious enough for stable despite low trigger frequency. This is standard stable material for error-path resource cleanup. --- ## Verification - [Phase 1] Parsed commit message and tags from user-provided diff and `git show c8459ee2fef50` - [Phase 2] Read current `sctp_v4_add_protocol()` / `sctp_v6_add_protocol()` in tree; confirmed pre-fix state - [Phase 2] Read `sctp_init()` error paths (`err_add_protocol`, `err_v6_add_protocol`) — v4 notifier not unwound on v4 failure; v6 notifier not unwound on v6 failure - [Phase 3] `git describe HEAD` → `v6.18.44`; `make kernelversion` → `6.18.44` - [Phase 3] `git blame` on changed lines — bug present since 2007/2008 - [Phase 3] `merge-base --is-ancestor c8459ee2fef50 HEAD` → fix NOT in current tree - [Phase 3] `git apply --check` with mainline patch → applies cleanly - [Phase 4] `b4 dig -c c8459ee2fef50` → lore URL found - [Phase 4] `b4 dig -a` → v1 only - [Phase 4] `b4 dig -w` → SCTP/netdev maintainers CC'd - [Phase 4] UNVERIFIED: Lore thread content (Anubis bot blocked fetch) - [Phase 5] Read `register_inetaddr_notifier()` / `inet_add_protocol()` implementations - [Phase 5] Read `sctp_inetaddr_event()` / notifier block definitions - [Phase 5] Compared with mac80211 notifier unwind pattern - [Phase 6] Confirmed buggy code exists at current line numbers in 6.18.44 - [Phase 8] Assessed failure mode: leaked notifier after failed `module_init` → UAF on address events **YES** net/sctp/ipv6.c | 10 ++++++++-- net/sctp/protocol.c | 10 ++++++++-- 2 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
index 7434309785cc5..922b19982d4fe 100644
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c@@ -1225,11 +1225,17 @@ void sctp_v6_protosw_exit(void) /* Register with inet6 layer. */ int sctp_v6_add_protocol(void) { + int ret; + /* Register notifier for inet6 address additions/deletions. */ - register_inet6addr_notifier(&sctp_inet6addr_notifier); + ret = register_inet6addr_notifier(&sctp_inet6addr_notifier); + if (ret) + return ret; - if (inet6_add_protocol(&sctpv6_protocol, IPPROTO_SCTP) < 0) + if (inet6_add_protocol(&sctpv6_protocol, IPPROTO_SCTP) < 0) { + unregister_inet6addr_notifier(&sctp_inet6addr_notifier); return -EAGAIN; + } return 0; }
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index 5718328cb8a79..c718dc066f3a9 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c@@ -1297,12 +1297,18 @@ static void sctp_v4_protosw_exit(void) static int sctp_v4_add_protocol(void) { + int ret; + /* Register notifier for inet address additions/deletions. */ - register_inetaddr_notifier(&sctp_inetaddr_notifier); + ret = register_inetaddr_notifier(&sctp_inetaddr_notifier); + if (ret) + return ret; /* Register SCTP with inet layer. */ - if (inet_add_protocol(&sctp_protocol, IPPROTO_SCTP) < 0) + if (inet_add_protocol(&sctp_protocol, IPPROTO_SCTP) < 0) { + unregister_inetaddr_notifier(&sctp_inetaddr_notifier); return -EAGAIN; + } return 0; }
--
2.53.0