netdev - Re: [PATCH v2 net-next 4/4] net: dsa: validate that DT nodes of shared ports have the properties they need

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:   Fri, 29 Jul 2022 10:22:49 -0600
From:   Rob Herring <robh+dt@...nel.org>
To:     Vladimir Oltean <vladimir.oltean@....com>
Cc:     netdev <netdev@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Oleksij Rempel <linux@...pel-privat.de>,
        Christian Marangi <ansuelsmth@...il.com>,
        John Crispin <john@...ozen.org>,
        Kurt Kanzenbach <kurt@...utronix.de>,
        Mans Rullgard <mans@...sr.com>,
        Arun Ramadoss <arun.ramadoss@...rochip.com>,
        Woojung Huh <woojung.huh@...rochip.com>,
        UNGLinuxDriver@...rochip.com,
        Claudiu Manoil <claudiu.manoil@....com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        George McCollister <george.mccollister@...il.com>,
        DENG Qingfang <dqfext@...il.com>,
        Sean Wang <sean.wang@...iatek.com>,
        Landen Chao <Landen.Chao@...iatek.com>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Hauke Mehrtens <hauke@...ke-m.de>,
        Martin Blumenstingl <martin.blumenstingl@...glemail.com>,
        Aleksander Jan Bajkowski <olek2@...pl>,
        Alvin Šipraga <alsi@...g-olufsen.dk>,
        Luiz Angelo Daros de Luca <luizluca@...il.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Pawel Dembicki <paweldembicki@...il.com>,
        Clément Léger <clement.leger@...tlin.com>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Russell King <rmk+kernel@...linux.org.uk>,
        Marek Behún <kabel@...nel.org>,
        Marcin Wojtas <mw@...ihalf.com>,
        Frank Rowand <frowand.list@...il.com>
Subject: Re: [PATCH v2 net-next 4/4] net: dsa: validate that DT nodes of
 shared ports have the properties they need

On Fri, Jul 29, 2022 at 7:21 AM Vladimir Oltean <vladimir.oltean@....com> wrote:
>
> There is a desire coming from Russell King to make all DSA drivers
> register unconditionally with phylink, to simplify the code paths:
> https://lore.kernel.org/netdev/YtGPO5SkMZfN8b%2Fs@shell.armlinux.org.uk/
>
> However this is not possible today without risking to break drivers
> which rely on a different mechanism, that where ports are manually
> brought up to the highest link speed during setup, and never touched by
> phylink at runtime.
>
> This happens because DSA was not always integrated with phylink, and
> when the early drivers were converted from platform data to the new DSA
> bindings, there was no information kept in the platform data structures
> about port link speeds, so as a result, there was no information
> translated into the first DT bindings.
>
> https://lore.kernel.org/all/YtXFtTsf++AeDm1l@lunn.ch/
>
> Today we have a workaround in place, introduced by commit a20f997010c4
> ("net: dsa: Don't instantiate phylink for CPU/DSA ports unless needed"),
> where shared ports would be checked for the presence of phy-handle/
> fixed-link/managed OF properties, and if missing, phylink registration
> would be skipped.
>
> We modify the logic of this workaround such as to stop the proliferation
> of more port OF nodes with lacking information, to put an upper bound to
> the number of switches for which a link management description must be
> faked in order for phylink registration to become possible for them.
>
> Today we have drivers introduced years after the phylink migration of
> CPU/DSA ports, and yet we're still not completely sure whether all new
> drivers use phylink, because this depends on dynamic information
> (DT blob, which may very well not be upstream, because why would it).
> Driver maintainers may even be unaware about the fact that omitting
> fixed-link/phy-handle for CPU/DSA ports is legal, and even works with
> some of the old pre-phylink drivers.
>
> Add central validation in DSA for the OF properties required by phylink,
> in an attempt to sanitize the environment for future driver writers, and
> as much as possible for existing driver maintainers.

It's not the kernel's job to validate the DT. If it was, it does a
horrible job. Is the schema providing this validation? If not, you
need to add it.

Rob