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]

Message-ID: <20220729183444.jzr3eoj6xdumezwu@skbuf>
Date:   Fri, 29 Jul 2022 18:34:45 +0000
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     Marcin Wojtas <mw@...ihalf.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" <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>,
        Rob Herring <robh+dt@...nel.org>,
        Frank Rowand <frowand.list@...il.com>,
        Tomasz Nowicki <tn@...ihalf.com>,
        Grzegorz Jaszczyk <jaz@...ihalf.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 07:57:50PM +0200, Marcin Wojtas wrote:
> I'm ok with enforcing the phylink usage and updating the binding
> description, so the CPU / DSA ports have a proper full description of
> the link. What I find problematic is including the drivers' related
> ifdefs and compat strings in the subsystem's generic code. With this
> change, if someone adds a new driver (or extends the existing ones),
> they will have to add the string in the driver AND net/dsa...

I chuckled when I read this. You must have missed:

 * If you are considering expanding this table for newly introduced switches,
 * think again. It is OK to remove switches from this table if there aren't DT
 * blobs in circulation which rely on defaulting the shared ports.

The #ifdef's are there such that the compatible array is smaller on a
kernel when those drivers are compiled out.

> How about the following scenario:
> - Remove allow/blocklist from this patch and validate the description
> always (no opt out).

We're validating the description always. We're opting a fixed number of
switches out of _enforcing_ it, number which will not increase.
That's why the people here are copied, to state if they're ok with being
in one camp or the other.

> For an agreed timeframe (1 year? 2 LTS releases?)
> it wouldn't cause the switch probe to fail, but instead of
> dev_warn/dev_err, there should be a big fat WARN_ON(). Spoiled bootlog
> will encourage users to update the device trees.

The intention is _not_ to fail probing for drivers with incomplete
bindings, neither now nor after 1 year or 2 LTS releases.

The intention is to not allow drivers which didn't have any such DT
blobs, or awareness of the feature, to gain any parasitic users.
The DSA core currently allows it. If planets align just the right way,
those ports might even work by accident, until they don't.

> - After the deadline, the switch probe should start failing with
> improper description and everyone will have to use phylink.

Not applicable after the explanation above, I think. At least, it's not
my goal to fail drivers. If individual maintainers want to do so,
they're free to do it from my side.

> - Announce the binding change and start updating DT binding
> description schema (adding the validation on that level too).
> ?

The announcement is here, what else are you thinking of?