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] [day] [month] [year] [list]
Date:   Thu, 5 Sep 2019 18:15:19 +0900
From:   Taehee Yoo <ap420073@...il.com>
To:     Stephen Hemminger <stephen@...workplumber.org>
Cc:     David Miller <davem@...emloft.net>,
        Netdev <netdev@...r.kernel.org>, j.vosburgh@...il.com,
        vfalico@...il.com, Andy Gospodarek <andy@...yhouse.net>,
        Jiří Pírko <jiri@...nulli.us>,
        sd@...asysnail.net, Roopa Prabhu <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 at 03:58, Stephen Hemminger
<stephen@...workplumber.org> wrote:
>
> 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(-)
> >
>

Hi Stephen,
Thank you so much for the review!

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

Sorry, I don't understand the point that you mentioned.
I appreciate if you tell me more in details about your review.