[PATCH AUTOSEL 6.18] net: mscc: ocelot: validate netdev belongs to switch in .netdev_to_port()
From: Sasha Levin <sashal@kernel.org>
Date: 2026-08-31 13:43:40
Also in:
linux-patches, lkml, stable
Subsystem:
networking drivers, networking [dsa], ocelot ethernet switch driver, the rest · Maintainers:
Andrew Lunn, "David S. Miller", Eric Dumazet, Jakub Kicinski, Paolo Abeni, Andrew Lunn, Vladimir Oltean, Vladimir Oltean, Linus Torvalds
From: David Yang <mmyangfl@gmail.com> [ Upstream commit ea07514700f7a5125db67c9d1cbfb06e65c30a07 ] The .netdev_to_port() currently takes only a net_device and returns the port index, without verifying the netdev actually belongs to the switch being operated on. This can cause flower rule parsing to silently resolve to a wrong port on the local hardware. Update both implementations felix_netdev_to_port() and ocelot_netdev_to_port() to validate ownership. Also update the callers in ocelot_flower.c to pass through the ocelot context. Signed-off-by: David Yang <mmyangfl@gmail.com> Link: https://patch.msgid.link/20260606125247.305167-1-mmyangfl@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: `[net: mscc: ocelot] [validate] validate netdev belongs to switch in .netdev_to_port()` **Step 1.2 — Tags** Record: - **Signed-off-by:** David Yang `[off-list ref]` (author) - **Link:** https://patch.msgid.link/20260606125247.305167-1-mmyangfl@gmail.com - **Signed-off-by:** Jakub Kicinski `[off-list ref]` (committer) - No Fixes:, Reported-by:, Tested-by:, Reviewed-by:, Acked-by:, or Cc: stable@vger.kernel.org - Notable: no syzbot/reporter tags; no explicit stable nomination **Step 1.3 — Body analysis** Record: - **Bug:** `.netdev_to_port()` maps a `net_device` to a port index without checking that netdev belongs to the `ocelot` switch instance being configured. - **Symptom:** tc-flower rule parsing can **silently** resolve to the wrong port on the local switch hardware. - **Root cause:** Both `felix_netdev_to_port()` (DSA) and `ocelot_netdev_to_port()` (standalone) accept any netdev of the correct driver type, regardless of which switch instance owns it. - **Fix approach:** Pass `struct ocelot *` into both implementations and validate ownership (`dp->ds != ds` for Felix; `priv->port.ocelot != ocelot` for standalone ocelot); update `ocelot_ops` callback and `ocelot_flower.c` callers. **Step 1.4 — Hidden bug fix?** Record: **Yes.** Despite “validate” wording rather than “fix”, this is a real logic/correctness bug in hardware offload parsing, not cosmetic cleanup. --- ## Phase 2: Diff Analysis **Step 2.1 — Inventory** Record: - **Files:** 6 files, +12 / −8 lines - **Functions modified:** `felix_netdev_to_port()`, `ocelot_netdev_to_port()`, `ocelot_flower_parse_egress_port()`, `ocelot_flower_parse_indev()` - **Headers:** `ocelot_ops.netdev_to_port` signature changed - **Scope:** Single-subsystem, surgical fix **Step 2.2 — Code flow per hunk** Record: - **`felix_netdev_to_port()`:** Before: any DSA user port netdev → port index. After: rejects netdevs belonging to a different `dsa_switch` (`dp->ds != ds`). - **`ocelot_netdev_to_port()`:** Before: any netdev with `ocelot_port_netdev_ops` → port index. After: also requires `priv->port.ocelot == ocelot`. - **`ocelot_flower_parse_egress_port()`:** Before: `netdev_to_port(a->dev)`. After: `netdev_to_port(ocelot, a->dev)` so validation is switch-scoped. - **`ocelot_flower_parse_indev()`:** Same change for ingress-ifindex matching. **Step 2.3 — Bug mechanism** Record: **Logic / correctness fix** in tc-flower hardware offload path. On systems with multiple Ocelot/Felix switches, a rule installed on switch A referencing a netdev from switch B could return switch B’s port index and program switch A’s VCAP hardware with that index — wrong redirect, mirror, or ingress-port match. **Step 2.4 — Fix quality** Record: Fix is obviously correct and minimal. Regression risk is very low: single-switch systems always pass the new checks. The internal `ocelot_ops` signature change is fully contained within this patch (all implementations and callers updated). No deadlock or hot-path performance concern. --- ## Phase 3: Git History Investigation **Step 3.1 — Blame** Record: `ocelot_netdev_to_port()` introduced in `319e4dd11a207` (2020-10-02, “introduce conversion helpers between port and netdev”). The missing ownership check has been present since introduction. `priv->port.ocelot` field exists in `struct ocelot_port` since the port structure was defined. **Step 3.2 — Fixes: tag** Record: Not applicable — no Fixes: tag in commit message. **Step 3.3 — Related file history** Record: `ocelot_flower.c` has active tc-flower development (mirred-to- foreign-interfaces in `49a09073cb23e`, control-flag validation, etc.). This fix is standalone — not part of a multi-patch series. **Step 3.4 — Author context** Record: David Yang; subsystem maintainers (Vladimir Oltean, Andrew Lunn, netdev maintainers) were CC’d on submission per `b4 dig -w`. No other commits from this author in the mscc/ocelot paths in this tree. **Step 3.5 — Dependencies** Record: No prerequisites. Commit is self-contained. `git show ea07514700f7a | git apply --check` succeeds on this tree. --- ## Phase 4: Mailing List and External Research **Step 4.1 — Original discussion** Record: - **URL:** https://patch.msgid.link/20260606125247.305167-1-mmyangfl@gmail.com - **Series:** v2 only (v1 referenced at https://lore.kernel.org/r/20260603024234.66603-1-mmyangfl@gmail.com (local)) - **Review feedback:** Thread contains only the patch and patchwork-bot “applied to net-next” notice — no human review replies, no stable nominations, no NAKs in saved mbox **Step 4.2 — Reviewers** Record: CC list includes Vladimir Oltean (NXP ocelot maintainer), Andrew Lunn, Jakub Kicinski, netdev list. No Reviewed-by/Acked-by in final commit. **Step 4.3 — Bug reports** Record: No external bug report, syzbot link, orbugzilla reference. Bug identified by code inspection. **Step 4.4 — Related patches** Record: v1 also fixed `ocelot_netdev_to_port()`; v2 is the committed version. Standalone one-patch fix. **Step 4.5 — Stable list** Record: Not searched on lore stable list (no indicators in thread). No prior stable discussion found. --- ## Phase 5: Code Semantic Analysis **Step 5.1 — Key functions** Record: `felix_netdev_to_port()`, `ocelot_netdev_to_port()`, `ocelot_flower_parse_egress_port()`, `ocelot_flower_parse_indev()`, `ocelot_cls_flower_replace()` (caller chain entry) **Step 5.2 — Callers** Record: - `netdev_to_port` is only invoked via `ocelot->ops->netdev_to_port()` from `ocelot_flower.c` (2 call sites). - Flower offload reached from userspace via `ndo_setup_tc` → `ocelot_setup_tc_cls_flower()` → `ocelot_cls_flower_replace()` (`ocelot_net.c:197-209`) and Felix DSA `felix_port_setup_tc()` → `ocelot_cls_flower_replace()` (`felix.c:1961`). - Requires `CAP_NET_ADMIN` for tc rule installation. **Step 5.3 — Callees** Record: `dsa_port_from_netdev()`, `ocelot_netdevice_dev_check()`, `netdev_priv()`, port-index extraction. **Step 5.4 — Reachability** Record: Userspace `tc filter add` / netlink FLOW_CLS_REPLACE on an ocelot/Felix port netdev → flower parse → `netdev_to_port()`. Reachable on any system with MSCC ocelot or Felix DSA hardware and tc-flower offload enabled. **Step 5.5 — Similar patterns** Record: `port_to_netdev()` already takes `struct ocelot *` and is switch-scoped; `netdev_to_port()` was the asymmetric missing half. `ocelot_netdevice_dev_check()` only validates driver type (same `netdev_ops` for all instances), which is why cross-instance netdevs were accepted. --- ## Phase 6: Cross-Referencing Against Local Tree **Step 6.1 — Buggy code present?** Record: **Yes.** Local tree is **Linux 6.18.44** (`git describe HEAD` → `v6.18.44`). Commit `ea07514700f7a` is **not** in this tree (`git merge- base --is-ancestor` returns false). Buggy code confirmed at `ocelot_net.c:987-996`, `felix.c:2382-2390`, `ocelot_flower.c:237,583`. **Step 6.2 — Backport complications** Record: **Clean apply expected** — verified with `git apply --check`. No conflicting refactors in these functions between this tree and mainline patch. **Step 6.3 — Related fixes already present?** Record: No equivalent ownership validation found in this tree. --- ## Phase 7: Subsystem and Maintainer Context **Step 7.1 — Subsystem criticality** Record: **IMPORTANT** — MSCC Ocelot / NXP Felix DSA Ethernet switch drivers. Not core kernel, but hardware datapath offload for managed switches used in embedded/industrial/TSN (NXP Layerscape boards, Microchip switches). **Step 7.2 — Subsystem activity** Record: Actively maintained in 6.18.y (recent commits: lock protection in xmit, FDMA paths, timestamping, mirred support). --- ## Phase 8: Impact and Risk Assessment **Step 8.1 — Who is affected** Record: **Driver-specific, config-specific** — users of MSCC ocelot or Felix DSA hardware with **multiple switch instances** on one system who install **tc-flower hardware offload rules** referencing netdevs (redirect, mirror, or `ingress_ifindex` match). **Step 8.2 — Trigger conditions** Record: - Install tc-flower rule on switch A’s port - Rule references a netdev belonging to switch B (another ocelot/Felix instance) - Common in multi-switch automotive/industrial boards; uncommon on generic servers - Requires root/CAP_NET_ADMIN; not unprivileged trigger **Step 8.3 — Failure mode severity** Record: - **Failure mode:** Silent misprogramming of VCAP hardware filters — traffic redirected/mirrored to wrong port, or ingress matching on wrong port index - **Severity: MEDIUM-HIGH** for affected deployments (wrong datapath behavior, broken network policy/isolation), but **not CRITICAL** (no kernel oops, hang, or memory corruption) **Step 8.4 — Risk-benefit** Record: - **Benefit:** Prevents insidious silent hardware offload errors on multi-switch systems; bug present since 2020 - **Risk:** Very low — ~20 lines, no behavior change for correct single- switch configurations - **Ratio:** Good benefit/risk for affected users; limited benefit for typical single-switch deployments --- ## Phase 9: Final Synthesis **Step 9.1 — Evidence compile** **FOR backport:** - Real, verifiable logic bug in hardware offload path - Silent wrong port programming — “oh, that’s not good” per stable- kernel-rules.rst - Bug present since 2020 in this 6.18.44 tree - Small (20 lines), obviously correct, applies cleanly - Zero regression risk on single-switch systems - Affects production embedded/TSN multi-switch deployments (Felix on NXP SoCs) **AGAINST backport:** - No user reports, syzbot, or CVE - Requires uncommon multi-switch + tc-flower configuration - No kernel crash/oops/memory corruption - Niche driver subsystem - No reviewer stable nomination **Unresolved:** No reproduction report from a deployed system; exact prevalence of multi-switch Felix/Ocelot configs in 6.18.y production is unverified. **Step 9.2 — Stable rules checklist** 1. Obviously correct and tested? **PASS** — logic is straightforward; no Tested-by but maintainer merged to netdev 2. Fixes a real bug affecting users? **PASS** — silent wrong hardware port mapping on multi-switch tc-flower offload 3. Important issue? **PASS (borderline)** — wrong datapath steering / broken offload policy; not crash-level but serious functional incorrectness for affected users 4. Small and contained? **PASS** — 6 files, 20 lines net 5. No new features or APIs? **PASS** — internal callback signature change only 6. Can apply to local tree? **PASS** — `git apply --check` succeeds **Step 9.3 — Exception categories** Record: None (not device ID, quirk, DT, build fix, or docs fix). **Step 9.4 — Decision rationale** This commit fixes a longstanding correctness bug where tc-flower hardware offload on one Ocelot/Felix switch can silently program the wrong port when a rule references a netdev from a different switch instance. The failure mode is insidious mis-steering of traffic rather than a kernel crash, but for managed-switch deployments relying on hardware offload for redirect, mirror, or ingress-port matching, that is a serious functional defect. The fix is minimal, self-contained, applies cleanly to Linux 6.18.44, and is zero-risk for the common single-switch case. --- ## Verification - [Phase 1] Parsed commit message tags from provided diff and `git show ea07514700f7a` - [Phase 2] Diff analysis: 6 files, +12/−8; ownership checks in `felix_netdev_to_port` and `ocelot_netdev_to_port` - [Phase 3] `git blame` on `ocelot_net.c:987-997`: introduced `319e4dd11a207` (2020-10-02) - [Phase 3] `git show 319e4dd11a207`: original commit introducing helpers without ownership check - [Phase 3] `git show ea07514700f7a | git apply --check`: patch applies cleanly - [Phase 3] `git merge-base --is-ancestor ea07514700f7a HEAD`: exit 1 — fix NOT in 6.18.44 - [Phase 4] `b4 dig -c ea07514700f7a`: lore URL found - [Phase 4] `b4 dig -c ea07514700f7a -a`: v2 series only - [Phase 4] `b4 dig -c ea07514700f7a -w`: maintainers CC’d (Oltean, Lunn, Kicinski, netdev) - [Phase 4] `b4 dig -m /tmp/ocelot_netdev.mbox`: thread has patch + bot apply notice only; no stable nomination - [Phase 5] `grep netdev_to_port`: only 2 call sites in `ocelot_flower.c`; ops table assignments in felix/ocelot drivers - [Phase 5] Read `ocelot_net.c:197-216`: userspace tc path via `ndo_setup_tc` - [Phase 5] Read `felix.c:1961,2023`: DSA tc-flower path - [Phase 5] Read `ocelot_flower.c:231-254,550-593`: egress redirect/mirror and ingress-ifindex parse paths - [Phase 6] `git describe HEAD`: v6.18.44 - [Phase 6] Read current buggy code at `ocelot_net.c:987-996`, `felix.c:2382-2390` - [Phase 6] Verified `priv->port.ocelot` field in `include/soc/mscc/ocelot.h:772-773` - [Phase 8] Failure mode: silent wrong VCAP port programming, severity MEDIUM-HIGH for multi-switch offload users **YES** drivers/net/dsa/ocelot/felix.c | 6 ++++-- drivers/net/dsa/ocelot/felix.h | 2 +- drivers/net/ethernet/mscc/ocelot.h | 2 +- drivers/net/ethernet/mscc/ocelot_flower.c | 4 ++-- drivers/net/ethernet/mscc/ocelot_net.c | 4 +++- include/soc/mscc/ocelot.h | 2 +- 6 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 20ab558fde247..c4f1b6b335a24 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c@@ -2379,12 +2379,14 @@ struct net_device *felix_port_to_netdev(struct ocelot *ocelot, int port) } EXPORT_SYMBOL_GPL(felix_port_to_netdev); -int felix_netdev_to_port(struct net_device *dev) +int felix_netdev_to_port(struct ocelot *ocelot, struct net_device *dev) { + struct felix *felix = ocelot_to_felix(ocelot); + struct dsa_switch *ds = felix->ds; struct dsa_port *dp; dp = dsa_port_from_netdev(dev); - if (IS_ERR(dp)) + if (IS_ERR(dp) || dp->ds != ds) return -EINVAL; return dp->index;
diff --git a/drivers/net/dsa/ocelot/felix.h b/drivers/net/dsa/ocelot/felix.h
index a657b190c5d7b..19addcfd62bec 100644
--- a/drivers/net/dsa/ocelot/felix.h
+++ b/drivers/net/dsa/ocelot/felix.h@@ -104,6 +104,6 @@ int felix_register_switch(struct device *dev, resource_size_t switch_base, enum dsa_tag_protocol init_tag_proto, const struct felix_info *info); struct net_device *felix_port_to_netdev(struct ocelot *ocelot, int port); -int felix_netdev_to_port(struct net_device *dev); +int felix_netdev_to_port(struct ocelot *ocelot, struct net_device *dev); #endif
diff --git a/drivers/net/ethernet/mscc/ocelot.h b/drivers/net/ethernet/mscc/ocelot.h
index e50be508c1663..42d2c456f7128 100644
--- a/drivers/net/ethernet/mscc/ocelot.h
+++ b/drivers/net/ethernet/mscc/ocelot.h@@ -92,7 +92,7 @@ int ocelot_mact_learn(struct ocelot *ocelot, int port, int ocelot_mact_forget(struct ocelot *ocelot, const unsigned char mac[ETH_ALEN], unsigned int vid); struct net_device *ocelot_port_to_netdev(struct ocelot *ocelot, int port); -int ocelot_netdev_to_port(struct net_device *dev); +int ocelot_netdev_to_port(struct ocelot *ocelot, struct net_device *dev); int ocelot_probe_port(struct ocelot *ocelot, int port, struct regmap *target, struct device_node *portnp);
diff --git a/drivers/net/ethernet/mscc/ocelot_flower.c b/drivers/net/ethernet/mscc/ocelot_flower.c
index 986b1f150e3b3..e80ede65f81ad 100644
--- a/drivers/net/ethernet/mscc/ocelot_flower.c
+++ b/drivers/net/ethernet/mscc/ocelot_flower.c@@ -233,8 +233,8 @@ ocelot_flower_parse_egress_port(struct ocelot *ocelot, struct flow_cls_offload * const struct flow_action_entry *a, bool mirror, struct netlink_ext_ack *extack) { + int egress_port = ocelot->ops->netdev_to_port(ocelot, a->dev); const char *act_string = mirror ? "mirror" : "redirect"; - int egress_port = ocelot->ops->netdev_to_port(a->dev); enum flow_action_id offloadable_act_id; offloadable_act_id = mirror ? FLOW_ACTION_MIRRED : FLOW_ACTION_REDIRECT;
@@ -580,7 +580,7 @@ static int ocelot_flower_parse_indev(struct ocelot *ocelot, int port, return -ENOENT; } - ingress_port = ocelot->ops->netdev_to_port(indev); + ingress_port = ocelot->ops->netdev_to_port(ocelot, indev); if (ingress_port < 0) { NL_SET_ERR_MSG_MOD(extack, "Can only offload an ocelot ingress port");
diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
index 1b82693204640..4d45e136f08f5 100644
--- a/drivers/net/ethernet/mscc/ocelot_net.c
+++ b/drivers/net/ethernet/mscc/ocelot_net.c@@ -984,7 +984,7 @@ static bool ocelot_netdevice_dev_check(const struct net_device *dev) return dev->netdev_ops == &ocelot_port_netdev_ops; } -int ocelot_netdev_to_port(struct net_device *dev) +int ocelot_netdev_to_port(struct ocelot *ocelot, struct net_device *dev) { struct ocelot_port_private *priv;
@@ -992,6 +992,8 @@ int ocelot_netdev_to_port(struct net_device *dev) return -EINVAL; priv = netdev_priv(dev); + if (priv->port.ocelot != ocelot) + return -EINVAL; return priv->port.index; }
diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index 48d6deb3efd71..05a8191b148e9 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h@@ -650,7 +650,7 @@ struct device_node; struct ocelot_ops { struct net_device *(*port_to_netdev)(struct ocelot *ocelot, int port); - int (*netdev_to_port)(struct net_device *dev); + int (*netdev_to_port)(struct ocelot *ocelot, struct net_device *dev); int (*reset)(struct ocelot *ocelot); u16 (*wm_enc)(u16 value); u16 (*wm_dec)(u16 value);
--
2.53.0