netdev - Re: [PATCH RFC 2/4] net: phy: allow to bind genphy driver at probe time

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: <010ae64f-7e48-5e1e-2928-af3c4364f6e3@gmail.com>
Date:   Tue, 13 Aug 2019 15:53:54 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Heiner Kallweit <hkallweit1@...il.com>,
        Andrew Lunn <andrew@...n.ch>, Marek Behun <marek.behun@....cz>,
        David Miller <davem@...emloft.net>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH RFC 2/4] net: phy: allow to bind genphy driver at probe
 time

On 8/13/19 2:25 PM, Heiner Kallweit wrote:
> In cases like a fixed phy that is never attached to a net_device we
> may want to bind the genphy driver at probe time. Setting a PHY ID of
> 0xffffffff to bind the genphy driver would fail due to a check in
> get_phy_device(). Therefore let's change the PHY ID the genphy driver
> binds to to 0xfffffffe. This still shouldn't match any real PHY,
> and it will pass the check in get_phy_devcie().
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@...il.com>
> ---
>  drivers/net/phy/phy_device.c | 3 +--
>  include/linux/phy.h          | 4 ++++
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 163295dbc..54f80af31 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -2388,8 +2388,7 @@ void phy_drivers_unregister(struct phy_driver *drv, int n)
>  EXPORT_SYMBOL(phy_drivers_unregister);
>  
>  static struct phy_driver genphy_driver = {
> -	.phy_id		= 0xffffffff,
> -	.phy_id_mask	= 0xffffffff,
> +	PHY_ID_MATCH_EXACT(GENPHY_ID),
>  	.name		= "Generic PHY",
>  	.soft_reset	= genphy_no_soft_reset,
>  	.get_features	= genphy_read_abilities,
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 5ac7d2137..3b07bce78 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -37,6 +37,10 @@
>  #define PHY_1000BT_FEATURES	(SUPPORTED_1000baseT_Half | \
>  				 SUPPORTED_1000baseT_Full)
>  
> +#define GENPHY_ID_HIGH		0xffffU
> +#define GENPHY_ID_LOW		0xfffeU
> +#define GENPHY_ID		((GENPHY_ID_HIGH << 16) | GENPHY_ID_LOW)

This is a possible user ABI change here, if there is anything that
relies on reading 0xffff_ffff as a valid PHY OUI, you would be breaking
it. We might as well try to assign ourselves a specific PHY OUI, very
much like the Linux USB hubs show up with a Linux Foundation vendor ID.
-- 
Florian