| Message ID | 20200117110201.15959-2-giulio.benetti@benettiengineering.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | fix pllv3 defects reported by Coverity | expand |
On Fri, Jan 17, 2020 at 5:02 AM Giulio Benetti <giulio.benetti at benettiengineering.com> wrote: > > Guard 'parent_rate==0' to prevent 'divide by zero' issue in > clk_pplv3_sys_get_rate(). Also, guard 'parent_rate<0' that would mean > that clk_get_parent_rate() returned an error, so better to return > early with error EINVAL. > > Signed-off-by: Giulio Benetti <giulio.benetti at benettiengineering.com> > --- > drivers/clk/imx/clk-pllv3.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c > index fc16416d5f..0d8e39cfbb 100644 > --- a/drivers/clk/imx/clk-pllv3.c > +++ b/drivers/clk/imx/clk-pllv3.c > @@ -121,10 +121,16 @@ static ulong clk_pllv3_sys_set_rate(struct clk *clk, ulong rate) > { > struct clk_pllv3 *pll = to_clk_pllv3(clk); > unsigned long parent_rate = clk_get_parent_rate(clk); parent_rate is unsigned here... > - unsigned long min_rate = parent_rate * 54 / 2; > - unsigned long max_rate = parent_rate * 108 / 2; > + unsigned long min_rate; > + unsigned long max_rate; > u32 val, div; > > + if (parent_rate <= 0) So I don't see how it is possible that it could be < 0. > + return -EINVAL; > + > + min_rate = parent_rate * 54 / 2; > + max_rate = parent_rate * 108 / 2; > + > if (rate < min_rate || rate > max_rate) > return -EINVAL; > > -- adam > 2.20.1 >
Hi Adam, On 1/17/20 12:30 PM, Adam Ford wrote: > On Fri, Jan 17, 2020 at 5:02 AM Giulio Benetti > <giulio.benetti at benettiengineering.com> wrote: >> >> Guard 'parent_rate==0' to prevent 'divide by zero' issue in >> clk_pplv3_sys_get_rate(). Also, guard 'parent_rate<0' that would mean >> that clk_get_parent_rate() returned an error, so better to return >> early with error EINVAL. >> >> Signed-off-by: Giulio Benetti <giulio.benetti at benettiengineering.com> >> --- >> drivers/clk/imx/clk-pllv3.c | 10 ++++++++-- >> 1 file changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c >> index fc16416d5f..0d8e39cfbb 100644 >> --- a/drivers/clk/imx/clk-pllv3.c >> +++ b/drivers/clk/imx/clk-pllv3.c >> @@ -121,10 +121,16 @@ static ulong clk_pllv3_sys_set_rate(struct clk *clk, ulong rate) >> { >> struct clk_pllv3 *pll = to_clk_pllv3(clk); >> unsigned long parent_rate = clk_get_parent_rate(clk); > > parent_rate is unsigned here... Oops, I didn't notice it. I'm going to guard "parent_rate==0" only then. Same for other patches I've submitted. Thank you Best regards
diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c index fc16416d5f..0d8e39cfbb 100644 --- a/drivers/clk/imx/clk-pllv3.c +++ b/drivers/clk/imx/clk-pllv3.c @@ -121,10 +121,16 @@ static ulong clk_pllv3_sys_set_rate(struct clk *clk, ulong rate) { struct clk_pllv3 *pll = to_clk_pllv3(clk); unsigned long parent_rate = clk_get_parent_rate(clk); - unsigned long min_rate = parent_rate * 54 / 2; - unsigned long max_rate = parent_rate * 108 / 2; + unsigned long min_rate; + unsigned long max_rate; u32 val, div; + if (parent_rate <= 0) + return -EINVAL; + + min_rate = parent_rate * 54 / 2; + max_rate = parent_rate * 108 / 2; + if (rate < min_rate || rate > max_rate) return -EINVAL;
Guard 'parent_rate==0' to prevent 'divide by zero' issue in clk_pplv3_sys_get_rate(). Also, guard 'parent_rate<0' that would mean that clk_get_parent_rate() returned an error, so better to return early with error EINVAL. Signed-off-by: Giulio Benetti <giulio.benetti at benettiengineering.com> --- drivers/clk/imx/clk-pllv3.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)