Re: [PATCH net] net: bridge: mcast: fix use-after-free of a master VLAN's multicast context

Nikolay Aleksandrov <[email protected]>
Newsgroups org.kernel.vger.netdev,dev.linux.lists.bridge,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
On 23/08/2026 14:58, Norbert Szetei wrote:
> br_multicast_toggle_one_vlan() clears BR_VLFLAG_MCAST_ENABLED under
> br->multicast_lock before stopping a VLAN's multicast context.  That is
> the teardown handshake: lockless readers gate on the flag through
> br_multicast_ctx_should_use() -> br_multicast_ctx_vlan_disabled(), so
> once it is cleared under the lock no reader can arm the context again.
> 
> For a master VLAN the handshake never runs.  __vlan_del() clears
> BRIDGE_VLAN_INFO_BRENTRY before calling br_vlan_put_master(), so
> br_multicast_toggle_one_vlan(masterv, false) returns early on
> !br_vlan_is_brentry(vlan): the flag stays set and br->multicast_lock is
> never taken.  br_vlan_put_master() then drains the context in
> br_multicast_ctx_deinit() and frees the VLAN through call_rcu(), while a
> reader still inside rcu_read_lock() sees the context as enabled and
> re-arms it.  The port and port-VLAN branch of the function has no
> br_vlan_is_brentry() test and flips the flag under br->multicast_lock,
> so it is not affected.
> 
> The reader is the bridge transmit path.  For a master VLAN
> br_multicast_rcv() selects brmctx = &vlan->br_mcast_ctx with
> pmctx = NULL, so IGMP sent to the bridge device re-arms the context's
> timers after br_multicast_ctx_deinit() has already stopped them.
> 
> Any user able to create a bridge can trigger this, which includes an
> unprivileged user in a user namespace, by deleting and re-adding the
> master VLAN while sending IGMP to the bridge device:
> 

So much AI slop... at least drop this paragraph which is completely useless.

>    BUG: KASAN: slab-use-after-free in detach_if_pending+0x412/0x4a0
>    Write of size 8 at addr ffff88810ac39918 by task brmc/601
>     __mod_timer+0x51a/0xc50
>     br_multicast_host_join+0x25b/0x390
>     __br_multicast_add_group+0x468/0x530
>     br_ip4_multicast_add_group+0x1a0/0x260
>     br_multicast_rcv+0x2cda/0x61e0
>     br_dev_xmit+0x6c4/0x1540
>    Allocated by task 610:
>     br_vlan_add+0x111/0xb40
>     br_vlan_info+0x370/0x3e0
>    Freed by task 0:
>     kfree+0x1a7/0x4f0
>     rcu_core+0x7dc/0x10a0
> 
> Only test br_vlan_is_brentry() when enabling, like the
> br_multicast_ctx_vlan_global_disabled() test next to it.  Disabling then
> always clears BR_VLFLAG_MCAST_ENABLED under br->multicast_lock before
> br_multicast_ctx_deinit() drains the context.
> 
> Fixes: 7b54aaaf53cb ("net: bridge: multicast: add vlan state initialization and control")
> Cc: [email protected]
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Norbert Szetei <[email protected]>
> ---
>   net/bridge/br_multicast.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index 75e1e2a8fc83..3ef5d8bbf552 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -4377,8 +4377,8 @@ void br_multicast_toggle_one_vlan(struct net_bridge_vlan *vlan, bool on)
>   	if (br_vlan_is_master(vlan)) {
>   		br = vlan->br;
>   
> -		if (!br_vlan_is_brentry(vlan) ||
> -		    (on &&
> +		if (on &&
> +		    (!br_vlan_is_brentry(vlan) ||
>   		     br_multicast_ctx_vlan_global_disabled(&vlan->br_mcast_ctx)))
>   			return;
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.