| Message ID | 20190127051012.5928-1-yzhai003@ucr.edu (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | clk: gemini: Variable "val" in function gemini_clk_probe() could be uninitialized | expand |
Quoting Yizhuo (2019-01-26 21:10:12) > In function gemini_clk_probe(), local variable "val" could > be uninitialized if function regmap_read() returns -EINVAL. > However, it will be used as index in the later context, which > could potentially be unsafe. Ok. How did you find this? Any pointers? > > Signed-off-by: Yizhuo <yzhai003@ucr.edu> > --- > drivers/clk/clk-gemini.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) There are other locations in this file where 'val' is used even if regmap_read() fails. For example gemini_pci_recalc_rate() does this. Can you fix all the callers in this file? Presumably nobody cares that this API could fail in this driver because it's a thin wrapper around mmio read that never fails. Maybe we could have a comment instead that this is the case and then ignore this patch entirely.
diff --git a/drivers/clk/clk-gemini.c b/drivers/clk/clk-gemini.c index 5e66e6c0205e..07e1b551d1d9 100644 --- a/drivers/clk/clk-gemini.c +++ b/drivers/clk/clk-gemini.c @@ -314,7 +314,10 @@ static int gemini_clk_probe(struct platform_device *pdev) gemini_clk_data->hws[GEMINI_CLK_RTC] = hw; /* CPU clock derived as a fixed ratio from the AHB clock */ - regmap_read(map, GEMINI_GLOBAL_STATUS, &val); + ret = regmap_read(map, GEMINI_GLOBAL_STATUS, &val); + if (ret) + return ret; + val >>= CPU_AHB_RATIO_SHIFT; val &= CPU_AHB_RATIO_MASK; hw = clk_hw_register_fixed_factor(NULL, "cpu", "ahb", 0, @@ -323,7 +326,10 @@ static int gemini_clk_probe(struct platform_device *pdev) gemini_clk_data->hws[GEMINI_CLK_CPU] = hw; /* Security clock is 1:1 or 0.75 of APB */ - regmap_read(map, GEMINI_GLOBAL_CLOCK_CONTROL, &val); + ret = regmap_read(map, GEMINI_GLOBAL_CLOCK_CONTROL, &val); + if (ret) + return ret; + if (val & SECURITY_CLK_SEL) { mult = 1; div = 1;
In function gemini_clk_probe(), local variable "val" could be uninitialized if function regmap_read() returns -EINVAL. However, it will be used as index in the later context, which could potentially be unsafe. Signed-off-by: Yizhuo <yzhai003@ucr.edu> --- drivers/clk/clk-gemini.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)