netdev - Re: [PATCH net-next] net: phy: Relax error checking on sysfs_create

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] [day] [month] [year] [list]

Message-ID: <021D486A-A81D-45F6-BB66-8FF075B03F70@gmail.com>
Date:   Sat, 27 May 2017 08:40:54 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     netdev@...r.kernel.org
CC:     davem@...emloft.net, andrew@...n.ch, Woojung.Huh@...rochip.com
Subject: Re: [PATCH net-next] net: phy: Relax error checking on sysfs_create_link()

On May 26, 2017 8:34:18 PM PDT, Florian Fainelli <f.fainelli@...il.com> wrote:
>Some Ethernet drivers will attach/connect to a PHY device before
>calling
>register_netdevice() which is responsible for calling
>netdev_register_kobject()
>which would do the network device's kobject initialization. In such a
>case,
>sysfs_create_link() would return -ENOENT because the network device's
>kobject
>is not ready yet, and we would fail to connect to the PHY device.
>
>In order to keep things simple and symetrical, we just take the success
>path as
>indicative of the ability to access the network device's kobject, and
>create
>the second link if that's the case.
>
>Fixes: 5568363f0cb3 ("net: phy: Create sysfs reciprocal links for
>attached_dev/phydev")
>Reported-by: Woojung Hung <Woojung.Huh@...rochip.com>
>Signed-off-by: Florian Fainelli <f.fainelli@...il.com>
>---
> drivers/net/phy/phy_device.c | 28 ++++++++++++++++++++--------
> include/linux/phy.h          |  2 ++
> 2 files changed, 22 insertions(+), 8 deletions(-)
>
>diff --git a/drivers/net/phy/phy_device.c
>b/drivers/net/phy/phy_device.c
>index f84414b8f2ee..523366bd705a 100644
>--- a/drivers/net/phy/phy_device.c
>+++ b/drivers/net/phy/phy_device.c
>@@ -960,15 +960,25 @@ int phy_attach_direct(struct net_device *dev,
>struct phy_device *phydev,
> 
> 	phydev->attached_dev = dev;
> 	dev->phydev = phydev;
>+
>+	/* Some Ethernet drivers try to connect to a PHY device before
>+	 * calling register_netdevice() -> netdev_register_kobject() and
>+	 * does the dev->dev.kobj initialization. Here we only check for
>+	 * success which indicates that the network device kobject is
>+	 * ready. Once we do that we still need to keep track of whether
>+	 * links were successfully set up or not for phy_detach() to
>+	 * remove them accordingly.
>+	 */
> 	err = sysfs_create_link(&phydev->mdio.dev.kobj, &dev->dev.kobj,
> 				"attached_dev");
>-	if (err)
>-		goto error;
>+	if (!err) {
>+		err = sysfs_create_link(&dev->dev.kobj, &phydev->mdio.dev.kobj,
>+					"phydev");
>+		if (err)
>+			goto error;
> 
>-	err = sysfs_create_link(&dev->dev.kobj, &phydev->mdio.dev.kobj,
>-				"phydev");
>-	if (err)
>-		goto error;
>+		phydev->sysfs_links = true;
>+	}
> 

We should not assume that this Boolean will be true or false if we enter this function a second time, v2 coming.

-- 
Florian