netdev - Re: [PATCH net 08/11] net: bridge: Fix VLANs memory leak

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <db6d7137-28d2-795d-34fd-58b61f70b027@cumulusnetworks.com>
Date:   Tue, 8 Jan 2019 20:33:57 +0200
From:   Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
To:     Ido Schimmel <idosch@...lanox.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Cc:     "davem@...emloft.net" <davem@...emloft.net>,
        Jiri Pirko <jiri@...lanox.com>,
        Petr Machata <petrm@...lanox.com>,
        Nir Dotan <nird@...lanox.com>, mlxsw <mlxsw@...lanox.com>,
        Roopa Prabhu <roopa@...ulusnetworks.com>,
        "bridge@...ts.linux-foundation.org" 
        <bridge@...ts.linux-foundation.org>
Subject: Re: [PATCH net 08/11] net: bridge: Fix VLANs memory leak

On 08/01/2019 18:48, Ido Schimmel wrote:
> 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: d70e42b22dd4 ("mlxsw: spectrum: Enable VxLAN enslavement to VLAN-aware bridges")
> Signed-off-by: Ido Schimmel <idosch@...lanox.com>
> Reviewed-by: Petr Machata <petrm@...lanox.com>
> Cc: Roopa Prabhu <roopa@...ulusnetworks.com>
> Cc: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
> Cc: bridge@...ts.linux-foundation.org
> ---
>  net/bridge/br_private.h |  1 +
>  net/bridge/br_vlan.c    | 26 +++++++++++++-------------
>  2 files changed, 14 insertions(+), 13 deletions(-)

Acked-by: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>