All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] net: phy: micrel: fix crash when statistic requested for KSZ9031 phy
@ 2017-04-12 22:55 Grygorii Strashko
  2017-04-13 18:51 ` Andrew Lunn
  0 siblings, 1 reply; 3+ messages in thread
From: Grygorii Strashko @ 2017-04-12 22:55 UTC (permalink / raw)
  To: David S. Miller, netdev, Andrew Lunn, Florian Fainelli
  Cc: Sekhar Nori, linux-kernel, Grygorii Strashko

Now the command:
	ethtool --phy-statistics eth0
will cause system crash with meassage "Unable to handle kernel NULL pointer
dereference at virtual address 00000010" from:

 (kszphy_get_stats) from [<c069f1d8>] (ethtool_get_phy_stats+0xd8/0x210)
 (ethtool_get_phy_stats) from [<c06a0738>] (dev_ethtool+0x5b8/0x228c)
 (dev_ethtool) from [<c06b5484>] (dev_ioctl+0x3fc/0x964)
 (dev_ioctl) from [<c0679f7c>] (sock_ioctl+0x170/0x2c0)
 (sock_ioctl) from [<c02419d4>] (do_vfs_ioctl+0xa8/0x95c)
 (do_vfs_ioctl) from [<c02422c4>] (SyS_ioctl+0x3c/0x64)
 (SyS_ioctl) from [<c0107d60>] (ret_fast_syscall+0x0/0x44)

The reason: phy_driver structure for KSZ9031 phy has no .probe() callback
defined. As result, struct phy_device *phydev->priv pointer will not be
initializes (null).
This issue will affect also following phys:
 KSZ8795, KSZ886X, KSZ8873MLL, KSZ9031, KSZ9021, KSZ8061, KS8737

Fix it by:
- adding .probe() = kszphy_probe() callback to KSZ9031, KSZ9021
phys. The kszphy_probe() can be re-used as it doesn't do any phy specific
settings.
- removing statistic callbacks from other phys (KSZ8795, KSZ886X,
KSZ8873MLL, KSZ8061, KS8737) as they doesn't have corresponding
statistic counters.

Fixes: 2b2427d06426 ("phy: micrel: Add ethtool statistics counters")
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
changes in v2:
 - probe callback added to KSZ9031, KSZ9021
 - statistic callback removed from KSZ8795, KSZ886X, KSZ8873MLL, KSZ8061, KS8737

Link on v1:
 https://lkml.org/lkml/2017/4/10/1183

 drivers/net/phy/micrel.c | 18 ++----------------
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 6742070..6f207e6 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -574,7 +574,6 @@ static int ksz9031_config_init(struct phy_device *phydev)
 				MII_KSZ9031RN_TX_DATA_PAD_SKEW, 4,
 				tx_data_skews, 4);
 	}
-
 	return ksz9031_center_flp_timing(phydev);
 }
 
@@ -798,9 +797,6 @@ static struct phy_driver ksphy_driver[] = {
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,
 	.config_intr	= kszphy_config_intr,
-	.get_sset_count = kszphy_get_sset_count,
-	.get_strings	= kszphy_get_strings,
-	.get_stats	= kszphy_get_stats,
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
 }, {
@@ -940,9 +936,6 @@ static struct phy_driver ksphy_driver[] = {
 	.read_status	= genphy_read_status,
 	.ack_interrupt	= kszphy_ack_interrupt,
 	.config_intr	= kszphy_config_intr,
-	.get_sset_count = kszphy_get_sset_count,
-	.get_strings	= kszphy_get_strings,
-	.get_stats	= kszphy_get_stats,
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
 }, {
@@ -952,6 +945,7 @@ static struct phy_driver ksphy_driver[] = {
 	.features	= PHY_GBIT_FEATURES,
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
 	.driver_data	= &ksz9021_type,
+	.probe		= kszphy_probe,
 	.config_init	= ksz9021_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
@@ -971,6 +965,7 @@ static struct phy_driver ksphy_driver[] = {
 	.features	= PHY_GBIT_FEATURES,
 	.flags		= PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
 	.driver_data	= &ksz9021_type,
+	.probe		= kszphy_probe,
 	.config_init	= ksz9031_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= ksz9031_read_status,
@@ -989,9 +984,6 @@ static struct phy_driver ksphy_driver[] = {
 	.config_init	= kszphy_config_init,
 	.config_aneg	= ksz8873mll_config_aneg,
 	.read_status	= ksz8873mll_read_status,
-	.get_sset_count = kszphy_get_sset_count,
-	.get_strings	= kszphy_get_strings,
-	.get_stats	= kszphy_get_stats,
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
 }, {
@@ -1003,9 +995,6 @@ static struct phy_driver ksphy_driver[] = {
 	.config_init	= kszphy_config_init,
 	.config_aneg	= genphy_config_aneg,
 	.read_status	= genphy_read_status,
-	.get_sset_count = kszphy_get_sset_count,
-	.get_strings	= kszphy_get_strings,
-	.get_stats	= kszphy_get_stats,
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
 }, {
@@ -1017,9 +1006,6 @@ static struct phy_driver ksphy_driver[] = {
 	.config_init	= kszphy_config_init,
 	.config_aneg	= ksz8873mll_config_aneg,
 	.read_status	= ksz8873mll_read_status,
-	.get_sset_count = kszphy_get_sset_count,
-	.get_strings	= kszphy_get_strings,
-	.get_stats	= kszphy_get_stats,
 	.suspend	= genphy_suspend,
 	.resume		= genphy_resume,
 } };
-- 
2.10.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH v2] net: phy: micrel: fix crash when statistic requested for KSZ9031 phy
  2017-04-12 22:55 [PATCH v2] net: phy: micrel: fix crash when statistic requested for KSZ9031 phy Grygorii Strashko
@ 2017-04-13 18:51 ` Andrew Lunn
  2017-04-13 18:56   ` Grygorii Strashko
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Lunn @ 2017-04-13 18:51 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: David S. Miller, netdev, Florian Fainelli, Sekhar Nori, linux-kernel

On Wed, Apr 12, 2017 at 05:55:10PM -0500, Grygorii Strashko wrote:
> Now the command:
> 	ethtool --phy-statistics eth0
> will cause system crash with meassage "Unable to handle kernel NULL pointer
> dereference at virtual address 00000010" from:
> 
>  (kszphy_get_stats) from [<c069f1d8>] (ethtool_get_phy_stats+0xd8/0x210)
>  (ethtool_get_phy_stats) from [<c06a0738>] (dev_ethtool+0x5b8/0x228c)
>  (dev_ethtool) from [<c06b5484>] (dev_ioctl+0x3fc/0x964)
>  (dev_ioctl) from [<c0679f7c>] (sock_ioctl+0x170/0x2c0)
>  (sock_ioctl) from [<c02419d4>] (do_vfs_ioctl+0xa8/0x95c)
>  (do_vfs_ioctl) from [<c02422c4>] (SyS_ioctl+0x3c/0x64)
>  (SyS_ioctl) from [<c0107d60>] (ret_fast_syscall+0x0/0x44)
> 
> The reason: phy_driver structure for KSZ9031 phy has no .probe() callback
> defined. As result, struct phy_device *phydev->priv pointer will not be
> initializes (null).
> This issue will affect also following phys:
>  KSZ8795, KSZ886X, KSZ8873MLL, KSZ9031, KSZ9021, KSZ8061, KS8737
> 
> Fix it by:
> - adding .probe() = kszphy_probe() callback to KSZ9031, KSZ9021
> phys. The kszphy_probe() can be re-used as it doesn't do any phy specific
> settings.
> - removing statistic callbacks from other phys (KSZ8795, KSZ886X,
> KSZ8873MLL, KSZ8061, KS8737) as they doesn't have corresponding
> statistic counters.
> 
> Fixes: 2b2427d06426 ("phy: micrel: Add ethtool statistics counters")
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
> changes in v2:
>  - probe callback added to KSZ9031, KSZ9021
>  - statistic callback removed from KSZ8795, KSZ886X, KSZ8873MLL, KSZ8061, KS8737
> 
> Link on v1:
>  https://lkml.org/lkml/2017/4/10/1183
> 
>  drivers/net/phy/micrel.c | 18 ++----------------
>  1 file changed, 2 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index 6742070..6f207e6 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
> @@ -574,7 +574,6 @@ static int ksz9031_config_init(struct phy_device *phydev)
>  				MII_KSZ9031RN_TX_DATA_PAD_SKEW, 4,
>  				tx_data_skews, 4);
>  	}
> -
>  	return ksz9031_center_flp_timing(phydev);
>  }

Hi Grygorii

Whitespace changed like this should be in a separate patch, or not
made at all.

Otherwise, thanks for looking at the datasheets and fixing this up.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH v2] net: phy: micrel: fix crash when statistic requested for KSZ9031 phy
  2017-04-13 18:51 ` Andrew Lunn
@ 2017-04-13 18:56   ` Grygorii Strashko
  0 siblings, 0 replies; 3+ messages in thread
From: Grygorii Strashko @ 2017-04-13 18:56 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: David S. Miller, netdev, Florian Fainelli, Sekhar Nori, linux-kernel



On 04/13/2017 01:51 PM, Andrew Lunn wrote:
> On Wed, Apr 12, 2017 at 05:55:10PM -0500, Grygorii Strashko wrote:
>> Now the command:
>> 	ethtool --phy-statistics eth0
>> will cause system crash with meassage "Unable to handle kernel NULL pointer
>> dereference at virtual address 00000010" from:
>>
>>  (kszphy_get_stats) from [<c069f1d8>] (ethtool_get_phy_stats+0xd8/0x210)
>>  (ethtool_get_phy_stats) from [<c06a0738>] (dev_ethtool+0x5b8/0x228c)
>>  (dev_ethtool) from [<c06b5484>] (dev_ioctl+0x3fc/0x964)
>>  (dev_ioctl) from [<c0679f7c>] (sock_ioctl+0x170/0x2c0)
>>  (sock_ioctl) from [<c02419d4>] (do_vfs_ioctl+0xa8/0x95c)
>>  (do_vfs_ioctl) from [<c02422c4>] (SyS_ioctl+0x3c/0x64)
>>  (SyS_ioctl) from [<c0107d60>] (ret_fast_syscall+0x0/0x44)
>>
>> The reason: phy_driver structure for KSZ9031 phy has no .probe() callback
>> defined. As result, struct phy_device *phydev->priv pointer will not be
>> initializes (null).
>> This issue will affect also following phys:
>>  KSZ8795, KSZ886X, KSZ8873MLL, KSZ9031, KSZ9021, KSZ8061, KS8737
>>
>> Fix it by:
>> - adding .probe() = kszphy_probe() callback to KSZ9031, KSZ9021
>> phys. The kszphy_probe() can be re-used as it doesn't do any phy specific
>> settings.
>> - removing statistic callbacks from other phys (KSZ8795, KSZ886X,
>> KSZ8873MLL, KSZ8061, KS8737) as they doesn't have corresponding
>> statistic counters.
>>
>> Fixes: 2b2427d06426 ("phy: micrel: Add ethtool statistics counters")
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> ---
>> changes in v2:
>>  - probe callback added to KSZ9031, KSZ9021
>>  - statistic callback removed from KSZ8795, KSZ886X, KSZ8873MLL, KSZ8061, KS8737
>>
>> Link on v1:
>>  https://lkml.org/lkml/2017/4/10/1183
>>
>>  drivers/net/phy/micrel.c | 18 ++----------------
>>  1 file changed, 2 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
>> index 6742070..6f207e6 100644
>> --- a/drivers/net/phy/micrel.c
>> +++ b/drivers/net/phy/micrel.c
>> @@ -574,7 +574,6 @@ static int ksz9031_config_init(struct phy_device *phydev)
>>  				MII_KSZ9031RN_TX_DATA_PAD_SKEW, 4,
>>  				tx_data_skews, 4);
>>  	}
>> -
>>  	return ksz9031_center_flp_timing(phydev);
>>  }
>
> Hi Grygorii
>
> Whitespace changed like this should be in a separate patch, or not
> made at all.


Oh. sry i've missed it. Will resend


>
> Otherwise, thanks for looking at the datasheets and fixing this up.
>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>


-- 
regards,
-grygorii

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-04-13 18:57 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-12 22:55 [PATCH v2] net: phy: micrel: fix crash when statistic requested for KSZ9031 phy Grygorii Strashko
2017-04-13 18:51 ` Andrew Lunn
2017-04-13 18:56   ` Grygorii Strashko

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.