diff mbox series

[1/3] clk: imx: pllv3: fix potential 'divide by zero' in sys_get_rate()

Message ID 20200117110201.15959-2-giulio.benetti@benettiengineering.com
State Superseded
Headers show
Series fix pllv3 defects reported by Coverity | expand

Commit Message

Giulio Benetti Jan. 17, 2020, 11:01 a.m. UTC
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(-)

Comments

Adam Ford Jan. 17, 2020, 11:30 a.m. UTC | #1
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
>
Giulio Benetti Jan. 17, 2020, 11:49 a.m. UTC | #2
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 mbox series

Patch

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;