Commit 9d332e69 authored by Nikolay Aleksandrov's avatar Nikolay Aleksandrov Committed by David S. Miller

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: 9163a0fc ("net: bridge: add support for per-port vlan stats")
Reported-by: syzbot+04681da557a0e49a52e5@syzkaller.appspotmail.com
Signed-off-by: default avatarNikolay Aleksandrov <nikolay@cumulusnetworks.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 95506588
...@@ -102,12 +102,18 @@ struct br_tunnel_info { ...@@ -102,12 +102,18 @@ struct br_tunnel_info {
struct metadata_dst *tunnel_dst; struct metadata_dst *tunnel_dst;
}; };
/* private vlan flags */
enum {
BR_VLFLAG_PER_PORT_STATS = BIT(0),
};
/** /**
* struct net_bridge_vlan - per-vlan entry * struct net_bridge_vlan - per-vlan entry
* *
* @vnode: rhashtable member * @vnode: rhashtable member
* @vid: VLAN id * @vid: VLAN id
* @flags: bridge vlan flags * @flags: bridge vlan flags
* @priv_flags: private (in-kernel) bridge vlan flags
* @stats: per-cpu VLAN statistics * @stats: per-cpu VLAN statistics
* @br: if MASTER flag set, this points to a bridge struct * @br: if MASTER flag set, this points to a bridge struct
* @port: if MASTER flag unset, this points to a port struct * @port: if MASTER flag unset, this points to a port struct
...@@ -127,6 +133,7 @@ struct net_bridge_vlan { ...@@ -127,6 +133,7 @@ struct net_bridge_vlan {
struct rhash_head tnode; struct rhash_head tnode;
u16 vid; u16 vid;
u16 flags; u16 flags;
u16 priv_flags;
struct br_vlan_stats __percpu *stats; struct br_vlan_stats __percpu *stats;
union { union {
struct net_bridge *br; struct net_bridge *br;
......
...@@ -197,7 +197,7 @@ static void nbp_vlan_rcu_free(struct rcu_head *rcu) ...@@ -197,7 +197,7 @@ static void nbp_vlan_rcu_free(struct rcu_head *rcu)
v = container_of(rcu, struct net_bridge_vlan, rcu); v = container_of(rcu, struct net_bridge_vlan, rcu);
WARN_ON(br_vlan_is_master(v)); WARN_ON(br_vlan_is_master(v));
/* if we had per-port stats configured then free them here */ /* 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); free_percpu(v->stats);
v->stats = NULL; v->stats = NULL;
kfree(v); kfree(v);
...@@ -264,6 +264,7 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags) ...@@ -264,6 +264,7 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags)
err = -ENOMEM; err = -ENOMEM;
goto out_filt; goto out_filt;
} }
v->priv_flags |= BR_VLFLAG_PER_PORT_STATS;
} else { } else {
v->stats = masterv->stats; v->stats = masterv->stats;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment