* [PATCH net 0/2] net: phy: Unbind fixes
@ 2020-09-17 3:43 Florian Fainelli
2020-09-17 3:43 ` [PATCH net 1/2] net: phy: Avoid NPD upon phy_detach() when driver is unbound Florian Fainelli
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Florian Fainelli @ 2020-09-17 3:43 UTC (permalink / raw)
To: netdev; +Cc: Florian Fainelli, andrew, hkallweit1, kuba, davem
This patch series fixes a couple of issues with the unbinding of the PHY
drivers and then bringing down a network interface. The first is a NULL
pointer de-reference and the second was an incorrect warning being
triggered.
Florian Fainelli (2):
net: phy: Avoid NPD upon phy_detach() when driver is unbound
net: phy: Do not warn in phy_stop() on PHY_DOWN
drivers/net/phy/phy.c | 2 +-
drivers/net/phy/phy_device.c | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH net 1/2] net: phy: Avoid NPD upon phy_detach() when driver is unbound
2020-09-17 3:43 [PATCH net 0/2] net: phy: Unbind fixes Florian Fainelli
@ 2020-09-17 3:43 ` Florian Fainelli
2020-09-17 13:15 ` Andrew Lunn
2020-09-17 17:28 ` Andrew Lunn
2020-09-17 3:43 ` [PATCH net 2/2] net: phy: Do not warn in phy_stop() on PHY_DOWN Florian Fainelli
2020-09-17 23:56 ` [PATCH net 0/2] net: phy: Unbind fixes David Miller
2 siblings, 2 replies; 8+ messages in thread
From: Florian Fainelli @ 2020-09-17 3:43 UTC (permalink / raw)
To: netdev; +Cc: Florian Fainelli, andrew, hkallweit1, kuba, davem
If we have unbound the PHY driver prior to calling phy_detach() (often
via phy_disconnect()) then we can cause a NULL pointer de-reference
accessing the driver owner member. The steps to reproduce are:
echo unimac-mdio-0:01 > /sys/class/net/eth0/phydev/driver/unbind
ip link set eth0 down
Fixes: cafe8df8b9bc ("net: phy: Fix lack of reference count on PHY driver")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/phy/phy_device.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 8adfbad0a1e8..81eb76a8295b 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1682,7 +1682,8 @@ void phy_detach(struct phy_device *phydev)
phy_led_triggers_unregister(phydev);
- module_put(phydev->mdio.dev.driver->owner);
+ if (phydev->mdio.dev.driver)
+ module_put(phydev->mdio.dev.driver->owner);
/* If the device had no specific driver before (i.e. - it
* was using the generic driver), we unbind the device
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH net 1/2] net: phy: Avoid NPD upon phy_detach() when driver is unbound
2020-09-17 3:43 ` [PATCH net 1/2] net: phy: Avoid NPD upon phy_detach() when driver is unbound Florian Fainelli
@ 2020-09-17 13:15 ` Andrew Lunn
2020-09-17 16:32 ` Florian Fainelli
2020-09-17 17:28 ` Andrew Lunn
1 sibling, 1 reply; 8+ messages in thread
From: Andrew Lunn @ 2020-09-17 13:15 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, hkallweit1, kuba, davem
On Wed, Sep 16, 2020 at 08:43:09PM -0700, Florian Fainelli wrote:
> If we have unbound the PHY driver prior to calling phy_detach() (often
> via phy_disconnect()) then we can cause a NULL pointer de-reference
> accessing the driver owner member. The steps to reproduce are:
>
> echo unimac-mdio-0:01 > /sys/class/net/eth0/phydev/driver/unbind
> ip link set eth0 down
Hi Florian
How forceful is this unbind? Can we actually block it while the
interface is up? Or returning -EBUSY would make sense.
Andrew
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net 1/2] net: phy: Avoid NPD upon phy_detach() when driver is unbound
2020-09-17 13:15 ` Andrew Lunn
@ 2020-09-17 16:32 ` Florian Fainelli
0 siblings, 0 replies; 8+ messages in thread
From: Florian Fainelli @ 2020-09-17 16:32 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev, hkallweit1, kuba, davem
On 9/17/2020 6:15 AM, Andrew Lunn wrote:
> On Wed, Sep 16, 2020 at 08:43:09PM -0700, Florian Fainelli wrote:
>> If we have unbound the PHY driver prior to calling phy_detach() (often
>> via phy_disconnect()) then we can cause a NULL pointer de-reference
>> accessing the driver owner member. The steps to reproduce are:
>>
>> echo unimac-mdio-0:01 > /sys/class/net/eth0/phydev/driver/unbind
>> ip link set eth0 down
>
> Hi Florian
>
> How forceful is this unbind? Can we actually block it while the
> interface is up? Or returning -EBUSY would make sense.
It it not forceful, you can unbind the PHY driver from underneath the
net_device and nothing bad happens, really. This is not a very realistic
or practical use case, but several years ago, I went into making sure we
would not create NPD if that happened.
--
Florian
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net 1/2] net: phy: Avoid NPD upon phy_detach() when driver is unbound
2020-09-17 3:43 ` [PATCH net 1/2] net: phy: Avoid NPD upon phy_detach() when driver is unbound Florian Fainelli
2020-09-17 13:15 ` Andrew Lunn
@ 2020-09-17 17:28 ` Andrew Lunn
1 sibling, 0 replies; 8+ messages in thread
From: Andrew Lunn @ 2020-09-17 17:28 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, hkallweit1, kuba, davem
On Wed, Sep 16, 2020 at 08:43:09PM -0700, Florian Fainelli wrote:
> If we have unbound the PHY driver prior to calling phy_detach() (often
> via phy_disconnect()) then we can cause a NULL pointer de-reference
> accessing the driver owner member. The steps to reproduce are:
>
> echo unimac-mdio-0:01 > /sys/class/net/eth0/phydev/driver/unbind
> ip link set eth0 down
>
> Fixes: cafe8df8b9bc ("net: phy: Fix lack of reference count on PHY driver")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net 2/2] net: phy: Do not warn in phy_stop() on PHY_DOWN
2020-09-17 3:43 [PATCH net 0/2] net: phy: Unbind fixes Florian Fainelli
2020-09-17 3:43 ` [PATCH net 1/2] net: phy: Avoid NPD upon phy_detach() when driver is unbound Florian Fainelli
@ 2020-09-17 3:43 ` Florian Fainelli
2020-09-17 13:16 ` Andrew Lunn
2020-09-17 23:56 ` [PATCH net 0/2] net: phy: Unbind fixes David Miller
2 siblings, 1 reply; 8+ messages in thread
From: Florian Fainelli @ 2020-09-17 3:43 UTC (permalink / raw)
To: netdev; +Cc: Florian Fainelli, andrew, hkallweit1, kuba, davem
When phy_is_started() was added to catch incorrect PHY states,
phy_stop() would not be qualified against PHY_DOWN. It is possible to
reach that state when the PHY driver has been unbound and the network
device is then brought down.
Fixes: 2b3e88ea6528 ("net: phy: improve phy state checking")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/phy/phy.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 735a806045ac..8947d58f2a25 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -996,7 +996,7 @@ void phy_stop(struct phy_device *phydev)
{
struct net_device *dev = phydev->attached_dev;
- if (!phy_is_started(phydev)) {
+ if (!phy_is_started(phydev) && phydev->state != PHY_DOWN) {
WARN(1, "called from state %s\n",
phy_state_to_str(phydev->state));
return;
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH net 2/2] net: phy: Do not warn in phy_stop() on PHY_DOWN
2020-09-17 3:43 ` [PATCH net 2/2] net: phy: Do not warn in phy_stop() on PHY_DOWN Florian Fainelli
@ 2020-09-17 13:16 ` Andrew Lunn
0 siblings, 0 replies; 8+ messages in thread
From: Andrew Lunn @ 2020-09-17 13:16 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, hkallweit1, kuba, davem
On Wed, Sep 16, 2020 at 08:43:10PM -0700, Florian Fainelli wrote:
> When phy_is_started() was added to catch incorrect PHY states,
> phy_stop() would not be qualified against PHY_DOWN. It is possible to
> reach that state when the PHY driver has been unbound and the network
> device is then brought down.
>
> Fixes: 2b3e88ea6528 ("net: phy: improve phy state checking")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net 0/2] net: phy: Unbind fixes
2020-09-17 3:43 [PATCH net 0/2] net: phy: Unbind fixes Florian Fainelli
2020-09-17 3:43 ` [PATCH net 1/2] net: phy: Avoid NPD upon phy_detach() when driver is unbound Florian Fainelli
2020-09-17 3:43 ` [PATCH net 2/2] net: phy: Do not warn in phy_stop() on PHY_DOWN Florian Fainelli
@ 2020-09-17 23:56 ` David Miller
2 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2020-09-17 23:56 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev, andrew, hkallweit1, kuba
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Wed, 16 Sep 2020 20:43:08 -0700
> This patch series fixes a couple of issues with the unbinding of the PHY
> drivers and then bringing down a network interface. The first is a NULL
> pointer de-reference and the second was an incorrect warning being
> triggered.
Series applied and queued up for -stable, thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-09-17 23:56 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-17 3:43 [PATCH net 0/2] net: phy: Unbind fixes Florian Fainelli
2020-09-17 3:43 ` [PATCH net 1/2] net: phy: Avoid NPD upon phy_detach() when driver is unbound Florian Fainelli
2020-09-17 13:15 ` Andrew Lunn
2020-09-17 16:32 ` Florian Fainelli
2020-09-17 17:28 ` Andrew Lunn
2020-09-17 3:43 ` [PATCH net 2/2] net: phy: Do not warn in phy_stop() on PHY_DOWN Florian Fainelli
2020-09-17 13:16 ` Andrew Lunn
2020-09-17 23:56 ` [PATCH net 0/2] net: phy: Unbind fixes David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.