[PATCH net v3] net: bridge: disable MRP ports before the FDB flush in del_nbp()

[PATCH net v3] net: bridge: disable MRP ports before the FDB flush in del_nbp()

[xiaoblac]

From: Cheng Yongkang <teel4res@gmail.com>

del_nbp() flushes the port's FDB entries before it unregisters the RX
handler. For a normal port, br_stp_disable_port() moves the port to
BR_STATE_DISABLED before this cleanup.

That does not hold for an MRP ring port. br_set_state() ignores state
changes while BR_MRP_AWARE_BIT is set, so br_stp_disable_port() leaves
the port state alone. br_mrp_port_del() then restores forwarding when
the bridge is up before clearing the bit. A frame received between
br_fdb_cleanup_by_dst() and netdev_rx_handler_unregister() can therefore
create a new FDB entry referencing the dying port. Nothing removes that
entry before destroy_nbp_rcu() frees the port, so a later frame to the
learned MAC can be forwarded through the freed port.

Remove the MRP and CFM instances before disabling the port, so that
br_stp_disable_port() actually transitions it to BR_STATE_DISABLED.
With CONFIG_BRIDGE_VLAN_FILTERING=y, synchronize_net() in
nbp_vlan_group_unpublish() also drains receive paths that could still
learn with the old state before the final FDB cleanup.

Found through an LLM-assisted source code analysis workflow and
reproduced on net with KASAN and CONFIG_BRIDGE_VLAN_FILTERING=y in QEMU.
A diagnostic switchdev module accepts MRP objects and delays the
NETDEV_CHANGEUPPER callback for 1.5 s, holding del_nbp() after the FDB
cleanup and before RX handler removal. The reproducer floods the MRP
port with rotating source MACs, waits for the port's RCU callback
through the module's rcu_barrier(), then sends frames to those MACs
from another port. The unpatched kernel reports:

  BUG: KASAN: slab-use-after-free in br_forward+0xb1/0x820
  Call Trace:
   <IRQ>
   br_forward+0xb1/0x820
   br_handle_frame_finish+0x148b/0x1e50
   br_handle_frame+0x90a/0x15c0

With this patch, three runs of six cycles each reach the delay without
a KASAN report.

Fixes: 6536993371fa ("bridge: mrp: Integrate MRP into the bridge")
Suggested-by: Ido Schimmel <idosch@nvidia.com>
Link: https://lore.kernel.org/netdev/20261012123051.GA714215@shredder/
Closes: https://lore.kernel.org/netdev/20261022090431.3641707-1-teel4res@gmail.com/
Signed-off-by: Cheng Yongkang <teel4res@gmail.com>
---
Tested on net cb284914e749 (clang 20, KASAN), with and without this
patch:
- the reproducer above: the unpatched kernel hits the UAF on the first
  cycle; the patched kernel completes all cycles without KASAN, WARN
  or BUG, and a ping / FDB learning / port detach / bridge delete check
  passes
- selftests/net: vlan_bridge_binding.sh, bridge_vlan_dump.sh,
  test_bridge_backup_port.sh and forwarding/bridge_vlan_unaware.sh,
  bridge_vlan_aware.sh, bridge_fdb_learning_limit.sh,
  bridge_sticky_fdb.sh, bridge_port_isolation.sh, bridge_locked_port.sh
  pass on both kernels; bridge_stp_mode.sh skips on both (iproute2
  without stp_mode support)
- W=1 build of net/bridge/ with allmodconfig: no warnings

v3:
- Re-test on current net (Ido).
- Update the FDB cleanup and VLAN group unpublish names for current net.
- Describe the FDB's reference to the port without the old dst == p
  expression.
- Remove the deprecated Assisted-by trailer and describe discovery and
  diagnostic instrumentation in plain text.
- Add the original report/reproducer link and links to v1 and v2.
- State the VLAN support prerequisite for the receive-path drain.

v2: https://lore.kernel.org/netdev/179333317814.2102764.8780283783548366593@gmail.com/
- Use Ido's MRP/CFM reorder instead of adding a second FDB flush.
- Validate the reorder with the v7.3 reproducer and custom regressions.

v1: https://lore.kernel.org/netdev/179268538303.70821.8213834342995341232@gmail.com/

 net/bridge/br_if.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 2c05ebc1299d..fb25ce3e5e64 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -341,13 +341,13 @@ static void del_nbp(struct net_bridge_port *p)
 
 	nbp_delete_promisc(p);
 
+	br_mrp_port_del(br, p);
+	br_cfm_port_del(br, p);
+
 	spin_lock_bh(&br->lock);
 	br_stp_disable_port(p);
 	spin_unlock_bh(&br->lock);
 
-	br_mrp_port_del(br, p);
-	br_cfm_port_del(br, p);
-
 	br_ifinfo_notify(RTM_DELLINK, NULL, p);
 
 	list_del_rcu(&p->list);

base-commit: cb284914e749388eef402938bdb3115fe1f6d337

Re: [PATCH net v3] net: bridge: disable MRP ports before the FDB flush in del_nbp()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 2 · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The fix leaves the use-after-free reachable when
  CONFIG_BRIDGE_VLAN_FILTERING=n.
- [High] The reorder only clears BR_MRP_AWARE_BIT when br_mrp_find_port()
  can still find the port in an MRP instance.

--

net: bridge: disable MRP ports before the FDB flush in del_nbp()

This patch moves br_mrp_port_del() and br_cfm_port_del() ahead of
br_stp_disable_port() in del_nbp(). The idea is to clear
BR_MRP_AWARE_BIT before the port is moved to BR_STATE_DISABLED. That
should stop frames received during port removal from learning FDB entries
that still point at the port after the final FDB flush.

Link: https://lore.kernel.org/netdev/20261012123051.GA714215@shredder/
Closes: https://lore.kernel.org/netdev/20261022090431.3641707-1-teel4res@gmail.com/

> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> index 2c05ebc1299d1..fb25ce3e5e646 100644
> --- a/net/bridge/br_if.c
> +++ b/net/bridge/br_if.c
> @@ -341,13 +341,13 @@ static void del_nbp(struct net_bridge_port *p)
>  
>  	nbp_delete_promisc(p);
>  
> +	br_mrp_port_del(br, p);
> +	br_cfm_port_del(br, p);
> +

[Severity: High]

Does this reorder cover a port that is still MRP-aware but is no longer
referenced by any MRP instance?

br_mrp_port_del() only clears BR_MRP_AWARE_BIT when br_mrp_find_port()
finds an instance whose p_port, s_port or i_port is this port:

br_mrp_port_del() {
	struct br_mrp *mrp = br_mrp_find_port(br, p);

	/* If the port is not part of a MRP instance just bail out */
	if (!mrp)
		return;
    ...
}

br_mrp_set_port_role() overwrites the role pointer. It does not clear the
displaced port's bit, and it does not swap the roles:

br_mrp_set_port_role() {
    ...
	switch (role) {
	case BR_MRP_PORT_ROLE_PRIMARY:
		rcu_assign_pointer(mrp->p_port, p);
		break;
	case BR_MRP_PORT_ROLE_SECONDARY:
		rcu_assign_pointer(mrp->s_port, p);
		break;
    ...
}

Take an instance with p_port=A and s_port=B. An IFLA_BRIDGE_MRP_PORT_ROLE
request that makes B primary leaves p_port == s_port == B. Port A stays in
BR_STATE_FORWARDING with BR_MRP_AWARE_BIT set. A follow-up request to make
A secondary fails with -EINVAL, because br_mrp_find_port(A) returns NULL.

When A is later removed through br_del_if()->del_nbp(), br_mrp_port_del()
returns early. br_stp_disable_port()->br_set_state() then still skips the
state change:

br_set_state() {
	if (test_bit(BR_MRP_AWARE_BIT, &p->flags))
		return;
    ...
}

With runtime VLAN filtering off, br_allowed_ingress() still returns true
after the VLAN group is unpublished. So br_handle_frame_finish() can keep
learning on A after the final br_fdb_cleanup_by_dst(), up until
netdev_rx_handler_unregister(). That window includes the
netdev_upper_dev_unlink() notifier chain that the reproducer delays. The
learned entries outlive destroy_nbp_rcu(), and a later frame to those MACs
would hit the same br_forward() use-after-free.

The commit message says the reorder means that "br_stp_disable_port()
actually transitions it to BR_STATE_DISABLED". Does that hold for this
state?

The role bookkeeping in br_mrp_set_port_role() predates this patch. Still,
should del_nbp() clear BR_MRP_AWARE_BIT (or force BR_STATE_DISABLED)
unconditionally? Or should br_mrp_set_port_role() be changed so it cannot
leave a port orphaned like this?

>  	spin_lock_bh(&br->lock);
>  	br_stp_disable_port(p);
>  	spin_unlock_bh(&br->lock);

[Severity: High]

Can the use-after-free still happen when CONFIG_BRIDGE_VLAN_FILTERING=n?

The commit message relies on nbp_vlan_group_unpublish() to drain the
receive path:

  With CONFIG_BRIDGE_VLAN_FILTERING=y, synchronize_net() in
  nbp_vlan_group_unpublish() also drains receive paths that could still
  learn with the old state before the final FDB cleanup.

When VLAN filtering is compiled out, that function is an empty stub in
br_private.h:

static inline void nbp_vlan_group_unpublish(struct net_bridge_port *port)
{
}

CONFIG_BRIDGE_MRP only depends on CONFIG_BRIDGE, and
CONFIG_BRIDGE_VLAN_FILTERING defaults to n. So a build with MRP=y and
VLAN_FILTERING=n is valid.

In that build, br_mrp_del_impl() sets the port back to BR_STATE_FORWARDING
(if the bridge is up) before it clears BR_MRP_AWARE_BIT. A reader in
br_handle_frame_finish() can read p->state without a lock, see FORWARDING,
and only reach br_fdb_update() later:

br_handle_frame_finish() {
    ...
		if (p->state == BR_STATE_DISABLED) {
			reason = SKB_DROP_REASON_BRIDGE_INGRESS_STP_STATE;
			goto drop;
		}

		state = p->state;
    ...
		br_fdb_update(br, p, vlan, eth_hdr(skb)->h_source, 0);
    ...
}

Between the DISABLED store and the final flush, del_nbp() never waits for
such a reader:

del_nbp()
  br_stp_disable_port()          /* p->state = BR_STATE_DISABLED */
  br_ifinfo_notify(RTM_DELLINK, ...)
  list_del_rcu(&p->list)
  nbp_vlan_group_unpublish(p)    /* empty stub, no synchronize_net() */
  br_fdb_cleanup_by_dst(br, br_port_to_dst(p), 0, 1)
  ...
  netdev_rx_handler_unregister(dev)
  call_rcu(&p->rcu, destroy_nbp_rcu)

br_fdb_update() does not check whether the source port is being removed.
If the reader gets to fdb_create() after br_fdb_cleanup_by_dst() has run,
the new entry points at the dying port. The same happens if the reader was
spinning on br->hash_lock while the cleanup held it.

netdev_rx_handler_unregister() only waits for the reader to finish. It
does not remove the entry. A later frame to the learned MAC would then
reach br_forward() through the freed port. In this configuration, the same
window seems to apply to ordinary ports as well.

Would an unconditional synchronize_net() between br_stp_disable_port() and
the final br_fdb_cleanup_by_dst() be needed here, instead of relying on a
side effect of VLAN filtering?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261101044840.1595584-1-1020691186%40qq.com

Re: [PATCH net v3] net: bridge: disable MRP ports before the FDB flush in del_nbp()

[Ido Schimmel]

On Sat, Oct 31, 2026 at 09:48:40PM -0700, xiaoblac wrote:
> From: Cheng Yongkang <teel4res@gmail.com>
> 
> del_nbp() flushes the port's FDB entries before it unregisters the RX
> handler. For a normal port, br_stp_disable_port() moves the port to
> BR_STATE_DISABLED before this cleanup.
> 
> That does not hold for an MRP ring port. br_set_state() ignores state
> changes while BR_MRP_AWARE_BIT is set, so br_stp_disable_port() leaves
> the port state alone. br_mrp_port_del() then restores forwarding when
> the bridge is up before clearing the bit. A frame received between
> br_fdb_cleanup_by_dst() and netdev_rx_handler_unregister() can therefore
> create a new FDB entry referencing the dying port. Nothing removes that
> entry before destroy_nbp_rcu() frees the port, so a later frame to the
> learned MAC can be forwarded through the freed port.
> 
> Remove the MRP and CFM instances before disabling the port, so that
> br_stp_disable_port() actually transitions it to BR_STATE_DISABLED.
> With CONFIG_BRIDGE_VLAN_FILTERING=y, synchronize_net() in
> nbp_vlan_group_unpublish() also drains receive paths that could still
> learn with the old state before the final FDB cleanup.
> 
> Found through an LLM-assisted source code analysis workflow and
> reproduced on net with KASAN and CONFIG_BRIDGE_VLAN_FILTERING=y in QEMU.
> A diagnostic switchdev module accepts MRP objects and delays the
> NETDEV_CHANGEUPPER callback for 1.5 s, holding del_nbp() after the FDB
> cleanup and before RX handler removal. The reproducer floods the MRP
> port with rotating source MACs, waits for the port's RCU callback
> through the module's rcu_barrier(), then sends frames to those MACs
> from another port. The unpatched kernel reports:
> 
>   BUG: KASAN: slab-use-after-free in br_forward+0xb1/0x820
>   Call Trace:
>    <IRQ>
>    br_forward+0xb1/0x820
>    br_handle_frame_finish+0x148b/0x1e50
>    br_handle_frame+0x90a/0x15c0
> 
> With this patch, three runs of six cycles each reach the delay without
> a KASAN report.
> 
> Fixes: 6536993371fa ("bridge: mrp: Integrate MRP into the bridge")
> Suggested-by: Ido Schimmel <idosch@nvidia.com>
> Link: https://lore.kernel.org/netdev/20261012123051.GA714215@shredder/
> Closes: https://lore.kernel.org/netdev/20261022090431.3641707-1-teel4res@gmail.com/
> Signed-off-by: Cheng Yongkang <teel4res@gmail.com>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

Re: [PATCH net v3] net: bridge: disable MRP ports before the FDB flush in del_nbp()

[Ido Schimmel]

On Mon, Nov 02, 2026 at 10:25:53AM +0200, Ido Schimmel wrote:
> On Sat, Oct 31, 2026 at 09:48:40PM -0700, xiaoblac wrote:
> > From: Cheng Yongkang <teel4res@gmail.com>
> > 
> > del_nbp() flushes the port's FDB entries before it unregisters the RX
> > handler. For a normal port, br_stp_disable_port() moves the port to
> > BR_STATE_DISABLED before this cleanup.
> > 
> > That does not hold for an MRP ring port. br_set_state() ignores state
> > changes while BR_MRP_AWARE_BIT is set, so br_stp_disable_port() leaves
> > the port state alone. br_mrp_port_del() then restores forwarding when
> > the bridge is up before clearing the bit. A frame received between
> > br_fdb_cleanup_by_dst() and netdev_rx_handler_unregister() can therefore
> > create a new FDB entry referencing the dying port. Nothing removes that
> > entry before destroy_nbp_rcu() frees the port, so a later frame to the
> > learned MAC can be forwarded through the freed port.
> > 
> > Remove the MRP and CFM instances before disabling the port, so that
> > br_stp_disable_port() actually transitions it to BR_STATE_DISABLED.
> > With CONFIG_BRIDGE_VLAN_FILTERING=y, synchronize_net() in
> > nbp_vlan_group_unpublish() also drains receive paths that could still
> > learn with the old state before the final FDB cleanup.
> > 
> > Found through an LLM-assisted source code analysis workflow and
> > reproduced on net with KASAN and CONFIG_BRIDGE_VLAN_FILTERING=y in QEMU.
> > A diagnostic switchdev module accepts MRP objects and delays the
> > NETDEV_CHANGEUPPER callback for 1.5 s, holding del_nbp() after the FDB
> > cleanup and before RX handler removal. The reproducer floods the MRP
> > port with rotating source MACs, waits for the port's RCU callback
> > through the module's rcu_barrier(), then sends frames to those MACs
> > from another port. The unpatched kernel reports:
> > 
> >   BUG: KASAN: slab-use-after-free in br_forward+0xb1/0x820
> >   Call Trace:
> >    <IRQ>
> >    br_forward+0xb1/0x820
> >    br_handle_frame_finish+0x148b/0x1e50
> >    br_handle_frame+0x90a/0x15c0
> > 
> > With this patch, three runs of six cycles each reach the delay without
> > a KASAN report.
> > 
> > Fixes: 6536993371fa ("bridge: mrp: Integrate MRP into the bridge")
> > Suggested-by: Ido Schimmel <idosch@nvidia.com>
> > Link: https://lore.kernel.org/netdev/20261012123051.GA714215@shredder/
> > Closes: https://lore.kernel.org/netdev/20261022090431.3641707-1-teel4res@gmail.com/
> > Signed-off-by: Cheng Yongkang <teel4res@gmail.com>
> 
> Reviewed-by: Ido Schimmel <idosch@nvidia.com>

I replied to Sashiko before tagging this patch, but looks like the mail
didn't make it to the list. The two issues that it flagged are separate
pre-existing issues. The current patch fixes a UAF and doesn't make the
situation worse.