Commit 27973793 authored by Ido Schimmel's avatar Ido Schimmel Committed by David S. Miller

net: bridge: Fix VLANs memory leak

When adding / deleting VLANs to / from a bridge port, the bridge driver
first tries to propagate the information via switchdev and falls back to
the 8021q driver in case the underlying driver does not support
switchdev. This can result in a memory leak [1] when VXLAN and mlxsw
ports are enslaved to the bridge:

$ ip link set dev vxlan0 master br0
# No mlxsw ports are enslaved to 'br0', so mlxsw ignores the switchdev
# notification and the bridge driver adds the VLAN on 'vxlan0' via the
# 8021q driver
$ bridge vlan add vid 10 dev vxlan0 pvid untagged
# mlxsw port is enslaved to the bridge
$ ip link set dev swp1 master br0
# mlxsw processes the switchdev notification and the 8021q driver is
# skipped
$ bridge vlan del vid 10 dev vxlan0

This results in 'struct vlan_info' and 'struct vlan_vid_info' being
leaked, as they were allocated by the 8021q driver during VLAN addition,
but never freed as the 8021q driver was skipped during deletion.

Fix this by introducing a new VLAN private flag that indicates whether
the VLAN was added on the port by switchdev or the 8021q driver. If the
VLAN was added by the 8021q driver, then we make sure to delete it via
the 8021q driver as well.

[1]
unreferenced object 0xffff88822d20b1e8 (size 256):
  comm "bridge", pid 2532, jiffies 4295216998 (age 1188.830s)
  hex dump (first 32 bytes):
    e0 42 97 ce 81 88 ff ff 00 00 00 00 00 00 00 00  .B..............
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<00000000f82d851d>] kmem_cache_alloc_trace+0x1be/0x330
    [<00000000e0178b02>] vlan_vid_add+0x661/0x920
    [<00000000218ebd5f>] __vlan_add+0x1be9/0x3a00
    [<000000006eafa1ca>] nbp_vlan_add+0x8b3/0xd90
    [<000000003535392c>] br_vlan_info+0x132/0x410
    [<00000000aedaa9dc>] br_afspec+0x75c/0x870
    [<00000000f5716133>] br_setlink+0x3dc/0x6d0
    [<00000000aceca5e2>] rtnl_bridge_setlink+0x615/0xb30
    [<00000000a2f2d23e>] rtnetlink_rcv_msg+0x3a3/0xa80
    [<0000000064097e69>] netlink_rcv_skb+0x152/0x3c0
    [<000000008be8d614>] rtnetlink_rcv+0x21/0x30
    [<000000009ab2ca25>] netlink_unicast+0x52f/0x740
    [<00000000e7d9ac96>] netlink_sendmsg+0x9c7/0xf50
    [<000000005d1e2050>] sock_sendmsg+0xbe/0x120
    [<00000000d51426bc>] ___sys_sendmsg+0x778/0x8f0
    [<00000000b9d7b2cc>] __sys_sendmsg+0x112/0x270
unreferenced object 0xffff888227454308 (size 32):
  comm "bridge", pid 2532, jiffies 4295216998 (age 1188.882s)
  hex dump (first 32 bytes):
    88 b2 20 2d 82 88 ff ff 88 b2 20 2d 82 88 ff ff  .. -...... -....
    81 00 0a 00 01 00 00 00 00 00 00 00 00 00 00 00  ................
  backtrace:
    [<00000000f82d851d>] kmem_cache_alloc_trace+0x1be/0x330
    [<0000000018050631>] vlan_vid_add+0x3e6/0x920
    [<00000000218ebd5f>] __vlan_add+0x1be9/0x3a00
    [<000000006eafa1ca>] nbp_vlan_add+0x8b3/0xd90
    [<000000003535392c>] br_vlan_info+0x132/0x410
    [<00000000aedaa9dc>] br_afspec+0x75c/0x870
    [<00000000f5716133>] br_setlink+0x3dc/0x6d0
    [<00000000aceca5e2>] rtnl_bridge_setlink+0x615/0xb30
    [<00000000a2f2d23e>] rtnetlink_rcv_msg+0x3a3/0xa80
    [<0000000064097e69>] netlink_rcv_skb+0x152/0x3c0
    [<000000008be8d614>] rtnetlink_rcv+0x21/0x30
    [<000000009ab2ca25>] netlink_unicast+0x52f/0x740
    [<00000000e7d9ac96>] netlink_sendmsg+0x9c7/0xf50
    [<000000005d1e2050>] sock_sendmsg+0xbe/0x120
    [<00000000d51426bc>] ___sys_sendmsg+0x778/0x8f0
    [<00000000b9d7b2cc>] __sys_sendmsg+0x112/0x270

Fixes: d70e42b2 ("mlxsw: spectrum: Enable VxLAN enslavement to VLAN-aware bridges")
Signed-off-by: default avatarIdo Schimmel <idosch@mellanox.com>
Reviewed-by: default avatarPetr Machata <petrm@mellanox.com>
Cc: Roopa Prabhu <roopa@cumulusnetworks.com>
Cc: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Cc: bridge@lists.linux-foundation.org
Acked-by: default avatarNikolay Aleksandrov <nikolay@cumulusnetworks.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 16dc42e4
...@@ -107,6 +107,7 @@ struct br_tunnel_info { ...@@ -107,6 +107,7 @@ struct br_tunnel_info {
/* private vlan flags */ /* private vlan flags */
enum { enum {
BR_VLFLAG_PER_PORT_STATS = BIT(0), BR_VLFLAG_PER_PORT_STATS = BIT(0),
BR_VLFLAG_ADDED_BY_SWITCHDEV = BIT(1),
}; };
/** /**
......
...@@ -80,16 +80,18 @@ static bool __vlan_add_flags(struct net_bridge_vlan *v, u16 flags) ...@@ -80,16 +80,18 @@ static bool __vlan_add_flags(struct net_bridge_vlan *v, u16 flags)
} }
static int __vlan_vid_add(struct net_device *dev, struct net_bridge *br, static int __vlan_vid_add(struct net_device *dev, struct net_bridge *br,
u16 vid, u16 flags, struct netlink_ext_ack *extack) struct net_bridge_vlan *v, u16 flags,
struct netlink_ext_ack *extack)
{ {
int err; int err;
/* Try switchdev op first. In case it is not supported, fallback to /* Try switchdev op first. In case it is not supported, fallback to
* 8021q add. * 8021q add.
*/ */
err = br_switchdev_port_vlan_add(dev, vid, flags, extack); err = br_switchdev_port_vlan_add(dev, v->vid, flags, extack);
if (err == -EOPNOTSUPP) if (err == -EOPNOTSUPP)
return vlan_vid_add(dev, br->vlan_proto, vid); return vlan_vid_add(dev, br->vlan_proto, v->vid);
v->priv_flags |= BR_VLFLAG_ADDED_BY_SWITCHDEV;
return err; return err;
} }
...@@ -121,19 +123,17 @@ static void __vlan_del_list(struct net_bridge_vlan *v) ...@@ -121,19 +123,17 @@ static void __vlan_del_list(struct net_bridge_vlan *v)
} }
static int __vlan_vid_del(struct net_device *dev, struct net_bridge *br, static int __vlan_vid_del(struct net_device *dev, struct net_bridge *br,
u16 vid) const struct net_bridge_vlan *v)
{ {
int err; int err;
/* Try switchdev op first. In case it is not supported, fallback to /* Try switchdev op first. In case it is not supported, fallback to
* 8021q del. * 8021q del.
*/ */
err = br_switchdev_port_vlan_del(dev, vid); err = br_switchdev_port_vlan_del(dev, v->vid);
if (err == -EOPNOTSUPP) { if (!(v->priv_flags & BR_VLFLAG_ADDED_BY_SWITCHDEV))
vlan_vid_del(dev, br->vlan_proto, vid); vlan_vid_del(dev, br->vlan_proto, v->vid);
return 0; return err == -EOPNOTSUPP ? 0 : err;
}
return err;
} }
/* Returns a master vlan, if it didn't exist it gets created. In all cases a /* Returns a master vlan, if it didn't exist it gets created. In all cases a
...@@ -242,7 +242,7 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags, ...@@ -242,7 +242,7 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags,
* This ensures tagged traffic enters the bridge when * This ensures tagged traffic enters the bridge when
* promiscuous mode is disabled by br_manage_promisc(). * promiscuous mode is disabled by br_manage_promisc().
*/ */
err = __vlan_vid_add(dev, br, v->vid, flags, extack); err = __vlan_vid_add(dev, br, v, flags, extack);
if (err) if (err)
goto out; goto out;
...@@ -305,7 +305,7 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags, ...@@ -305,7 +305,7 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags,
out_filt: out_filt:
if (p) { if (p) {
__vlan_vid_del(dev, br, v->vid); __vlan_vid_del(dev, br, v);
if (masterv) { if (masterv) {
if (v->stats && masterv->stats != v->stats) if (v->stats && masterv->stats != v->stats)
free_percpu(v->stats); free_percpu(v->stats);
...@@ -338,7 +338,7 @@ static int __vlan_del(struct net_bridge_vlan *v) ...@@ -338,7 +338,7 @@ static int __vlan_del(struct net_bridge_vlan *v)
__vlan_delete_pvid(vg, v->vid); __vlan_delete_pvid(vg, v->vid);
if (p) { if (p) {
err = __vlan_vid_del(p->dev, p->br, v->vid); err = __vlan_vid_del(p->dev, p->br, v);
if (err) if (err)
goto out; goto out;
} else { } else {
......
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