| Message ID | 20201029002845.28984-1-sbauer@blackbox.su |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | fix for potential NULL pointer dereference with bare lan743x | expand |
| Context | Check | Description |
|---|---|---|
| jkicinski/cover_letter | success | Link |
| jkicinski/fixes_present | success | Link |
| jkicinski/patch_count | success | Link |
| jkicinski/tree_selection | success | Guessed tree name to be net-next |
| jkicinski/subject_prefix | warning | Target tree name not specified in the subject |
| jkicinski/source_inline | success | Was 0 now: 0 |
| jkicinski/verify_signedoff | success | Link |
| jkicinski/module_param | success | Was 0 now: 0 |
| jkicinski/build_32bit | fail | Errors and warnings before: 4 this patch: 4 |
| jkicinski/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| jkicinski/verify_fixes | success | Link |
| jkicinski/checkpatch | fail | Link |
| jkicinski/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| jkicinski/header_inline | success | Link |
| jkicinski/stable | success | Stable not CCed |
On Thu, 29 Oct 2020 03:28:45 +0300 Sergej Bauer wrote: > This is just a minor fix which prevents a kernel NULL pointer > dereference when using phy-less lan743x. > > Signed-off-by: Sergej Bauer <sbauer@blackbox.su> I take you mean when the device is down netdev->phydev will be NULL? > diff --git a/drivers/net/ethernet/microchip/lan743x_ethtool.c b/drivers/net/ethernet/microchip/lan743x_ethtool.c > index dcde496da7fb..354d72d550f2 100644 > --- a/drivers/net/ethernet/microchip/lan743x_ethtool.c > +++ b/drivers/net/ethernet/microchip/lan743x_ethtool.c > @@ -793,6 +795,9 @@ static int lan743x_ethtool_set_wol(struct net_device *netdev, > { > struct lan743x_adapter *adapter = netdev_priv(netdev); > > + if (!netdev->phydev) > + return -EIO; Does it make sense to just skip the phy_ethtool_set_wol() call instead? Also doesn't the wol configuration of the PHY get lost across an netdev up/down cycle in this driver? Should it be re-applied after phy is connected back? > adapter->wolopts = 0; > if (wol->wolopts & WAKE_UCAST) > adapter->wolopts |= WAKE_UCAST;
diff --git a/drivers/net/ethernet/microchip/lan743x_ethtool.c b/drivers/net/ethernet/microchip/lan743x_ethtool.c index dcde496da7fb..354d72d550f2 100644 --- a/drivers/net/ethernet/microchip/lan743x_ethtool.c +++ b/drivers/net/ethernet/microchip/lan743x_ethtool.c @@ -780,7 +780,9 @@ static void lan743x_ethtool_get_wol(struct net_device *netdev, wol->supported = 0; wol->wolopts = 0; - phy_ethtool_get_wol(netdev->phydev, wol); + + if (netdev->phydev) + phy_ethtool_get_wol(netdev->phydev, wol); wol->supported |= WAKE_BCAST | WAKE_UCAST | WAKE_MCAST | WAKE_MAGIC | WAKE_PHY | WAKE_ARP; @@ -793,6 +795,9 @@ static int lan743x_ethtool_set_wol(struct net_device *netdev, { struct lan743x_adapter *adapter = netdev_priv(netdev); + if (!netdev->phydev) + return -EIO; + adapter->wolopts = 0; if (wol->wolopts & WAKE_UCAST) adapter->wolopts |= WAKE_UCAST;
This is just a minor fix which prevents a kernel NULL pointer dereference when using phy-less lan743x. Signed-off-by: Sergej Bauer <sbauer@blackbox.su> --- drivers/net/ethernet/microchip/lan743x_ethtool.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)