netdev - Re: [PATCH v2] net: phy: dp83867: support Wake on LAN

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, 22 Oct 2019 21:15:20 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Heiner Kallweit <hkallweit1@...il.com>
Cc:     Thomas Hämmerle 
        <Thomas.Haemmerle@...fvision.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "f.fainelli@...il.com" <f.fainelli@...il.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "m.tretter@...gutronix.de" <m.tretter@...gutronix.de>
Subject: Re: [PATCH v2] net: phy: dp83867: support Wake on LAN

> > +static int dp83867_set_wol(struct phy_device *phydev,
> > +			   struct ethtool_wolinfo *wol)
> > +{
> > +	struct net_device *ndev = phydev->attached_dev;
> > +	u16 val_rxcfg, val_micr;
> > +	const u8 *mac;
> > +
> > +	val_rxcfg = phy_read_mmd(phydev, DP83867_DEVADDR, DP83867_RXFCFG);
> > +	val_micr = phy_read(phydev, MII_DP83867_MICR);
> > +
> > +	if (wol->wolopts & (WAKE_MAGIC | WAKE_MAGICSECURE | WAKE_UCAST |
> > +			    WAKE_BCAST)) {
> > +		val_rxcfg |= DP83867_WOL_ENH_MAC;
> > +		val_micr |= MII_DP83867_MICR_WOL_INT_EN;
> > +
> > +		if (wol->wolopts & WAKE_MAGIC) {
> > +			mac = (const u8 *)ndev->dev_addr;
> 
> Using a cast to add/remove a const qualifier usually isn't too nice.
> Why not simply declare mac w/o const?
> 
> Also PHY might not be attached. I think ndev should be checked for NULL.

Hi Heiner

I thought about that as well. But the ethtool API is invoked using a
network interface name. It would be odd to call into the PHY driver if
the PHY was not attached. And the resulting Oops would help us
identify the bug.

Plus all the other PHY drivers which implement magic packet WoL assume
the PHY is attached. It could be they are all broken i suppose...

    Andrew