net: bridge: fix vlan stats use-after-free on destruction

Syzbot reported a use-after-free of the global vlan context on port vlan
destruction. When I added per-port vlan stats I missed the fact that the
global vlan context can be freed before the per-port vlan rcu callback.
There're a few different ways to deal with this, I've chosen to add a
new private flag that is set only when per-port stats are allocated so
we can directly check it on destruction without dereferencing the global
context at all. The new field in net_bridge_vlan uses a hole.

v2: cosmetic change, move the check to br_process_vlan_info where the
    other checks are done
v3: add change log in the patch, add private (in-kernel only) flags in a
    hole in net_bridge_vlan struct and use that instead of mixing
    user-space flags with private flags

Fixes: 9163a0fc1f0c ("net: bridge: add support for per-port vlan stats")
Reported-by: syzbot+04681da557a0e49a52e5@syzkaller.appspotmail.com
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 2920e06a540329ffb8f1286c65bc5acc6f53afad..04c19a37e500e872d564497c9410509c0df61fce 100644 (file)
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -102,12 +102,18 @@ struct br_tunnel_info {
        struct metadata_dst     *tunnel_dst;
 };
 
+/* private vlan flags */
+enum {
+       BR_VLFLAG_PER_PORT_STATS = BIT(0),
+};
+
 /**
  * struct net_bridge_vlan - per-vlan entry
  *
  * @vnode: rhashtable member
  * @vid: VLAN id
  * @flags: bridge vlan flags
+ * @priv_flags: private (in-kernel) bridge vlan flags
  * @stats: per-cpu VLAN statistics
  * @br: if MASTER flag set, this points to a bridge struct
  * @port: if MASTER flag unset, this points to a port struct
@@ -127,6 +133,7 @@ struct net_bridge_vlan {
        struct rhash_head               tnode;
        u16                             vid;
        u16                             flags;
+       u16                             priv_flags;
        struct br_vlan_stats __percpu   *stats;
        union {
                struct net_bridge       *br;

diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 8c9297a019475fe68ee110a85cda1647deaf9075..e84be08b82854916d91bc0195ad2a9ab7dae8564 100644 (file)
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -197,7 +197,7 @@ static void nbp_vlan_rcu_free(struct rcu_head *rcu)
        v = container_of(rcu, struct net_bridge_vlan, rcu);
        WARN_ON(br_vlan_is_master(v));
        /* if we had per-port stats configured then free them here */
-       if (v->brvlan->stats != v->stats)
+       if (v->priv_flags & BR_VLFLAG_PER_PORT_STATS)
                free_percpu(v->stats);
        v->stats = NULL;
        kfree(v);
@@ -264,6 +264,7 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags)
                                err = -ENOMEM;
                                goto out_filt;
                        }
+                       v->priv_flags |= BR_VLFLAG_PER_PORT_STATS;
                } else {
                        v->stats = masterv->stats;
                }