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 <[email protected]>
---
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, ®_value);
+ ret = regmap_read(mdio_dev->subctrl_vbase, st_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
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 <[email protected]> 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 <[email protected]>
> ---
> 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, ®_value);
> + ret = regmap_read(mdio_dev->subctrl_vbase, st_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
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 <[email protected]>
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, ®_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