netdev - Re: [PATCH net v5 01/10] net: core: limit nested device depth

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:   Mon, 21 Oct 2019 18:51:11 -0600
From:   David Ahern <dsahern@...il.com>
To:     Taehee Yoo <ap420073@...il.com>, davem@...emloft.net,
        netdev@...r.kernel.org, linux-wireless@...r.kernel.org,
        jakub.kicinski@...ronome.com, johannes@...solutions.net,
        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,
        stephen@...workplumber.org, sashal@...nel.org, hare@...e.de,
        varun@...lsio.com, ubraun@...ux.ibm.com, kgraul@...ux.ibm.com,
        jay.vosburgh@...onical.com, schuffelen@...gle.com, bjorn@...k.no
Subject: Re: [PATCH net v5 01/10] net: core: limit nested device depth

On 10/21/19 12:47 PM, Taehee Yoo wrote:
> Current code doesn't limit the number of nested devices.
> Nested devices would be handled recursively and this needs huge stack
> memory. So, unlimited nested devices could make stack overflow.
> 
> This patch adds upper_level and lower_level, they are common variables
> and represent maximum lower/upper depth.
> When upper/lower device is attached or dettached,
> {lower/upper}_level are updated. and if maximum depth is bigger than 8,
> attach routine fails and returns -EMLINK.
> 
> In addition, this patch converts recursive routine of
> netdev_walk_all_{lower/upper} to iterator routine.

They were made recursive because of a particular setup. Did you verify
your changes did not break it? See commits starting with
5bb61cb5fd115bed1814f6b97417e0f397da3c79

> 
> Test commands:
>     ip link add dummy0 type dummy
>     ip link add link dummy0 name vlan1 type vlan id 1
>     ip link set vlan1 up
> 
>     for i in {2..55}
>     do
> 	    let A=$i-1
> 
> 	    ip link add vlan$i link vlan$A type vlan id $i
>     done
>     ip link del dummy0

8 levels of nested vlan seems like complete nonsense. Why not just limit
that stacking and not mess with the rest which can affect real use cases?