| Message ID | 20191001130921.24571-1-m.szyprowski@samsung.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | clk: samsung: exynos5433: Fix potential NULL pointer dereference | expand |
On Tue, Oct 01, 2019 at 03:09:21PM +0200, Marek Szyprowski wrote: > devm_kcalloc might fail, so avoid accessing the allocated object in such > case. > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > drivers/clk/samsung/clk-exynos5433.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/clk/samsung/clk-exynos5433.c b/drivers/clk/samsung/clk-exynos5433.c > index 7824c2ba3d8e..6afbcd0ae96f 100644 > --- a/drivers/clk/samsung/clk-exynos5433.c > +++ b/drivers/clk/samsung/clk-exynos5433.c > @@ -5592,7 +5592,8 @@ static int __init exynos5433_cmu_probe(struct platform_device *pdev) > if (data->nr_pclks > 0) { > data->pclks = devm_kcalloc(dev, sizeof(struct clk *), > data->nr_pclks, GFP_KERNEL); > - > + if (!data->pclks) > + return -ENOMEM; You leak the memory from the samsung_clk_alloc_reg_dump() call. The error path few lines later (from of_clk_get()) leaks it as well. Best regards, Krzysztof > for (i = 0; i < data->nr_pclks; i++) { > struct clk *clk = of_clk_get(dev->of_node, i); > > -- > 2.17.1 >
On 10/1/19 3:16 PM, Krzysztof Kozlowski wrote: > On Tue, Oct 01, 2019 at 03:09:21PM +0200, Marek Szyprowski wrote: >> devm_kcalloc might fail, so avoid accessing the allocated object in such >> case. >> >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >> --- >> drivers/clk/samsung/clk-exynos5433.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/clk/samsung/clk-exynos5433.c b/drivers/clk/samsung/clk-exynos5433.c >> index 7824c2ba3d8e..6afbcd0ae96f 100644 >> --- a/drivers/clk/samsung/clk-exynos5433.c >> +++ b/drivers/clk/samsung/clk-exynos5433.c >> @@ -5592,7 +5592,8 @@ static int __init exynos5433_cmu_probe(struct platform_device *pdev) >> if (data->nr_pclks > 0) { >> data->pclks = devm_kcalloc(dev, sizeof(struct clk *), >> data->nr_pclks, GFP_KERNEL); >> - >> + if (!data->pclks) >> + return -ENOMEM; > > You leak the memory from the samsung_clk_alloc_reg_dump() call. Also we may want to fix the code to check samsung_clk_alloc_reg_dump() return value for NULL while we are at it.. > The error path few lines later (from of_clk_get()) leaks it as well. > > Best regards, > Krzysztof > >> for (i = 0; i < data->nr_pclks; i++) { >> struct clk *clk = of_clk_get(dev->of_node, i); >> >> -- >> 2.17.1 Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
Hi, On 19. 10. 1. 오후 10:09, Marek Szyprowski wrote: > devm_kcalloc might fail, so avoid accessing the allocated object in such > case. > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > drivers/clk/samsung/clk-exynos5433.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/clk/samsung/clk-exynos5433.c b/drivers/clk/samsung/clk-exynos5433.c > index 7824c2ba3d8e..6afbcd0ae96f 100644 > --- a/drivers/clk/samsung/clk-exynos5433.c > +++ b/drivers/clk/samsung/clk-exynos5433.c > @@ -5592,7 +5592,8 @@ static int __init exynos5433_cmu_probe(struct platform_device *pdev) > if (data->nr_pclks > 0) { > data->pclks = devm_kcalloc(dev, sizeof(struct clk *), > data->nr_pclks, GFP_KERNEL); > - > + if (!data->pclks) > + return -ENOMEM; > for (i = 0; i < data->nr_pclks; i++) { > struct clk *clk = of_clk_get(dev->of_node, i); > > I think it is needed when 'data->pclks' memory allocation failed. Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
diff --git a/drivers/clk/samsung/clk-exynos5433.c b/drivers/clk/samsung/clk-exynos5433.c index 7824c2ba3d8e..6afbcd0ae96f 100644 --- a/drivers/clk/samsung/clk-exynos5433.c +++ b/drivers/clk/samsung/clk-exynos5433.c @@ -5592,7 +5592,8 @@ static int __init exynos5433_cmu_probe(struct platform_device *pdev) if (data->nr_pclks > 0) { data->pclks = devm_kcalloc(dev, sizeof(struct clk *), data->nr_pclks, GFP_KERNEL); - + if (!data->pclks) + return -ENOMEM; for (i = 0; i < data->nr_pclks; i++) { struct clk *clk = of_clk_get(dev->of_node, i);
devm_kcalloc might fail, so avoid accessing the allocated object in such case. Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- drivers/clk/samsung/clk-exynos5433.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)