netdev - Re: [PATCH net 00/11] net: fix nested device bugs

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]

Date:   Wed, 4 Sep 2019 11:58:39 -0700
From:   Stephen Hemminger <stephen@...workplumber.org>
To:     Taehee Yoo <ap420073@...il.com>
Cc:     davem@...emloft.net, netdev@...r.kernel.org, j.vosburgh@...il.com,
        vfalico@...il.com, andy@...yhouse.net, jiri@...nulli.us,
        sd@...asysnail.net, roopa@...ulusnetworks.com, saeedm@...lanox.com,
        manishc@...vell.com, rahulv@...vell.com, kys@...rosoft.com,
        haiyangz@...rosoft.com, sthemmin@...rosoft.com, sashal@...nel.org,
        hare@...e.de, varun@...lsio.com, ubraun@...ux.ibm.com,
        kgraul@...ux.ibm.com
Subject: Re: [PATCH net 00/11] net: fix nested device bugs

On Thu,  5 Sep 2019 03:38:28 +0900
Taehee Yoo <ap420073@...il.com> wrote:

> This patchset fixes several bugs that are related to nesting
> device infrastructure.
> Current nesting infrastructure code doesn't limit the depth level of
> devices. nested devices could be handled recursively. at that moment,
> it needs huge memory and stack overflow could occur.
> Below devices type have same bug.
> VLAN, BONDING, TEAM, MACSEC, MACVLAN and VXLAN.
> 
> Test commands:
>     ip link add dummy0 type dummy
>     ip link add vlan1 link dummy0 type vlan id 1
> 
>     for i in {2..100}
>     do
> 	    let A=$i-1
> 	    ip link add name vlan$i link vlan$A type vlan id $i
>     done
>     ip link del dummy0
> 
> 1st patch actually fixes the root cause.
> It adds new common variables {upper/lower}_level that represent
> depth level. upper_level variable is depth of upper devices.
> lower_level variable is depth of lower devices.
> 
>       [U][L]       [U][L]
> vlan1  1  5  vlan4  1  4
> vlan2  2  4  vlan5  2  3
> vlan3  3  3    |
>   |            |
>   +------------+
>   |
> vlan6  4  2
> dummy0 5  1
> 
> After this patch, the nesting infrastructure code uses this variable to
> check the depth level.
> 
> 2, 4, 5, 6, 7 patches fix lockdep related problem.
> Before this patch, devices use static lockdep map.
> So, if devices that are same type is nested, lockdep will warn about
> recursive situation.
> These patches make these devices use dynamic lockdep key instead of
> static lock or subclass.
> 
> 3rd patch splits IFF_BONDING flag into IFF_BONDING and IFF_BONDING_SLAVE.
> Before this patch, there is only IFF_BONDING flags, which means
> a bonding master or a bonding slave device.
> But this single flag could be problem when bonding devices are set to
> nested.
> 
> 8th patch fixes a refcnt leak in the macsec module.
> 
> 9th patch adds ignore flag to an adjacent structure.
> In order to exchange an adjacent node safely, ignore flag is needed.
> 
> 10th patch makes vxlan add an adjacent link to limit depth level.
> 
> 11th patch removes unnecessary variables and callback.
> 
> Taehee Yoo (11):
>   net: core: limit nested device depth
>   vlan: use dynamic lockdep key instead of subclass
>   bonding: split IFF_BONDING into IFF_BONDING and IFF_BONDING_SLAVE
>   bonding: use dynamic lockdep key instead of subclass
>   team: use dynamic lockdep key instead of static key
>   macsec: use dynamic lockdep key instead of subclass
>   macvlan: use dynamic lockdep key instead of subclass
>   macsec: fix refcnt leak in module exit routine
>   net: core: add ignore flag to netdev_adjacent structure
>   vxlan: add adjacent link to limit depth level
>   net: remove unnecessary variables and callback
> 
>  drivers/net/bonding/bond_alb.c                |   2 +-
>  drivers/net/bonding/bond_main.c               |  87 ++++--
>  .../net/ethernet/mellanox/mlx5/core/en_tc.c   |   2 +-
>  .../ethernet/qlogic/netxen/netxen_nic_main.c  |   2 +-
>  drivers/net/hyperv/netvsc_drv.c               |   3 +-
>  drivers/net/macsec.c                          |  50 ++--
>  drivers/net/macvlan.c                         |  36 ++-
>  drivers/net/team/team.c                       |  61 ++++-
>  drivers/net/vxlan.c                           |  71 ++++-
>  drivers/scsi/fcoe/fcoe.c                      |   2 +-
>  drivers/target/iscsi/cxgbit/cxgbit_cm.c       |   2 +-
>  include/linux/if_macvlan.h                    |   3 +-
>  include/linux/if_team.h                       |   5 +
>  include/linux/if_vlan.h                       |  13 +-
>  include/linux/netdevice.h                     |  29 +-
>  include/net/bonding.h                         |   4 +-
>  include/net/vxlan.h                           |   1 +
>  net/8021q/vlan.c                              |   1 -
>  net/8021q/vlan_dev.c                          |  32 +--
>  net/core/dev.c                                | 252 ++++++++++++++++--
>  net/core/dev_addr_lists.c                     |  12 +-
>  net/smc/smc_core.c                            |   2 +-
>  net/smc/smc_pnet.c                            |   2 +-
>  23 files changed, 519 insertions(+), 155 deletions(-)
> 

The network receive path already avoids excessive stack
depth. Maybe the real problem is in the lockdep code.