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

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

[Cheng Yongkang]

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

del_nbp() flushes the port's FDB entries before it unregisters the RX
handler.  For a normal port, br_stp_disable_port() has already moved the
port to BR_STATE_DISABLED, so no frame is learned in between.

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, and br_mrp_port_del() then sets the port to
BR_STATE_FORWARDING (when the bridge is up) before clearing the bit.  A
frame received between br_fdb_delete_by_port() and
netdev_rx_handler_unregister() is learned with dst == p.  Nothing
removes that entry before destroy_nbp_rcu() frees the port, and a later
frame to the learned MAC is forwarded to the freed port:

  BUG: KASAN: slab-use-after-free in br_forward+0x55/0x3c0
  Read of size 8 at addr ffff8880200bf038 by task mrp-poc/9420
  Call Trace:
   <IRQ>
   br_forward+0x55/0x3c0
   br_handle_frame_finish+0x1300/0x1d40
   br_handle_frame+0x90a/0x15c0

Remove the MRP and CFM instances before disabling the port, so that
br_stp_disable_port() actually transitions the port to BR_STATE_DISABLED.
The synchronize_net() in nbp_vlan_flush() then drains the receive paths
that could still learn with the old state, and the FDB flush that
follows removes their entries.  No frame can be learned after that
flush and before netdev_rx_handler_unregister().

Tested on v7.3 with KASAN and a reproducer that holds del_nbp() in the
post-FDB-flush/pre-unregister window for 1.5 s while a flood keeps
sending frames.  The unpatched kernel reports the KASAN use-after-free
above.  With this change the same window is exercised in six cycles and
no KASAN report is produced; normal bridge forwarding, FDB learning and
port detach also pass.

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/
Assisted-by: LLM
Signed-off-by: Cheng Yongkang <teel4res@gmail.com>
---
 net/bridge/br_if.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

v2:
- Implement the reorder suggested by Ido instead of adding a second FDB
  flush after netdev_rx_handler_unregister().
- The reproducer still exercises the post-flush window; the unpatched
  kernel reports the KASAN use-after-free and the patched kernel does
  not.
- Regression-tested normal bridge forwarding, FDB learning and port
  detach.

diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index c52613431f88..45642b6a5866 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -340,13 +340,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);

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

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

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

[Ido Schimmel]

Diff itself looks OK, but the commit message needs to be adjusted since
it no longer matches current net.git. See below. Please re-test on
current net.git and wait at least 24h before posting v3.

On Thu, Oct 29, 2026 at 09:06:18PM -0700, Cheng Yongkang wrote:
> net: bridge: disable MRP ports before the FDB flush in del_nbp()
> 
> del_nbp() flushes the port's FDB entries before it unregisters the RX
> handler.  For a normal port, br_stp_disable_port() has already moved the
> port to BR_STATE_DISABLED, so no frame is learned in between.
> 
> 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, and br_mrp_port_del() then sets the port to
> BR_STATE_FORWARDING (when the bridge is up) before clearing the bit.  A
> frame received between br_fdb_delete_by_port() and

s/br_fdb_delete_by_port/br_fdb_cleanup_by_dst/

> netdev_rx_handler_unregister() is learned with dst == p.  Nothing
> removes that entry before destroy_nbp_rcu() frees the port, and a later
> frame to the learned MAC is forwarded to the freed port:
> 
>   BUG: KASAN: slab-use-after-free in br_forward+0x55/0x3c0
>   Read of size 8 at addr ffff8880200bf038 by task mrp-poc/9420
>   Call Trace:
>    <IRQ>
>    br_forward+0x55/0x3c0
>    br_handle_frame_finish+0x1300/0x1d40
>    br_handle_frame+0x90a/0x15c0
> 
> Remove the MRP and CFM instances before disabling the port, so that
> br_stp_disable_port() actually transitions the port to BR_STATE_DISABLED.
> The synchronize_net() in nbp_vlan_flush() then drains the receive paths

s/nbp_vlan_flush/nbp_vlan_group_unpublish/

> that could still learn with the old state, and the FDB flush that
> follows removes their entries.  No frame can be learned after that
> flush and before netdev_rx_handler_unregister().
> 
> Tested on v7.3 with KASAN and a reproducer that holds del_nbp() in the
> post-FDB-flush/pre-unregister window for 1.5 s while a flood keeps
> sending frames.  The unpatched kernel reports the KASAN use-after-free
> above.  With this change the same window is exercised in six cycles and
> no KASAN report is produced; normal bridge forwarding, FDB learning and
> port detach also pass.
> 
> 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/
> Assisted-by: LLM

This tag is deprecated since commit 1e035297ffda ("docs: remove
Assisted-by references").

> Signed-off-by: Cheng Yongkang <teel4res@gmail.com>
> ---
>  net/bridge/br_if.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> v2:
> - Implement the reorder suggested by Ido instead of adding a second FDB
>   flush after netdev_rx_handler_unregister().
> - The reproducer still exercises the post-flush window; the unpatched
>   kernel reports the KASAN use-after-free and the patched kernel does
>   not.
> - Regression-tested normal bridge forwarding, FDB learning and port
>   detach.

Please also include a link to v1.

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

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

[netdev-bot+sashiko]

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

Critical: 0 · High: 2 · Medium: 1 · 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 needs an RCU grace period between br_stp_disable_port()
  writing p->state = BR_STATE_DISABLED (br_if.c:348) and the final…
- [High] The reorder assumes br_mrp_port_del() always clears
  BR_MRP_AWARE_BIT on the port being deleted, so that…
- [Medium] The commit message's safety argument describes code that is
  not in the target tree.

--

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(). An MRP ring port then has
BR_MRP_AWARE_BIT cleared before STP disables it. The goal is for the port
to really reach BR_STATE_DISABLED before the FDB flush, so that no frame
is learned with dst == p and later forwarded to the freed port.

Link: https://lore.kernel.org/netdev/20261012123051.GA714215@shredder/

> Remove the MRP and CFM instances before disabling the port, so that
> br_stp_disable_port() actually transitions the port to BR_STATE_DISABLED.
> The synchronize_net() in nbp_vlan_flush() then drains the receive paths
> that could still learn with the old state, and the FDB flush that
> follows removes their entries.  No frame can be learned after that
> flush and before netdev_rx_handler_unregister().

[Severity: Medium]
Does this description match the tree the patch applies to?

Earlier in the message the flush is called br_fdb_delete_by_port(), but
that function no longer exists under net/bridge/. The current del_nbp()
does this:

	vg = nbp_vlan_group(p);
	nbp_vlan_group_unpublish(p);
	br_fdb_cleanup_by_dst(br, br_port_to_dst(p), 0, 1);
	nbp_vlan_flush(p, vg);

nbp_vlan_flush() contains no synchronize_net(), and it runs after the FDB
flush:

net/bridge/br_vlan.c:nbp_vlan_flush() {
	ASSERT_RTNL();

	__vlan_flush(port->br, port, vg);
	__vlan_group_free(vg);
}

The only grace period before the flush is in nbp_vlan_group_unpublish():

net/bridge/br_vlan.c:nbp_vlan_group_unpublish() {
	ASSERT_RTNL();

	RCU_INIT_POINTER(port->vlgrp, NULL);
	synchronize_net();
}

This layout came from commit 83edc87a9172 ("net: bridge: vlan: quiesce
readers before freeing port VLANs"). The testing in the changelog was
done on v7.3, which still has the older ordering.

Could the message name nbp_vlan_group_unpublish() and
br_fdb_cleanup_by_dst() instead, or describe both layouts if a stable
backport is intended? Could the patch also be tested on the target tree?

> 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 br_mrp_port_del() always clear BR_MRP_AWARE_BIT on the port being
deleted?

br_mrp_set_port_role() can leave a port MRP-aware with no instance
pointing at it. Take a ring with primary A and secondary B. Setting
IFLA_BRIDGE_MRP_PORT_ROLE=PRIMARY on B is accepted, because B is
MRP-aware and br_mrp_find_port() finds the instance:

net/bridge/br_mrp.c:br_mrp_set_port_role() {
	...
	case BR_MRP_PORT_ROLE_PRIMARY:
		rcu_assign_pointer(mrp->p_port, p);
		break;
	...
}

Now p_port == s_port == B. A keeps BR_MRP_AWARE_BIT and stays in the
BR_STATE_FORWARDING state that br_mrp_add() set.

Later, when A is removed from the bridge, br_mrp_find_port(br, A) checks
only p_port, s_port and i_port. So br_mrp_port_del() returns early:

	if (!mrp)
		return;

br_stp_disable_port()->br_set_state() also returns early:

	if (test_bit(BR_MRP_AWARE_BIT, &p->flags))
		return;

As a result, A stays FORWARDING.

If VLAN filtering is disabled at runtime, br_allowed_ingress() returns
true without looking at the unpublished VLAN group. A frame that arrives
between br_fdb_cleanup_by_dst() and netdev_rx_handler_unregister() is then
learned with dst == A. Nothing flushes that entry before
call_rcu(&p->rcu, destroy_nbp_rcu). A later frame to that MAC would reach
br_forward() with the freed port.

This looks like the same use-after-free as the one in the changelog. It
needs no reader preemption, only traffic during the teardown window.

Should teardown force the port out of MRP-aware mode, or disable it
directly, even when no instance references it? Alternatively, should
br_mrp_set_port_role() avoid orphaning the port it displaces?

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

[Severity: High]
Is there still a window here with CONFIG_BRIDGE_VLAN_FILTERING=n?

The fix needs a grace period between br_stp_disable_port() writing
BR_STATE_DISABLED and the final br_fdb_cleanup_by_dst(br,
br_port_to_dst(p), 0, 1) in del_nbp(). The only synchronize_net() in that
window is in nbp_vlan_group_unpublish(). Without VLAN filtering, that
function is an empty stub:

net/bridge/br_private.h:
static inline void nbp_vlan_group_unpublish(struct net_bridge_port *port)
{
}

Nothing else in the window synchronizes. That includes br_ifinfo_notify(),
list_del_rcu(), update_headroom() and netdev_reset_rx_headroom().

One possible interleaving:

CPU1 (RX softirq, br_handle_frame_finish())
    reads p->state and sees FORWARDING
    passes the BR_STATE_DISABLED check

CPU0 (del_nbp())
    br_stp_disable_port()
        br_set_state(p, BR_STATE_DISABLED)
    br_fdb_cleanup_by_dst(br, br_port_to_dst(p), 0, 1)

CPU1
    br_fdb_update(br, p, vlan, eth_hdr(skb)->h_source, 0)
        fdb_create(br, dst, addr, vid, flags)   /* dst == p */

br_fdb_update() does not check the port state again. No FDB flush runs
after this point: netdev_rx_handler_unregister() only drains, and
destroy_nbp() does no FDB cleanup. After call_rcu(destroy_nbp_rcu), a
frame to the learned MAC would reach br_forward() with the freed port.

BRIDGE_MRP and BRIDGE_CFM depend only on BRIDGE. BRIDGE_VLAN_FILTERING is
default n and depends on VLAN_8021Q. So MRP=y with VLAN_FILTERING=n is a
valid configuration. The window is short on !PREEMPT_RT, but on PREEMPT_RT
the softirq reader can be preempted inside it.

This also seems to contradict this line in the changelog: "No frame can
be learned after that flush and before netdev_rx_handler_unregister()".

Would an unconditional synchronize_net() after br_stp_disable_port() and
before the final FDB flush close this window?

>  
> -	br_mrp_port_del(br, p);
> -	br_cfm_port_del(br, p);
> -
>  	br_ifinfo_notify(RTM_DELLINK, NULL, p);
>  
>  	list_del_rcu(&p->list);

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/179333317814.2102764.8780283783548366593%40gmail.com