Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1623282 > unrolled thread
| Started by | Grygorii Strashko <grygorii.strashko@ti.com> |
|---|---|
| First post | 2017-04-13 21:20 +0200 |
| Last post | 2017-04-17 19:40 +0200 |
| Articles | 5 — 5 participants |
Back to article view | Back to linux.kernel
[PATCH v3] net: phy: micrel: fix crash when statistic requested for KSZ9031 phy Grygorii Strashko <grygorii.strashko@ti.com> - 2017-04-13 21:20 +0200
Re: [PATCH v3] net: phy: micrel: fix crash when statistic requested for KSZ9031 phy Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> - 2017-04-13 21:30 +0200
Re: [PATCH v3] net: phy: micrel: fix crash when statistic requested for KSZ9031 phy Andrew Lunn <andrew@lunn.ch> - 2017-04-13 21:40 +0200
Re: [PATCH v3] net: phy: micrel: fix crash when statistic requested for KSZ9031 phy Florian Fainelli <f.fainelli@gmail.com> - 2017-04-13 21:40 +0200
Re: [PATCH v3] net: phy: micrel: fix crash when statistic requested for KSZ9031 phy David Miller <davem@davemloft.net> - 2017-04-17 19:40 +0200
| From | Grygorii Strashko <grygorii.strashko@ti.com> |
|---|---|
| Date | 2017-04-13 21:20 +0200 |
| Subject | [PATCH v3] net: phy: micrel: fix crash when statistic requested for KSZ9031 phy |
| Message-ID | <tvSrD-7oj-9@gated-at.bofh.it> |
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>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
changes in v3:
- occasional whitespace change removed
changes in v2:
- probe callback added to KSZ9031, KSZ9021
- statistic callback removed from KSZ8795, KSZ886X, KSZ8873MLL, KSZ8061, KS8737
Links
v2: https://patchwork.ozlabs.org/patch/750194/
v1: https://lkml.org/lkml/2017/4/10/1183
drivers/net/phy/micrel.c | 17 ++---------------
1 file changed, 2 insertions(+), 15 deletions(-)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 6742070..1326d99 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -798,9 +798,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 +937,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 +946,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 +966,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 +985,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 +996,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 +1007,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
[toc] | [next] | [standalone]
| From | Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> |
|---|---|
| Date | 2017-04-13 21:30 +0200 |
| Subject | Re: [PATCH v3] net: phy: micrel: fix crash when statistic requested for KSZ9031 phy |
| Message-ID | <tvSBk-7sF-15@gated-at.bofh.it> |
| In reply to | #1623282 |
On 04/13/2017 10:11 PM, 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.
Not sure how the 2nd change fixes the reported issue. It looks like a
material for a separate patch...
> Fixes: 2b2427d06426 ("phy: micrel: Add ethtool statistics counters")
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
[...]
MBR, Sergei
[toc] | [prev] | [next] | [standalone]
| From | Andrew Lunn <andrew@lunn.ch> |
|---|---|
| Date | 2017-04-13 21:40 +0200 |
| Subject | Re: [PATCH v3] net: phy: micrel: fix crash when statistic requested for KSZ9031 phy |
| Message-ID | <tvSL0-7wP-15@gated-at.bofh.it> |
| In reply to | #1623286 |
On Thu, Apr 13, 2017 at 10:28:09PM +0300, Sergei Shtylyov wrote:
> On 04/13/2017 10:11 PM, 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.
>
> Not sure how the 2nd change fixes the reported issue. It looks
> like a material for a separate patch...
There are two different cases here:
1) The hardware supports the stats. So a probe function is needed, but
is missing.
2) The hardware does not support the stats, so there should not be
stats ops.
The same crash will happen, independent of which one of the above is
true. You need to fix them both, to stop it crashing.
Andrew
[toc] | [prev] | [next] | [standalone]
| From | Florian Fainelli <f.fainelli@gmail.com> |
|---|---|
| Date | 2017-04-13 21:40 +0200 |
| Subject | Re: [PATCH v3] net: phy: micrel: fix crash when statistic requested for KSZ9031 phy |
| Message-ID | <tvSKZ-7wP-1@gated-at.bofh.it> |
| In reply to | #1623282 |
On 04/13/2017 12:11 PM, 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>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
[toc] | [prev] | [next] | [standalone]
| From | David Miller <davem@davemloft.net> |
|---|---|
| Date | 2017-04-17 19:40 +0200 |
| Subject | Re: [PATCH v3] net: phy: micrel: fix crash when statistic requested for KSZ9031 phy |
| Message-ID | <txiN3-3w9-9@gated-at.bofh.it> |
| In reply to | #1623282 |
From: Grygorii Strashko <grygorii.strashko@ti.com>
Date: Thu, 13 Apr 2017 14:11:27 -0500
> 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>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Applied.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web