Message ID | 20200615144501.1140870-1-heiko@sntech.de |
---|---|
State | Changes Requested |
Delegated to: | David Miller |
Headers | show |
Series | [v3,1/3] net: phy: mscc: move shared probe code into a helper | expand |
Hi Heiko, I love your patch! Perhaps something to improve: [auto build test WARNING on net-next/master] [also build test WARNING on sparc-next/master net/master linus/master v5.8-rc1 next-20200615] [cannot apply to robh/for-next] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Heiko-Stuebner/net-phy-mscc-move-shared-probe-code-into-a-helper/20200615-224727 base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git cb8e59cc87201af93dfbb6c3dccc8fcad72a09c2 config: x86_64-allyesconfig (attached as .config) compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 3d8149c2a1228609fd7d7c91a04681304a2f0ca9) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install x86_64 cross compiling tool for clang build # apt-get install binutils-x86-64-linux-gnu # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>, old ones prefixed by <<): >> drivers/net/phy/mscc/mscc_main.c:2002:10: warning: variable 'vsc8531' is uninitialized when used here [-Wuninitialized] vsc8531->base_addr, 0); ^~~~~~~ drivers/net/phy/mscc/mscc_main.c:1986:33: note: initialize the variable 'vsc8531' to silence this warning struct vsc8531_private *vsc8531; ^ = NULL 1 warning generated. vim +/vsc8531 +2002 drivers/net/phy/mscc/mscc_main.c 1983 1984 static int vsc8574_probe(struct phy_device *phydev) 1985 { 1986 struct vsc8531_private *vsc8531; 1987 int rc; 1988 u32 default_mode[4] = {VSC8531_LINK_1000_ACTIVITY, 1989 VSC8531_LINK_100_ACTIVITY, VSC8531_LINK_ACTIVITY, 1990 VSC8531_DUPLEX_COLLISION}; 1991 1992 rc = vsc85xx_probe_helper(phydev, default_mode, 1993 ARRAY_SIZE(default_mode), 1994 VSC8584_SUPP_LED_MODES, 1995 vsc8584_hw_stats, 1996 ARRAY_SIZE(vsc8584_hw_stats)); 1997 if (rc < 0) 1998 return rc; 1999 2000 vsc8584_get_base_addr(phydev); 2001 return devm_phy_package_join(&phydev->mdio.dev, phydev, > 2002 vsc8531->base_addr, 0); 2003 } 2004 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
From: Heiko Stuebner <heiko@sntech.de> Date: Mon, 15 Jun 2020 16:44:59 +0200 > static int vsc8584_probe(struct phy_device *phydev) > { > struct vsc8531_private *vsc8531; > + int rc; > u32 default_mode[4] = {VSC8531_LINK_1000_ACTIVITY, > VSC8531_LINK_100_ACTIVITY, VSC8531_LINK_ACTIVITY, > VSC8531_DUPLEX_COLLISION}; > @@ -2005,32 +2015,24 @@ static int vsc8584_probe(struct phy_device *phydev) > return -ENOTSUPP; > } > > - vsc8531 = devm_kzalloc(&phydev->mdio.dev, sizeof(*vsc8531), GFP_KERNEL); > - if (!vsc8531) > - return -ENOMEM; Because you removed this devm_kzalloc() code, vsc8531 is never initialized. > + return devm_phy_package_join(&phydev->mdio.dev, phydev, > + vsc8531->base_addr, 0); But it is still dereferenced here. Did the compiler really not warn you about this when you test built these changes?
From: David Miller <davem@davemloft.net> Date: Mon, 15 Jun 2020 18:11:29 -0700 (PDT) > Because you removed this devm_kzalloc() code, vsc8531 is never initialized. You also need to provide a proper header posting when you repost this series after fixing this bug.
Hi, Am Dienstag, 16. Juni 2020, 03:12:25 CEST schrieb David Miller: > From: David Miller <davem@davemloft.net> > Date: Mon, 15 Jun 2020 18:11:29 -0700 (PDT) > > + return devm_phy_package_join(&phydev->mdio.dev, phydev, > > + vsc8531->base_addr, 0); > > But it is still dereferenced here. > > Did the compiler really not warn you about this when you test built > these changes? I'm wondering that myself ... it probably did and I overlooked it, which also is indicated by the fact that I did add the declaration of the vsc8531 when rebasing. > > Because you removed this devm_kzalloc() code, vsc8531 is never initialized. > > You also need to provide a proper header posting when you repost this series > after fixing this bug. not sure I understand what you mean with "header posting" here. Thanks Heiko
On Tue, Jun 16, 2020 at 11:10:27AM +0200, Heiko Stübner wrote: > > > > You also need to provide a proper header posting when you repost this series > > after fixing this bug. > > not sure I understand what you mean with "header posting" here. David is requesting that you send a "0/N" email summarising the purpose of the patch series and any other relevant information to the series as a whole. The subsequent patches should be threaded to the 0/N email. The 0/N email should also contain the overall diffstat for the series.
Hi Heiko, I love your patch! Perhaps something to improve: [auto build test WARNING on net-next/master] [also build test WARNING on sparc-next/master net/master linus/master v5.8-rc1 next-20200616] [cannot apply to robh/for-next] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Heiko-Stuebner/net-phy-mscc-move-shared-probe-code-into-a-helper/20200615-224727 base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git cb8e59cc87201af93dfbb6c3dccc8fcad72a09c2 config: i386-randconfig-m021-20200616 (attached as .config) compiler: gcc-9 (Debian 9.3.0-13) 9.3.0 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> smatch warnings: drivers/net/phy/mscc/mscc_main.c:2002 vsc8574_probe() error: potentially dereferencing uninitialized 'vsc8531'. # https://github.com/0day-ci/linux/commit/5be350208c014ad5e0afd06868ccaaefd6216345 git remote add linux-review https://github.com/0day-ci/linux git remote update linux-review git checkout 5be350208c014ad5e0afd06868ccaaefd6216345 vim +/vsc8531 +2002 drivers/net/phy/mscc/mscc_main.c 5be350208c014a drivers/net/phy/mscc/mscc_main.c Heiko Stuebner 2020-06-15 1984 static int vsc8574_probe(struct phy_device *phydev) 5be350208c014a drivers/net/phy/mscc/mscc_main.c Heiko Stuebner 2020-06-15 1985 { 5be350208c014a drivers/net/phy/mscc/mscc_main.c Heiko Stuebner 2020-06-15 1986 struct vsc8531_private *vsc8531; ^^^^^^^ 5be350208c014a drivers/net/phy/mscc/mscc_main.c Heiko Stuebner 2020-06-15 1987 int rc; 5be350208c014a drivers/net/phy/mscc/mscc_main.c Heiko Stuebner 2020-06-15 1988 u32 default_mode[4] = {VSC8531_LINK_1000_ACTIVITY, 5be350208c014a drivers/net/phy/mscc/mscc_main.c Heiko Stuebner 2020-06-15 1989 VSC8531_LINK_100_ACTIVITY, VSC8531_LINK_ACTIVITY, 5be350208c014a drivers/net/phy/mscc/mscc_main.c Heiko Stuebner 2020-06-15 1990 VSC8531_DUPLEX_COLLISION}; 5be350208c014a drivers/net/phy/mscc/mscc_main.c Heiko Stuebner 2020-06-15 1991 5be350208c014a drivers/net/phy/mscc/mscc_main.c Heiko Stuebner 2020-06-15 1992 rc = vsc85xx_probe_helper(phydev, default_mode, 5be350208c014a drivers/net/phy/mscc/mscc_main.c Heiko Stuebner 2020-06-15 1993 ARRAY_SIZE(default_mode), 5be350208c014a drivers/net/phy/mscc/mscc_main.c Heiko Stuebner 2020-06-15 1994 VSC8584_SUPP_LED_MODES, 5be350208c014a drivers/net/phy/mscc/mscc_main.c Heiko Stuebner 2020-06-15 1995 vsc8584_hw_stats, 5be350208c014a drivers/net/phy/mscc/mscc_main.c Heiko Stuebner 2020-06-15 1996 ARRAY_SIZE(vsc8584_hw_stats)); 5be350208c014a drivers/net/phy/mscc/mscc_main.c Heiko Stuebner 2020-06-15 1997 if (rc < 0) 5be350208c014a drivers/net/phy/mscc/mscc_main.c Heiko Stuebner 2020-06-15 1998 return rc; 00d70d8e0e7811 drivers/net/phy/mscc.c Quentin Schulz 2018-10-08 1999 5be350208c014a drivers/net/phy/mscc/mscc_main.c Heiko Stuebner 2020-06-15 2000 vsc8584_get_base_addr(phydev); 5be350208c014a drivers/net/phy/mscc/mscc_main.c Heiko Stuebner 2020-06-15 2001 return devm_phy_package_join(&phydev->mdio.dev, phydev, 5be350208c014a drivers/net/phy/mscc/mscc_main.c Heiko Stuebner 2020-06-15 @2002 vsc8531->base_addr, 0); ^^^^^^^^^^^^^^^^^^ Not initialized. 00d70d8e0e7811 drivers/net/phy/mscc.c Quentin Schulz 2018-10-08 2003 } --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/drivers/net/phy/mscc/mscc_main.c b/drivers/net/phy/mscc/mscc_main.c index 5ddc44f87eaf..68308d3e9589 100644 --- a/drivers/net/phy/mscc/mscc_main.c +++ b/drivers/net/phy/mscc/mscc_main.c @@ -1935,12 +1935,11 @@ static int vsc85xx_read_status(struct phy_device *phydev) return genphy_read_status(phydev); } -static int vsc8514_probe(struct phy_device *phydev) +static int vsc85xx_probe_helper(struct phy_device *phydev, + u32 *leds, int num_leds, u16 led_modes, + const struct vsc85xx_hw_stat *stats, int nstats) { struct vsc8531_private *vsc8531; - u32 default_mode[4] = {VSC8531_LINK_1000_ACTIVITY, - VSC8531_LINK_100_ACTIVITY, VSC8531_LINK_ACTIVITY, - VSC8531_DUPLEX_COLLISION}; vsc8531 = devm_kzalloc(&phydev->mdio.dev, sizeof(*vsc8531), GFP_KERNEL); if (!vsc8531) @@ -1948,54 +1947,65 @@ static int vsc8514_probe(struct phy_device *phydev) phydev->priv = vsc8531; - vsc8584_get_base_addr(phydev); - devm_phy_package_join(&phydev->mdio.dev, phydev, - vsc8531->base_addr, 0); - - vsc8531->nleds = 4; - vsc8531->supp_led_modes = VSC85XX_SUPP_LED_MODES; - vsc8531->hw_stats = vsc85xx_hw_stats; - vsc8531->nstats = ARRAY_SIZE(vsc85xx_hw_stats); + vsc8531->nleds = num_leds; + vsc8531->supp_led_modes = led_modes; + vsc8531->hw_stats = stats; + vsc8531->nstats = nstats; vsc8531->stats = devm_kcalloc(&phydev->mdio.dev, vsc8531->nstats, sizeof(u64), GFP_KERNEL); if (!vsc8531->stats) return -ENOMEM; - return vsc85xx_dt_led_modes_get(phydev, default_mode); + return vsc85xx_dt_led_modes_get(phydev, leds); } -static int vsc8574_probe(struct phy_device *phydev) +static int vsc8514_probe(struct phy_device *phydev) { struct vsc8531_private *vsc8531; + int rc; u32 default_mode[4] = {VSC8531_LINK_1000_ACTIVITY, VSC8531_LINK_100_ACTIVITY, VSC8531_LINK_ACTIVITY, VSC8531_DUPLEX_COLLISION}; - vsc8531 = devm_kzalloc(&phydev->mdio.dev, sizeof(*vsc8531), GFP_KERNEL); - if (!vsc8531) - return -ENOMEM; - - phydev->priv = vsc8531; + rc = vsc85xx_probe_helper(phydev, default_mode, + ARRAY_SIZE(default_mode), + VSC85XX_SUPP_LED_MODES, + vsc85xx_hw_stats, + ARRAY_SIZE(vsc85xx_hw_stats)); + if (rc < 0) + return rc; + vsc8531 = phydev->priv; vsc8584_get_base_addr(phydev); - devm_phy_package_join(&phydev->mdio.dev, phydev, - vsc8531->base_addr, 0); + return devm_phy_package_join(&phydev->mdio.dev, phydev, + vsc8531->base_addr, 0); +} - vsc8531->nleds = 4; - vsc8531->supp_led_modes = VSC8584_SUPP_LED_MODES; - vsc8531->hw_stats = vsc8584_hw_stats; - vsc8531->nstats = ARRAY_SIZE(vsc8584_hw_stats); - vsc8531->stats = devm_kcalloc(&phydev->mdio.dev, vsc8531->nstats, - sizeof(u64), GFP_KERNEL); - if (!vsc8531->stats) - return -ENOMEM; +static int vsc8574_probe(struct phy_device *phydev) +{ + struct vsc8531_private *vsc8531; + int rc; + u32 default_mode[4] = {VSC8531_LINK_1000_ACTIVITY, + VSC8531_LINK_100_ACTIVITY, VSC8531_LINK_ACTIVITY, + VSC8531_DUPLEX_COLLISION}; + + rc = vsc85xx_probe_helper(phydev, default_mode, + ARRAY_SIZE(default_mode), + VSC8584_SUPP_LED_MODES, + vsc8584_hw_stats, + ARRAY_SIZE(vsc8584_hw_stats)); + if (rc < 0) + return rc; - return vsc85xx_dt_led_modes_get(phydev, default_mode); + vsc8584_get_base_addr(phydev); + return devm_phy_package_join(&phydev->mdio.dev, phydev, + vsc8531->base_addr, 0); } static int vsc8584_probe(struct phy_device *phydev) { struct vsc8531_private *vsc8531; + int rc; u32 default_mode[4] = {VSC8531_LINK_1000_ACTIVITY, VSC8531_LINK_100_ACTIVITY, VSC8531_LINK_ACTIVITY, VSC8531_DUPLEX_COLLISION}; @@ -2005,32 +2015,24 @@ static int vsc8584_probe(struct phy_device *phydev) return -ENOTSUPP; } - vsc8531 = devm_kzalloc(&phydev->mdio.dev, sizeof(*vsc8531), GFP_KERNEL); - if (!vsc8531) - return -ENOMEM; - - phydev->priv = vsc8531; + rc = vsc85xx_probe_helper(phydev, default_mode, + ARRAY_SIZE(default_mode), + VSC8584_SUPP_LED_MODES, + vsc8584_hw_stats, + ARRAY_SIZE(vsc8584_hw_stats)); + if (rc < 0) + return rc; + vsc8531 = phydev->priv; vsc8584_get_base_addr(phydev); - devm_phy_package_join(&phydev->mdio.dev, phydev, - vsc8531->base_addr, 0); - - vsc8531->nleds = 4; - vsc8531->supp_led_modes = VSC8584_SUPP_LED_MODES; - vsc8531->hw_stats = vsc8584_hw_stats; - vsc8531->nstats = ARRAY_SIZE(vsc8584_hw_stats); - vsc8531->stats = devm_kcalloc(&phydev->mdio.dev, vsc8531->nstats, - sizeof(u64), GFP_KERNEL); - if (!vsc8531->stats) - return -ENOMEM; - - return vsc85xx_dt_led_modes_get(phydev, default_mode); + return devm_phy_package_join(&phydev->mdio.dev, phydev, + vsc8531->base_addr, 0); } static int vsc85xx_probe(struct phy_device *phydev) { struct vsc8531_private *vsc8531; - int rate_magic; + int rate_magic, rc; u32 default_mode[2] = {VSC8531_LINK_1000_ACTIVITY, VSC8531_LINK_100_ACTIVITY}; @@ -2038,23 +2040,18 @@ static int vsc85xx_probe(struct phy_device *phydev) if (rate_magic < 0) return rate_magic; - vsc8531 = devm_kzalloc(&phydev->mdio.dev, sizeof(*vsc8531), GFP_KERNEL); - if (!vsc8531) - return -ENOMEM; - - phydev->priv = vsc8531; + rc = vsc85xx_probe_helper(phydev, default_mode, + ARRAY_SIZE(default_mode), + VSC85XX_SUPP_LED_MODES, + vsc85xx_hw_stats, + ARRAY_SIZE(vsc85xx_hw_stats)); + if (rc < 0) + return rc; + vsc8531 = phydev->priv; vsc8531->rate_magic = rate_magic; - vsc8531->nleds = 2; - vsc8531->supp_led_modes = VSC85XX_SUPP_LED_MODES; - vsc8531->hw_stats = vsc85xx_hw_stats; - vsc8531->nstats = ARRAY_SIZE(vsc85xx_hw_stats); - vsc8531->stats = devm_kcalloc(&phydev->mdio.dev, vsc8531->nstats, - sizeof(u64), GFP_KERNEL); - if (!vsc8531->stats) - return -ENOMEM; - return vsc85xx_dt_led_modes_get(phydev, default_mode); + return 0; } /* Microsemi VSC85xx PHYs */