[PATCH] net: hisilicon: Variable "reg_value" in function mdio_sc_cfg_reg_write() could be uninitialized

[PATCH] net: hisilicon: Variable "reg_value" in function mdio_sc_cfg_reg_write() could be uninitialized

[Yizhuo]

In function mdio_sc_cfg_reg_write(), variable reg_value could be
uninitialized if regmap_read() fails. However, this variable is
used later in the if statement, which is potentially unsafe.

Signed-off-by: Yizhuo <yzhai003@ucr.edu>
---
 drivers/net/ethernet/hisilicon/hns_mdio.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/hisilicon/hns_mdio.c b/drivers/net/ethernet/hisilicon/hns_mdio.c
index 3e863a71c513..f5b64cb2d0f6 100644
--- a/drivers/net/ethernet/hisilicon/hns_mdio.c
+++ b/drivers/net/ethernet/hisilicon/hns_mdio.c
@@ -148,11 +148,17 @@ static int mdio_sc_cfg_reg_write(struct hns_mdio_device *mdio_dev,
 {
 	u32 time_cnt;
 	u32 reg_value;
+	int ret;
 
 	regmap_write(mdio_dev->subctrl_vbase, cfg_reg, set_val);
 
 	for (time_cnt = MDIO_TIMEOUT; time_cnt; time_cnt--) {
-		regmap_read(mdio_dev->subctrl_vbase, st_reg, &reg_value);
+		ret = regmap_read(mdio_dev->subctrl_vbase, st_reg, &reg_value);
+		if (ret) {
+			dev_err(mdio_dev->regmap->dev, "Fail to read from the register\n");
+			return ret;
+		}
+
 		reg_value &= st_msk;
 		if ((!!check_st) == (!!reg_value))
 			break;
-- 
2.17.1

Re: [PATCH] net: hisilicon: Variable "reg_value" in function mdio_sc_cfg_reg_write() could be uninitialized

[Yizhuo Zhai]

Sorry for the inconvenience. I made some mistake here, please ignore
this patch and I will submit a new one.

On Mon, Sep 2, 2019 at 4:14 PM Yizhuo <yzhai003@ucr.edu> wrote:
>
> In function mdio_sc_cfg_reg_write(), variable reg_value could be
> uninitialized if regmap_read() fails. However, this variable is
> used later in the if statement, which is potentially unsafe.
>
> Signed-off-by: Yizhuo <yzhai003@ucr.edu>
> ---
>  drivers/net/ethernet/hisilicon/hns_mdio.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/hisilicon/hns_mdio.c b/drivers/net/ethernet/hisilicon/hns_mdio.c
> index 3e863a71c513..f5b64cb2d0f6 100644
> --- a/drivers/net/ethernet/hisilicon/hns_mdio.c
> +++ b/drivers/net/ethernet/hisilicon/hns_mdio.c
> @@ -148,11 +148,17 @@ static int mdio_sc_cfg_reg_write(struct hns_mdio_device *mdio_dev,
>  {
>         u32 time_cnt;
>         u32 reg_value;
> +       int ret;
>
>         regmap_write(mdio_dev->subctrl_vbase, cfg_reg, set_val);
>
>         for (time_cnt = MDIO_TIMEOUT; time_cnt; time_cnt--) {
> -               regmap_read(mdio_dev->subctrl_vbase, st_reg, &reg_value);
> +               ret = regmap_read(mdio_dev->subctrl_vbase, st_reg, &reg_value);
> +               if (ret) {
> +                       dev_err(mdio_dev->regmap->dev, "Fail to read from the register\n");
> +                       return ret;
> +               }
> +
>                 reg_value &= st_msk;
>                 if ((!!check_st) == (!!reg_value))
>                         break;
> --
> 2.17.1
>


-- 
Kind Regards,

Yizhuo Zhai

Computer Science, Graduate Student
University of California, Riverside

Re: [PATCH] net: hisilicon: Variable "reg_value" in function mdio_sc_cfg_reg_write() could be uninitialized

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 6635 bytes --]

Hi Yizhuo,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[cannot apply to v5.3-rc7 next-20190902]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Yizhuo/net-hisilicon-Variable-reg_value-in-function-mdio_sc_cfg_reg_write-could-be-uninitialized/20190903-071544
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gcc (GCC) 7.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.4.0 make.cross ARCH=sh 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from include/linux/acpi.h:15:0,
                    from drivers/net//ethernet/hisilicon/hns_mdio.c:6:
   drivers/net//ethernet/hisilicon/hns_mdio.c: In function 'mdio_sc_cfg_reg_write':
>> drivers/net//ethernet/hisilicon/hns_mdio.c:158:20: error: 'struct hns_mdio_device' has no member named 'regmap'
       dev_err(mdio_dev->regmap->dev, "Fail to read from the register\n");
                       ^
   include/linux/device.h:1499:11: note: in definition of macro 'dev_err'
     _dev_err(dev, dev_fmt(fmt), ##__VA_ARGS__)
              ^~~

vim +158 drivers/net//ethernet/hisilicon/hns_mdio.c

   > 6	#include <linux/acpi.h>
     7	#include <linux/errno.h>
     8	#include <linux/etherdevice.h>
     9	#include <linux/init.h>
    10	#include <linux/kernel.h>
    11	#include <linux/mfd/syscon.h>
    12	#include <linux/module.h>
    13	#include <linux/mutex.h>
    14	#include <linux/netdevice.h>
    15	#include <linux/of_address.h>
    16	#include <linux/of.h>
    17	#include <linux/of_mdio.h>
    18	#include <linux/of_platform.h>
    19	#include <linux/phy.h>
    20	#include <linux/platform_device.h>
    21	#include <linux/regmap.h>
    22	
    23	#define MDIO_DRV_NAME "Hi-HNS_MDIO"
    24	#define MDIO_BUS_NAME "Hisilicon MII Bus"
    25	
    26	#define MDIO_TIMEOUT			1000000
    27	
    28	struct hns_mdio_sc_reg {
    29		u16 mdio_clk_en;
    30		u16 mdio_clk_dis;
    31		u16 mdio_reset_req;
    32		u16 mdio_reset_dreq;
    33		u16 mdio_clk_st;
    34		u16 mdio_reset_st;
    35	};
    36	
    37	struct hns_mdio_device {
    38		u8 __iomem *vbase;		/* mdio reg base address */
    39		struct regmap *subctrl_vbase;
    40		struct hns_mdio_sc_reg sc_reg;
    41	};
    42	
    43	/* mdio reg */
    44	#define MDIO_COMMAND_REG		0x0
    45	#define MDIO_ADDR_REG			0x4
    46	#define MDIO_WDATA_REG			0x8
    47	#define MDIO_RDATA_REG			0xc
    48	#define MDIO_STA_REG			0x10
    49	
    50	/* cfg phy bit map */
    51	#define MDIO_CMD_DEVAD_M	0x1f
    52	#define MDIO_CMD_DEVAD_S	0
    53	#define MDIO_CMD_PRTAD_M	0x1f
    54	#define MDIO_CMD_PRTAD_S	5
    55	#define MDIO_CMD_OP_S		10
    56	#define MDIO_CMD_ST_S		12
    57	#define MDIO_CMD_START_B	14
    58	
    59	#define MDIO_ADDR_DATA_M	0xffff
    60	#define MDIO_ADDR_DATA_S	0
    61	
    62	#define MDIO_WDATA_DATA_M	0xffff
    63	#define MDIO_WDATA_DATA_S	0
    64	
    65	#define MDIO_RDATA_DATA_M	0xffff
    66	#define MDIO_RDATA_DATA_S	0
    67	
    68	#define MDIO_STATE_STA_B	0
    69	
    70	enum mdio_st_clause {
    71		MDIO_ST_CLAUSE_45 = 0,
    72		MDIO_ST_CLAUSE_22
    73	};
    74	
    75	enum mdio_c22_op_seq {
    76		MDIO_C22_WRITE = 1,
    77		MDIO_C22_READ = 2
    78	};
    79	
    80	enum mdio_c45_op_seq {
    81		MDIO_C45_WRITE_ADDR = 0,
    82		MDIO_C45_WRITE_DATA,
    83		MDIO_C45_READ_INCREMENT,
    84		MDIO_C45_READ
    85	};
    86	
    87	/* peri subctrl reg */
    88	#define MDIO_SC_CLK_EN		0x338
    89	#define MDIO_SC_CLK_DIS		0x33C
    90	#define MDIO_SC_RESET_REQ	0xA38
    91	#define MDIO_SC_RESET_DREQ	0xA3C
    92	#define MDIO_SC_CLK_ST		0x531C
    93	#define MDIO_SC_RESET_ST	0x5A1C
    94	
    95	static void mdio_write_reg(u8 __iomem *base, u32 reg, u32 value)
    96	{
    97		writel_relaxed(value, base + reg);
    98	}
    99	
   100	#define MDIO_WRITE_REG(a, reg, value) \
   101		mdio_write_reg((a)->vbase, (reg), (value))
   102	
   103	static u32 mdio_read_reg(u8 __iomem *base, u32 reg)
   104	{
   105		return readl_relaxed(base + reg);
   106	}
   107	
   108	#define mdio_set_field(origin, mask, shift, val) \
   109		do { \
   110			(origin) &= (~((mask) << (shift))); \
   111			(origin) |= (((val) & (mask)) << (shift)); \
   112		} while (0)
   113	
   114	#define mdio_get_field(origin, mask, shift) (((origin) >> (shift)) & (mask))
   115	
   116	static void mdio_set_reg_field(u8 __iomem *base, u32 reg, u32 mask, u32 shift,
   117				       u32 val)
   118	{
   119		u32 origin = mdio_read_reg(base, reg);
   120	
   121		mdio_set_field(origin, mask, shift, val);
   122		mdio_write_reg(base, reg, origin);
   123	}
   124	
   125	#define MDIO_SET_REG_FIELD(dev, reg, mask, shift, val) \
   126		mdio_set_reg_field((dev)->vbase, (reg), (mask), (shift), (val))
   127	
   128	static u32 mdio_get_reg_field(u8 __iomem *base, u32 reg, u32 mask, u32 shift)
   129	{
   130		u32 origin;
   131	
   132		origin = mdio_read_reg(base, reg);
   133		return mdio_get_field(origin, mask, shift);
   134	}
   135	
   136	#define MDIO_GET_REG_FIELD(dev, reg, mask, shift) \
   137			mdio_get_reg_field((dev)->vbase, (reg), (mask), (shift))
   138	
   139	#define MDIO_GET_REG_BIT(dev, reg, bit) \
   140			mdio_get_reg_field((dev)->vbase, (reg), 0x1ull, (bit))
   141	
   142	#define MDIO_CHECK_SET_ST	1
   143	#define MDIO_CHECK_CLR_ST	0
   144	
   145	static int mdio_sc_cfg_reg_write(struct hns_mdio_device *mdio_dev,
   146					 u32 cfg_reg, u32 set_val,
   147					 u32 st_reg, u32 st_msk, u8 check_st)
   148	{
   149		u32 time_cnt;
   150		u32 reg_value;
   151		int ret;
   152	
   153		regmap_write(mdio_dev->subctrl_vbase, cfg_reg, set_val);
   154	
   155		for (time_cnt = MDIO_TIMEOUT; time_cnt; time_cnt--) {
   156			ret = regmap_read(mdio_dev->subctrl_vbase, st_reg, &reg_value);
   157			if (ret) {
 > 158				dev_err(mdio_dev->regmap->dev, "Fail to read from the register\n");
   159				return ret;
   160			}
   161	
   162			reg_value &= st_msk;
   163			if ((!!check_st) == (!!reg_value))
   164				break;
   165		}
   166	
   167		if ((!!check_st) != (!!reg_value))
   168			return -EBUSY;
   169	
   170		return 0;
   171	}
   172	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 51785 bytes --]