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

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

[Norbert Szetei]

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.

  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: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5
Signed-off-by: Norbert Szetei <norbert@doyensec.com>
---
Changes in v2:
- drop the paragraph above the splat, per review
- no functional change

Reproducer available on request.

v1: https://lore.kernel.org/netdev/B41EB55B-E5FC-431D-956C-503CA7B95C30@doyensec.com/
 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;
 
-- 
2.55.0

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

[Nikolay Aleksandrov]

On 26/08/2026 12:12, 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.
> 
>    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: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Norbert Szetei <norbert@doyensec.com>
> ---
> Changes in v2:
> - drop the paragraph above the splat, per review
> - no functional change
> 
> Reproducer available on request.
> 
> v1: https://lore.kernel.org/netdev/B41EB55B-E5FC-431D-956C-503CA7B95C30@doyensec.com/
>   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;
>   

Thanks,
Acked-by: Nikolay Aleksandrov <razor@blackwall.org>

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

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 26 Aug 2026 11:12:27 +0200 you 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.
> 
> [...]

Here is the summary with links:
  - [net,v2] net: bridge: mcast: fix use-after-free of a master VLAN's multicast context
    https://git.kernel.org/netdev/net/c/50e5c6605cc9

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html