This patch series fixes two issues in the Broadcom iProc clock driver,
reported by Dan Carpenter <[email protected]>: 1) a memory leak from
the clock names; 2) 32-bit/64-bit arithmetic
Code base used is the latest linux-next (20150629) and its GITHUB link is:
https://github.com/Broadcom/cygnus-linux/tree/iproc-clk-misc-fix-v1
Ray Jui (2):
clk: iproc: fix memory leak from clock name
clk: iproc: fix bit manipulation arithmetic
drivers/clk/bcm/clk-iproc-asiu.c | 6 +-----
drivers/clk/bcm/clk-iproc-pll.c | 13 ++++---------
2 files changed, 5 insertions(+), 14 deletions(-)
--
1.7.9.5
of_property_read_string_index takes array of pointers and assign them to
strings read from device tree property. No additional memory allocation
is needed prior to calling of_property_read_string_index. In fact, since
the array of pointers will be re-assigned to other strings, any memory
that it points to prior to calling of_property_read_string_index will be
leaked
Reported-by: Dan Carpenter <[email protected]>
Signed-off-by: Ray Jui <[email protected]>
---
drivers/clk/bcm/clk-iproc-asiu.c | 6 +-----
drivers/clk/bcm/clk-iproc-pll.c | 8 +-------
2 files changed, 2 insertions(+), 12 deletions(-)
diff --git a/drivers/clk/bcm/clk-iproc-asiu.c b/drivers/clk/bcm/clk-iproc-asiu.c
index e19c09c..f630e1b 100644
--- a/drivers/clk/bcm/clk-iproc-asiu.c
+++ b/drivers/clk/bcm/clk-iproc-asiu.c
@@ -222,10 +222,6 @@ void __init iproc_asiu_setup(struct device_node *node,
struct iproc_asiu_clk *asiu_clk;
const char *clk_name;
- clk_name = kzalloc(IPROC_CLK_NAME_LEN, GFP_KERNEL);
- if (WARN_ON(!clk_name))
- goto err_clk_register;
-
ret = of_property_read_string_index(node, "clock-output-names",
i, &clk_name);
if (WARN_ON(ret))
@@ -259,7 +255,7 @@ void __init iproc_asiu_setup(struct device_node *node,
err_clk_register:
for (i = 0; i < num_clks; i++)
- kfree(asiu->clks[i].name);
+ clk_unregister(asiu->clk_data.clks[i]);
iounmap(asiu->gate_base);
err_iomap_gate:
diff --git a/drivers/clk/bcm/clk-iproc-pll.c b/drivers/clk/bcm/clk-iproc-pll.c
index 46fb84b..a8d971b 100644
--- a/drivers/clk/bcm/clk-iproc-pll.c
+++ b/drivers/clk/bcm/clk-iproc-pll.c
@@ -655,10 +655,6 @@ void __init iproc_pll_clk_setup(struct device_node *node,
memset(&init, 0, sizeof(init));
parent_name = node->name;
- clk_name = kzalloc(IPROC_CLK_NAME_LEN, GFP_KERNEL);
- if (WARN_ON(!clk_name))
- goto err_clk_register;
-
ret = of_property_read_string_index(node, "clock-output-names",
i, &clk_name);
if (WARN_ON(ret))
@@ -690,10 +686,8 @@ void __init iproc_pll_clk_setup(struct device_node *node,
return;
err_clk_register:
- for (i = 0; i < num_clks; i++) {
- kfree(pll->clks[i].name);
+ for (i = 0; i < num_clks; i++)
clk_unregister(pll->clk_data.clks[i]);
- }
err_pll_register:
if (pll->asiu_base)
--
1.7.9.5
A 32-bit variable should be type casted to 64-bit before arithmetic
operation and assigning it to a 64-bit variable
Reported-by: Dan Carpenter <[email protected]>
Signed-off-by: Ray Jui <[email protected]>
---
drivers/clk/bcm/clk-iproc-pll.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/bcm/clk-iproc-pll.c b/drivers/clk/bcm/clk-iproc-pll.c
index a8d971b..2dda4e8 100644
--- a/drivers/clk/bcm/clk-iproc-pll.c
+++ b/drivers/clk/bcm/clk-iproc-pll.c
@@ -366,7 +366,7 @@ static unsigned long iproc_pll_recalc_rate(struct clk_hw *hw,
val = readl(pll->pll_base + ctrl->ndiv_int.offset);
ndiv_int = (val >> ctrl->ndiv_int.shift) &
bit_mask(ctrl->ndiv_int.width);
- ndiv = ndiv_int << ctrl->ndiv_int.shift;
+ ndiv = (u64)ndiv_int << ctrl->ndiv_int.shift;
if (ctrl->flags & IPROC_CLK_PLL_HAS_NDIV_FRAC) {
val = readl(pll->pll_base + ctrl->ndiv_frac.offset);
@@ -374,7 +374,8 @@ static unsigned long iproc_pll_recalc_rate(struct clk_hw *hw,
bit_mask(ctrl->ndiv_frac.width);
if (ndiv_frac != 0)
- ndiv = (ndiv_int << ctrl->ndiv_int.shift) | ndiv_frac;
+ ndiv = ((u64)ndiv_int << ctrl->ndiv_int.shift) |
+ ndiv_frac;
}
val = readl(pll->pll_base + ctrl->pdiv.offset);
--
1.7.9.5
To Michael's new email.
On 6/29/2015 2:30 PM, Ray Jui wrote:
> This patch series fixes two issues in the Broadcom iProc clock driver,
> reported by Dan Carpenter <[email protected]>: 1) a memory leak from
> the clock names; 2) 32-bit/64-bit arithmetic
>
> Code base used is the latest linux-next (20150629) and its GITHUB link is:
> https://github.com/Broadcom/cygnus-linux/tree/iproc-clk-misc-fix-v1
>
> Ray Jui (2):
> clk: iproc: fix memory leak from clock name
> clk: iproc: fix bit manipulation arithmetic
>
> drivers/clk/bcm/clk-iproc-asiu.c | 6 +-----
> drivers/clk/bcm/clk-iproc-pll.c | 13 ++++---------
> 2 files changed, 5 insertions(+), 14 deletions(-)
>
+ Michael's new email
On 6/29/2015 2:30 PM, Ray Jui wrote:
> of_property_read_string_index takes array of pointers and assign them to
> strings read from device tree property. No additional memory allocation
> is needed prior to calling of_property_read_string_index. In fact, since
> the array of pointers will be re-assigned to other strings, any memory
> that it points to prior to calling of_property_read_string_index will be
> leaked
>
> Reported-by: Dan Carpenter <[email protected]>
> Signed-off-by: Ray Jui <[email protected]>
> ---
> drivers/clk/bcm/clk-iproc-asiu.c | 6 +-----
> drivers/clk/bcm/clk-iproc-pll.c | 8 +-------
> 2 files changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/clk/bcm/clk-iproc-asiu.c b/drivers/clk/bcm/clk-iproc-asiu.c
> index e19c09c..f630e1b 100644
> --- a/drivers/clk/bcm/clk-iproc-asiu.c
> +++ b/drivers/clk/bcm/clk-iproc-asiu.c
> @@ -222,10 +222,6 @@ void __init iproc_asiu_setup(struct device_node *node,
> struct iproc_asiu_clk *asiu_clk;
> const char *clk_name;
>
> - clk_name = kzalloc(IPROC_CLK_NAME_LEN, GFP_KERNEL);
> - if (WARN_ON(!clk_name))
> - goto err_clk_register;
> -
> ret = of_property_read_string_index(node, "clock-output-names",
> i, &clk_name);
> if (WARN_ON(ret))
> @@ -259,7 +255,7 @@ void __init iproc_asiu_setup(struct device_node *node,
>
> err_clk_register:
> for (i = 0; i < num_clks; i++)
> - kfree(asiu->clks[i].name);
> + clk_unregister(asiu->clk_data.clks[i]);
> iounmap(asiu->gate_base);
>
> err_iomap_gate:
> diff --git a/drivers/clk/bcm/clk-iproc-pll.c b/drivers/clk/bcm/clk-iproc-pll.c
> index 46fb84b..a8d971b 100644
> --- a/drivers/clk/bcm/clk-iproc-pll.c
> +++ b/drivers/clk/bcm/clk-iproc-pll.c
> @@ -655,10 +655,6 @@ void __init iproc_pll_clk_setup(struct device_node *node,
> memset(&init, 0, sizeof(init));
> parent_name = node->name;
>
> - clk_name = kzalloc(IPROC_CLK_NAME_LEN, GFP_KERNEL);
> - if (WARN_ON(!clk_name))
> - goto err_clk_register;
> -
> ret = of_property_read_string_index(node, "clock-output-names",
> i, &clk_name);
> if (WARN_ON(ret))
> @@ -690,10 +686,8 @@ void __init iproc_pll_clk_setup(struct device_node *node,
> return;
>
> err_clk_register:
> - for (i = 0; i < num_clks; i++) {
> - kfree(pll->clks[i].name);
> + for (i = 0; i < num_clks; i++)
> clk_unregister(pll->clk_data.clks[i]);
> - }
>
> err_pll_register:
> if (pll->asiu_base)
>
+ Michael's new email
On 6/29/2015 2:30 PM, Ray Jui wrote:
> A 32-bit variable should be type casted to 64-bit before arithmetic
> operation and assigning it to a 64-bit variable
>
> Reported-by: Dan Carpenter <[email protected]>
> Signed-off-by: Ray Jui <[email protected]>
> ---
> drivers/clk/bcm/clk-iproc-pll.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/bcm/clk-iproc-pll.c b/drivers/clk/bcm/clk-iproc-pll.c
> index a8d971b..2dda4e8 100644
> --- a/drivers/clk/bcm/clk-iproc-pll.c
> +++ b/drivers/clk/bcm/clk-iproc-pll.c
> @@ -366,7 +366,7 @@ static unsigned long iproc_pll_recalc_rate(struct clk_hw *hw,
> val = readl(pll->pll_base + ctrl->ndiv_int.offset);
> ndiv_int = (val >> ctrl->ndiv_int.shift) &
> bit_mask(ctrl->ndiv_int.width);
> - ndiv = ndiv_int << ctrl->ndiv_int.shift;
> + ndiv = (u64)ndiv_int << ctrl->ndiv_int.shift;
>
> if (ctrl->flags & IPROC_CLK_PLL_HAS_NDIV_FRAC) {
> val = readl(pll->pll_base + ctrl->ndiv_frac.offset);
> @@ -374,7 +374,8 @@ static unsigned long iproc_pll_recalc_rate(struct clk_hw *hw,
> bit_mask(ctrl->ndiv_frac.width);
>
> if (ndiv_frac != 0)
> - ndiv = (ndiv_int << ctrl->ndiv_int.shift) | ndiv_frac;
> + ndiv = ((u64)ndiv_int << ctrl->ndiv_int.shift) |
> + ndiv_frac;
> }
>
> val = readl(pll->pll_base + ctrl->pdiv.offset);
>
On 06/29, Ray Jui wrote:
> To Michael's new email.
>
> On 6/29/2015 2:30 PM, Ray Jui wrote:
> > This patch series fixes two issues in the Broadcom iProc clock driver,
> > reported by Dan Carpenter <[email protected]>: 1) a memory leak from
> > the clock names; 2) 32-bit/64-bit arithmetic
> >
> > Code base used is the latest linux-next (20150629) and its GITHUB link is:
> > https://github.com/Broadcom/cygnus-linux/tree/iproc-clk-misc-fix-v1
> >
> > Ray Jui (2):
> > clk: iproc: fix memory leak from clock name
> > clk: iproc: fix bit manipulation arithmetic
> >
Applied both to clk-fixes
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project