Skip to content

Commit

Permalink
sh_eth: Fix ethtool operation crash when net device is down
Browse files Browse the repository at this point in the history
The driver connects and disconnects the PHY device whenever the
net device is brought up and down.  The ethtool get_settings,
set_settings and nway_reset operations will dereference a null
or dangling pointer if called while it is down.

I think it would be preferable to keep the PHY connected, but there
may be good reasons not to.

As an immediate fix for this bug:
- Set the phydev pointer to NULL after disconnecting the PHY
- Change those three operations to return -ENODEV while the PHY is
  not connected

Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
Ben Hutchings authored and David S. Miller committed Jan 19, 2015
1 parent b37feed commit 4f9dce2
Showing 1 changed file with 10 additions and 0 deletions.
10 changes: 10 additions & 0 deletions drivers/net/ethernet/renesas/sh_eth.c
Original file line number Diff line number Diff line change
Expand Up @@ -1827,6 +1827,9 @@ static int sh_eth_get_settings(struct net_device *ndev,
unsigned long flags;
int ret;

if (!mdp->phydev)
return -ENODEV;

spin_lock_irqsave(&mdp->lock, flags);
ret = phy_ethtool_gset(mdp->phydev, ecmd);
spin_unlock_irqrestore(&mdp->lock, flags);
Expand All @@ -1841,6 +1844,9 @@ static int sh_eth_set_settings(struct net_device *ndev,
unsigned long flags;
int ret;

if (!mdp->phydev)
return -ENODEV;

spin_lock_irqsave(&mdp->lock, flags);

/* disable tx and rx */
Expand Down Expand Up @@ -1875,6 +1881,9 @@ static int sh_eth_nway_reset(struct net_device *ndev)
unsigned long flags;
int ret;

if (!mdp->phydev)
return -ENODEV;

spin_lock_irqsave(&mdp->lock, flags);
ret = phy_start_aneg(mdp->phydev);
spin_unlock_irqrestore(&mdp->lock, flags);
Expand Down Expand Up @@ -2184,6 +2193,7 @@ static int sh_eth_close(struct net_device *ndev)
if (mdp->phydev) {
phy_stop(mdp->phydev);
phy_disconnect(mdp->phydev);
mdp->phydev = NULL;
}

free_irq(ndev->irq, ndev);
Expand Down

0 comments on commit 4f9dce2

Please sign in to comment.