net: bridge: fix use-after-free due to MST port state bypass

[ Upstream commit 8dca36978aa80bab9d4da130c211db75c9e00048 ]

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: ec7328b59176 ("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>
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index 870bdf2e082c4dd9500a9c3ea33801e35ec24af2..dea09096ad0fbaec04e125b73d3dca009b24e95b 100644 (file)
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -25,7 +25,7 @@ static inline int should_deliver(const struct net_bridge_port *p,
 
        vg = nbp_vlan_group_rcu(p);
        return ((p->flags & BR_HAIRPIN_MODE) || skb->dev != p->dev) &&
-               (br_mst_is_enabled(p->br) || p->state == BR_STATE_FORWARDING) &&
+               (br_mst_is_enabled(p) || p->state == BR_STATE_FORWARDING) &&
                br_allowed_egress(vg, skb) && nbp_switchdev_allowed_egress(p, skb) &&
                !br_skb_isolated(p, skb);
 }

diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 5f6ac9bf15275da6a4ec69eb44a971b87cb767b7..c07265b365b10424a7297cdc324bd79d71947388 100644 (file)
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -94,7 +94,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb
 
        br = p->br;
 
-       if (br_mst_is_enabled(br)) {
+       if (br_mst_is_enabled(p)) {
                state = BR_STATE_FORWARDING;
        } else {
                if (p->state == BR_STATE_DISABLED) {
@@ -421,7 +421,7 @@ static rx_handler_result_t br_handle_frame(struct sk_buff **pskb)
                return RX_HANDLER_PASS;
 
 forward:
-       if (br_mst_is_enabled(p->br))
+       if (br_mst_is_enabled(p))
                goto defer_stp_filtering;
 
        switch (p->state) {

diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 8de0904b9627f79556f5b7a40d3b3c00154a55c8..fdaf7d8374639cdfe6e6e662e4883bedc7451efa 100644 (file)
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -1932,10 +1932,12 @@ static inline bool br_vlan_state_allowed(u8 state, bool learn_allow)
 /* br_mst.c */
 #ifdef CONFIG_BRIDGE_VLAN_FILTERING
 DECLARE_STATIC_KEY_FALSE(br_mst_used);
-static inline bool br_mst_is_enabled(struct net_bridge *br)
+static inline bool br_mst_is_enabled(const struct net_bridge_port *p)
 {
+       /* check the port's vlan group to avoid racing with port deletion */
        return static_branch_unlikely(&br_mst_used) &&
-               br_opt_get(br, BROPT_MST_ENABLED);
+              br_opt_get(p->br, BROPT_MST_ENABLED) &&
+              rcu_access_pointer(p->vlgrp);
 }
 
 int br_mst_set_state(struct net_bridge_port *p, u16 msti, u8 state,
@@ -1950,7 +1952,7 @@ int br_mst_fill_info(struct sk_buff *skb,
 int br_mst_process(struct net_bridge_port *p, const struct nlattr *mst_attr,
                   struct netlink_ext_ack *extack);
 #else
-static inline bool br_mst_is_enabled(struct net_bridge *br)
+static inline bool br_mst_is_enabled(const struct net_bridge_port *p)
 {
        return false;
 }