All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ido Schimmel <idosch@nvidia.com>
To: Cheng Yongkang <teel4res@gmail.com>
Cc: Nikolay Aleksandrov <razor@blackwall.org>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@kernel.org>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Simon Horman <horms@kernel.org>,
	Horatiu Vultur <horatiu.vultur@microchip.com>,
	bridge@lists.linux.dev, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH net v2] net: bridge: disable MRP ports before the FDB flush in del_nbp()
Date: Fri, 30 Oct 2026 16:25:52 +0200	[thread overview]
Message-ID: <20261030142552.GA53460@shredder> (raw)
In-Reply-To: <179333317814.2102764.8780283783548366593@gmail.com>

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/

      parent reply	other threads:[~2026-10-30 14:26 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-30  4:06 [PATCH net v2] net: bridge: disable MRP ports before the FDB flush in del_nbp() Cheng Yongkang
2026-10-30  4:09 ` netdev-bot+sinfo
2026-10-30 14:25 ` Ido Schimmel [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261030142552.GA53460@shredder \
    --to=idosch@nvidia.com \
    --cc=bridge@lists.linux.dev \
    --cc=davem@davemloft.net \
    --cc=edumazet@kernel.org \
    --cc=horatiu.vultur@microchip.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=razor@blackwall.org \
    --cc=teel4res@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.