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
| ||
|
Message-ID: <BY5PR07MB6514DEE80FD0C24F9FD52ED1D35A0@BY5PR07MB6514.namprd07.prod.outlook.com> Date: Wed, 11 Dec 2019 08:21:57 +0000 From: Milind Parab <mparab@...ence.com> To: Russell King - ARM Linux admin <linux@...linux.org.uk> CC: "nicolas.nerre@...rochip.com" <nicolas.nerre@...rochip.com>, "andrew@...n.ch" <andrew@...n.ch>, "antoine.tenart@...tlin.com" <antoine.tenart@...tlin.com>, "f.fainelli@...il.com" <f.fainelli@...il.com>, "davem@...emloft.net" <davem@...emloft.net>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "hkallweit1@...il.com" <hkallweit1@...il.com>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Dhananjay Vilasrao Kangude <dkangude@...ence.com>, "a.fatoum@...gutronix.de" <a.fatoum@...gutronix.de>, "brad.mouring@...com" <brad.mouring@...com>, Parshuram Raju Thombare <pthombar@...ence.com> Subject: RE: [PATCH 1/3] net: macb: fix for fixed-link mode >> >> + ret = phylink_of_phy_connect(bp->phylink, dn, 0); >> >> + >> >> + if (!dn || (ret && !of_parse_phandle(dn, "phy-handle", 0))) { >> > >> >Hi, >> >If of_parse_phandle() returns non-null, the device_node it returns will >> >have its reference count increased by one. That reference needs to be >> >put. >> > >> >> Okay, as per your suggestion below addition will be okay to store the >"phy_node" and then of_node_put(phy_node) on error >> >> phy_node = of_parse_phandle(dn, "phy-handle", 0); >> if (!dn || (ret && !phy_node)) { >> phydev = phy_find_first(bp->mii_bus); >... >> if (phy_node) >> of_node_put(phy_node); > >As you're only interested in whether phy-handle exists or not, you >could do this instead: > > phy_node = of_parse_phandle(dn, "phy-handle", 0); > of_node_put(phy_node); > if (!dn || (ret && !phy_node)) { > ... > >Yes, it looks a bit weird, but the only thing you're interested in >here is whether of_parse_phandle() returned NULL or non-NULL. You're >not interested in dereferencing the pointer. > >Some may raise some eye-brows at that, so it may be better to have >this as a helper: > >static bool macb_phy_handle_exists(struct device_node *dn) >{ > dn = of_parse_phandle(dn, "phy-handle", 0); > of_node_put(dn); > return dn != NULL; >} > >and use it as: > > if (!dn || (ret && !macb_phy_handle_exists(dn))) { > >which is more obvious what is going on. > This is good. I will put this in the revised patch. > >> >> return ret; >> >> >I assume you're trying to determine whether phylink_of_phy_connect() >> >failed because of a missing phy-handle rather than of_phy_attach() >> >failing? Maybe those two failures ought to be distinguished by errno >> >return value? >> >> Yes, PHY will be scanned only if phylink_of_phy_connect() returns error due to missing "phy-handle". >> Currently, phylink_of_phy_connect() returns same error for missing "phy-handle" and of_phy_attach() failure. >> >> >of_phy_attach() may fail due to of_phy_find_device() failing to find >> >the PHY, or phy_attach_direct() failing. We could switch from using >> >of_phy_attach(), to using of_phy_find_device() directly so we can then >> >propagate phy_attach_direct()'s error code back, rather than losing it. >> >That would then leave the case of of_phy_find_device() failure to be >> >considered in terms of errno return value. > >Here's a patch I quickly knocked up that does this - may not apply to >the kernel you're using as there's a whole bunch of work I have >outstanding, but gives the outline idea. Does this help? > > Yes, this will help. Once available, we will adopt this change. >8<=== >From: Russell King <rmk+kernel@...linux.org.uk> >Subject: [PATCH] net: phylink: avoid of_phy_attach() > >of_phy_attach() hides the return value of phy_attach_direct(), forcing >us to return a "generic" ENODEV error code that is indistinguishable >from the lack-of-phy-property case. >Switch to using of_phy_find_device() to find the PHY device, and then >propagating any phy_attach_direct() error back to the caller. > > >Signed-off-by: Russell King <rmk+kernel@...linux.org.uk> >--- >
Powered by blists - more mailing lists