mbox series

[net,0/2] net: phy: Unbind fixes

Message ID 20200917034310.2360488-1-f.fainelli@gmail.com
Headers show
Series net: phy: Unbind fixes | expand

Message

Florian Fainelli Sept. 17, 2020, 3:43 a.m. UTC 
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(-)

Comments

Andrew Lunn Sept. 17, 2020, 1:15 p.m. UTC | #1 
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
Florian Fainelli Sept. 17, 2020, 4:32 p.m. UTC | #2 
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.
Andrew Lunn Sept. 17, 2020, 5:28 p.m. UTC | #3 
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
David Miller Sept. 17, 2020, 11:56 p.m. UTC | #4 
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.