From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755408AbdDMS5L (ORCPT ); Thu, 13 Apr 2017 14:57:11 -0400 Received: from fllnx210.ext.ti.com ([198.47.19.17]:39724 "EHLO fllnx210.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752206AbdDMS5I (ORCPT ); Thu, 13 Apr 2017 14:57:08 -0400 Subject: Re: [PATCH v2] net: phy: micrel: fix crash when statistic requested for KSZ9031 phy To: Andrew Lunn References: <20170412225510.16885-1-grygorii.strashko@ti.com> <20170413185141.GA26229@lunn.ch> CC: "David S. Miller" , , Florian Fainelli , Sekhar Nori , From: Grygorii Strashko Message-ID: Date: Thu, 13 Apr 2017 13:56:57 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <20170413185141.GA26229@lunn.ch> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [128.247.83.96] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 [] (ethtool_get_phy_stats+0xd8/0x210) >> (ethtool_get_phy_stats) from [] (dev_ethtool+0x5b8/0x228c) >> (dev_ethtool) from [] (dev_ioctl+0x3fc/0x964) >> (dev_ioctl) from [] (sock_ioctl+0x170/0x2c0) >> (sock_ioctl) from [] (do_vfs_ioctl+0xa8/0x95c) >> (do_vfs_ioctl) from [] (SyS_ioctl+0x3c/0x64) >> (SyS_ioctl) from [] (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 >> --- >> 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 -- regards, -grygorii