netdev - Re: [PATCH] net: phy: phy_remove_link_mode should not advertise new modes

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:   Tue, 14 Jul 2020 14:07:10 -0700 (PDT)
From:   David Miller <davem@...emloft.net>
To:     helmut.grohne@...enta.de
Cc:     andrew@...n.ch, f.fainelli@...il.com, hkallweit1@...il.com,
        linux@...linux.org.uk, kuba@...nel.org, netdev@...r.kernel.org,
        woojung.huh@...rochip.com, UNGLinuxDriver@...rochip.com,
        vivien.didelot@...il.com
Subject: Re: [PATCH] net: phy: phy_remove_link_mode should not advertise
 new modes

From: Helmut Grohne <helmut.grohne@...enta.de>
Date: Tue, 14 Jul 2020 10:25:42 +0200

> When doing "ip link set dev ... up" for a ksz9477 backed link,
> ksz9477_phy_setup is called and it calls phy_remove_link_mode to remove
> 1000baseT HDX. During phy_remove_link_mode, phy_advertise_supported is
> called.
> 
> If one wants to advertise fewer modes than the supported ones, one
> usually reduces the advertised link modes before upping the link (e.g.
> by passing an appropriate .link file to udev).  However upping
> overrwrites the advertised link modes due to the call to
> phy_advertise_supported reverting to the supported link modes.
> 
> It seems unintentional to have phy_remove_link_mode enable advertising
> bits and it does not match its description in any way. Instead of
> calling phy_advertise_supported, we should simply clear the link mode to
> be removed from both supported and advertising.
> 
> Signed-off-by: Helmut Grohne <helmut.grohne@...enta.de>
> Fixes: 41124fa64d4b29 ("net: ethernet: Add helper to remove a supported link mode")

The problem is that we can't allow the advertised setting to exceed
what is in the supported list.

That's why this helper is coded this way from day one.