netdev - RE: [PATCH net-next v3 0/5] net: dsa: remove .set

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: <CY1PR0401MB1536F870727AFA21BFC31115924F0@CY1PR0401MB1536.namprd04.prod.outlook.com>
Date:   Mon, 16 Oct 2017 15:23:42 +0000
From:   Rodney Cummings <rodney.cummings@...com>
To:     Vivien Didelot <vivien.didelot@...oirfairelinux.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "kernel@...oirfairelinux.com" <kernel@...oirfairelinux.com>,
        "David S. Miller" <davem@...emloft.net>,
        Florian Fainelli <f.fainelli@...il.com>,
        Andrew Lunn <andrew@...n.ch>,
        David Laight <David.Laight@...LAB.COM>
Subject: RE: [PATCH net-next v3 0/5] net: dsa: remove .set_addr

I am concerned about this proposed change.

According to IEEE Std 802.1Q, a "higher layer entity" (i.e. end station) that is internal to the bridge (i.e. switch) can use the same individual global MAC address that the switch is using. That behavior is common.

Use of a local semi-random MAC address is dangerous on Ethernet. Local MAC addresses can only be safely used in very narrow use cases, most of which are Wi-Fi. Generally speaking, local MAC addresses can be duplicated in the network, which causes many Ethernet protocols to break down. Therefore, general-purpose implementations like DSA cannot use a local MAC address in a reliable manner.

> -----Original Message-----
> From: netdev-owner@...r.kernel.org [mailto:netdev-owner@...r.kernel.org]
> On Behalf Of Vivien Didelot
> Sent: Friday, October 13, 2017 1:18 PM
> To: netdev@...r.kernel.org
> Cc: linux-kernel@...r.kernel.org; kernel@...oirfairelinux.com; David S.
> Miller <davem@...emloft.net>; Florian Fainelli <f.fainelli@...il.com>;
> Andrew Lunn <andrew@...n.ch>; David Laight <David.Laight@...LAB.COM>;
> Vivien Didelot <vivien.didelot@...oirfairelinux.com>
> Subject: [PATCH net-next v3 0/5] net: dsa: remove .set_addr
> 
> An Ethernet switch may support having a MAC address, which can be used
> as the switch's source address in transmitted full-duplex Pause frames.
> 
> If a DSA switch supports the related .set_addr operation, the DSA core
> sets the master's MAC address on the switch.
> 
> This won't make sense anymore in a multi-CPU ports system, because there
> won't be a unique master device assigned to a switch tree.
> 
> Moreover this operation is confusing because it makes the user think
> that it could be used to program the switch with the MAC address of the
> CPU/management port such that MAC address learning can be disabled on
> said port, but in fact, that's not how it is currently used.
> 
> To fix this, assign a random MAC address at setup time in the mv88e6060
> and mv88e6xxx drivers before removing .set_addr completely from DSA.
> 
> Changes in v3:
>   - include fix for mv88e6060 switch MAC address setter.
> 
> Changes in v2:
>   - remove .set_addr implementation from drivers and use a random MAC.
> 
> Vivien Didelot (5):
>   net: dsa: mv88e6xxx: setup random mac address
>   net: dsa: mv88e6060: fix switch MAC address
>   net: dsa: mv88e6060: setup random mac address
>   net: dsa: dsa_loop: remove .set_addr
>   net: dsa: remove .set_addr
> 
>  drivers/net/dsa/dsa_loop.c       |  8 --------
>  drivers/net/dsa/mv88e6060.c      | 37 ++++++++++++++++++++++++++---------
> --
>  drivers/net/dsa/mv88e6xxx/chip.c | 33 +++++++++++++++++----------------
>  include/net/dsa.h                |  1 -
>  net/dsa/dsa2.c                   |  6 ------
>  net/dsa/legacy.c                 |  6 ------
>  6 files changed, 43 insertions(+), 48 deletions(-)
> 
> --
> 2.14.2