[PATCH 0/3] fix pllv3 defects reported by Coverity
Giulio Benetti (3): clk: imx: pllv3: fix potential 'divide by zero' in sys_get_rate() clk: imx: pllv3: fix potential 'divide by zero' in av_get_rate() clk: imx: pllv3: fix potential 'divide by zero' in av_set_rate()
drivers/clk/imx/clk-pllv3.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-)
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@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); - 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;
On Fri, Jan 17, 2020 at 5:02 AM Giulio Benetti giulio.benetti@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@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@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@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
V1->V2 * check only if parent_rate==0, as signalled by Adam
Giulio Benetti (3): clk: imx: pllv3: fix potential 'divide by zero' in sys_get_rate() clk: imx: pllv3: fix potential 'divide by zero' in av_get_rate() clk: imx: pllv3: fix potential 'divide by zero' in av_set_rate()
drivers/clk/imx/clk-pllv3.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-)
Guard 'parent_rate==0' to prevent 'divide by zero' issue in clk_pplv3_sys_get_rate(). If it is 0, let's return with -EINVAL.
Signed-off-by: Giulio Benetti giulio.benetti@benettiengineering.com --- V1->V2 * check only if parent_rate==0, as signalled by Adam --- 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..a540a5b68c 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 'mfd==0' to prevent 'divide by zero' issue in clk_pplv3_av_get_rate(). If it is 0, let's return with EIO since mfd should never be 0 at all.
Signed-off-by: Giulio Benetti giulio.benetti@benettiengineering.com --- V1->V2 * improved a little commit log --- drivers/clk/imx/clk-pllv3.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c index a540a5b68c..72e6750615 100644 --- a/drivers/clk/imx/clk-pllv3.c +++ b/drivers/clk/imx/clk-pllv3.c @@ -163,6 +163,9 @@ static ulong clk_pllv3_av_get_rate(struct clk *clk) u32 div = readl(pll->base) & pll->div_mask; u64 temp64 = (u64)parent_rate;
+ if (mfd == 0) + return -EIO; + temp64 *= mfn; do_div(temp64, mfd);
Guard 'parent_rate==0' to prevent 'divide by zero' issue in clk_pplv3_av_set_rate(). If it is 0, let's return with -EINVAL.
Signed-off-by: Giulio Benetti giulio.benetti@benettiengineering.com --- V1->V2 * check only if parent_rate==0, as signalled by Adam --- 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 72e6750615..0cdb9df45d 100644 --- a/drivers/clk/imx/clk-pllv3.c +++ b/drivers/clk/imx/clk-pllv3.c @@ -176,13 +176,19 @@ static ulong clk_pllv3_av_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 * 27; - unsigned long max_rate = parent_rate * 54; + unsigned long min_rate; + unsigned long max_rate; u32 val, div; u32 mfn, mfd = 1000000; u32 max_mfd = 0x3FFFFFFF; u64 temp64;
+ if (parent_rate == 0) + return -EINVAL; + + min_rate = parent_rate * 27; + max_rate = parent_rate * 54; + if (rate < min_rate || rate > max_rate) return -EINVAL;
On 1/17/20 1:06 PM, Giulio Benetti wrote:
V1->V2
- check only if parent_rate==0, as signalled by Adam
Giulio Benetti (3): clk: imx: pllv3: fix potential 'divide by zero' in sys_get_rate() clk: imx: pllv3: fix potential 'divide by zero' in av_get_rate() clk: imx: pllv3: fix potential 'divide by zero' in av_set_rate()
drivers/clk/imx/clk-pllv3.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-)
Kindly ping
Guard 'mfd==0' to prevent 'divide by zero' issue in clk_pplv3_av_get_rate() and returning early with error EIO since mfd should never be 0 at all.
Signed-off-by: Giulio Benetti giulio.benetti@benettiengineering.com --- drivers/clk/imx/clk-pllv3.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c index 0d8e39cfbb..d2bb1e86e1 100644 --- a/drivers/clk/imx/clk-pllv3.c +++ b/drivers/clk/imx/clk-pllv3.c @@ -163,6 +163,9 @@ static ulong clk_pllv3_av_get_rate(struct clk *clk) u32 div = readl(pll->base) & pll->div_mask; u64 temp64 = (u64)parent_rate;
+ if (mfd == 0) + return -EIO; + temp64 *= mfn; do_div(temp64, mfd);
Guard 'parent_rate==0' to prevent 'divide by zero' issue in clk_pplv3_av_set_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@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 d2bb1e86e1..5383ef4816 100644 --- a/drivers/clk/imx/clk-pllv3.c +++ b/drivers/clk/imx/clk-pllv3.c @@ -176,13 +176,19 @@ static ulong clk_pllv3_av_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 * 27; - unsigned long max_rate = parent_rate * 54; + unsigned long min_rate; + unsigned long max_rate; u32 val, div; u32 mfn, mfd = 1000000; u32 max_mfd = 0x3FFFFFFF; u64 temp64;
+ if (parent_rate <= 0) + return -EINVAL; + + min_rate = parent_rate * 27; + max_rate = parent_rate * 54; + if (rate < min_rate || rate > max_rate) return -EINVAL;
participants (2)
-
Adam Ford -
Giulio Benetti