Skip to content

Commit 8dca369

Browse files
NikAleksandrovkuba-moo
authored andcommitted
net: bridge: fix use-after-free due to MST port state bypass
syzbot reported[1] a use-after-free when deleting an expired fdb. It is due to a race condition between learning still happening and a port being deleted, after all its fdbs have been flushed. The port's state has been toggled to disabled so no learning should happen at that time, but if we have MST enabled, it will bypass the port's state, that together with VLAN filtering disabled can lead to fdb learning at a time when it shouldn't happen while the port is being deleted. VLAN filtering must be disabled because we flush the port VLANs when it's being deleted which will stop learning. This fix adds a check for the port's vlan group which is initialized to NULL when the port is getting deleted, that avoids the port state bypass. When MST is enabled there would be a minimal new overhead in the fast-path because the port's vlan group pointer is cache-hot. [1] https://syzkaller.appspot.com/bug?extid=dd280197f0f7ab3917be Fixes: ec7328b ("net: bridge: mst: Multiple Spanning Tree (MST) mode") Reported-by: syzbot+dd280197f0f7ab3917be@syzkaller.appspotmail.com Closes: https://lore.kernel.org/netdev/69088ffa.050a0220.29fc44.003d.GAE@google.com/ Signed-off-by: Nikolay Aleksandrov <razor@blackwall.org> Reviewed-by: Ido Schimmel <idosch@nvidia.com> Link: https://patch.msgid.link/20251105111919.1499702-2-razor@blackwall.org Signed-off-by: Jakub Kicinski <kuba@kernel.org>
1 parent 0216721 commit 8dca369

3 files changed

Lines changed: 8 additions & 6 deletions

File tree

net/bridge/br_forward.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ static inline int should_deliver(const struct net_bridge_port *p,
2525

2626
vg = nbp_vlan_group_rcu(p);
2727
return ((p->flags & BR_HAIRPIN_MODE) || skb->dev != p->dev) &&
28-
(br_mst_is_enabled(p->br) || p->state == BR_STATE_FORWARDING) &&
28+
(br_mst_is_enabled(p) || p->state == BR_STATE_FORWARDING) &&
2929
br_allowed_egress(vg, skb) && nbp_switchdev_allowed_egress(p, skb) &&
3030
!br_skb_isolated(p, skb);
3131
}

net/bridge/br_input.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
9494

9595
br = p->br;
9696

97-
if (br_mst_is_enabled(br)) {
97+
if (br_mst_is_enabled(p)) {
9898
state = BR_STATE_FORWARDING;
9999
} else {
100100
if (p->state == BR_STATE_DISABLED) {
@@ -429,7 +429,7 @@ static rx_handler_result_t br_handle_frame(struct sk_buff **pskb)
429429
return RX_HANDLER_PASS;
430430

431431
forward:
432-
if (br_mst_is_enabled(p->br))
432+
if (br_mst_is_enabled(p))
433433
goto defer_stp_filtering;
434434

435435
switch (p->state) {

net/bridge/br_private.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1935,10 +1935,12 @@ static inline bool br_vlan_state_allowed(u8 state, bool learn_allow)
19351935
/* br_mst.c */
19361936
#ifdef CONFIG_BRIDGE_VLAN_FILTERING
19371937
DECLARE_STATIC_KEY_FALSE(br_mst_used);
1938-
static inline bool br_mst_is_enabled(struct net_bridge *br)
1938+
static inline bool br_mst_is_enabled(const struct net_bridge_port *p)
19391939
{
1940+
/* check the port's vlan group to avoid racing with port deletion */
19401941
return static_branch_unlikely(&br_mst_used) &&
1941-
br_opt_get(br, BROPT_MST_ENABLED);
1942+
br_opt_get(p->br, BROPT_MST_ENABLED) &&
1943+
rcu_access_pointer(p->vlgrp);
19421944
}
19431945

19441946
int br_mst_set_state(struct net_bridge_port *p, u16 msti, u8 state,
@@ -1953,7 +1955,7 @@ int br_mst_fill_info(struct sk_buff *skb,
19531955
int br_mst_process(struct net_bridge_port *p, const struct nlattr *mst_attr,
19541956
struct netlink_ext_ack *extack);
19551957
#else
1956-
static inline bool br_mst_is_enabled(struct net_bridge *br)
1958+
static inline bool br_mst_is_enabled(const struct net_bridge_port *p)
19571959
{
19581960
return false;
19591961
}

0 commit comments

Comments
 (0)