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: <20220730162351.rwfzpma5uvsg3kax@skbuf>
Date:   Sat, 30 Jul 2022 16:23:52 +0000
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     Rob Herring <robh+dt@...nel.org>
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>,
        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

Hi Rob,

On Fri, Jul 29, 2022 at 12:39:06PM -0600, Rob Herring wrote:
> All valid points. At least for the sea of warnings, you can limit
> checking to only a subset of schemas you care about. Setting
> 'DT_SCHEMA_FILES=net/' will only check networking schemas for example.
> Just need folks to care about those subsets.
> 
> I'm not saying don't put warnings in the kernel for this. Just don't
> make it the only source of warnings. Given you are tightening the
> requirements, it makes sense to have a warning. If it was something
> entirely new, then I'd be more inclined to say only the schema should
> check.

How does this look like? It says that if the 'ethernet' property exists
and is a phandle (i.e. this is a CPU port), or if the 'link' property
exists and is a phandle-array (i.e. this is a DSA port), then the
phylink-related properties are mandatory, in the combinations that they
may appear in.

diff --git a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
index 09317e16cb5d..ed828cec90fd 100644
--- a/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
+++ b/Documentation/devicetree/bindings/net/dsa/dsa-port.yaml
@@ -76,6 +76,31 @@ properties:
 required:
   - reg
 
+if:
+  oneOf:
+    - properties:
+        ethernet:
+          items:
+            $ref: /schemas/types.yaml#/definitions/phandle
+    - properties:
+        link:
+          items:
+            $ref: /schemas/types.yaml#/definitions/phandle-array
+then:
+  allOf:
+  - required:
+    - phy-mode
+  - oneOf:
+    - required:
+      - fixed-link
+    - required:
+      - phy-handle
+    - required:
+      - managed
+    - required:
+      - phy-handle
+      - managed
+
 additionalProperties: true
 

I have deliberately made this part of dsa-port.yaml and not depend on
any compatible string. The reason is the code - we'll warn about missing
properties regardless of whether we have fallback logic for some older
switches. Essentially the fact that some switches have the fallback to
not use phylink will remain an undocumented easter egg as far as the
dt-schema is concerned.

What do you think?

Can I also get an ACK for this patch and patch 1 please?