Messages in this thread Patch in this message | | | Date | Wed, 22 Nov 2023 14:12:22 +0800 | Subject | Re: [PATCH] ARM: imx: Check return value of devm_kasprintf in imx_mmdc_perf_init | From | Kunwu Chan <> |
| |
Hi,Ahmad
Thank you for your guidance to me It's my bad, i will update a v2 patch follow your suggestions, and send it in a new thread.
v2 changes: 1、use the first commit 'e76bdfd7403a' in the 'Fixes' tag. 2、Add new tag 'pmu_release_id' to release the id allocated in 'mmdc_pmu_init', move 'ida_simple_remove(&mmdc_ida, pmu_mmdc->id);' into 'pmu_release_id'.
the v2 patch look like:
+ if (!name) { + ret = -ENOMEM; + goto pmu_release_id; + }
pmu_mmdc->mmdc_ipg_clk = mmdc_ipg_clk; pmu_mmdc->devtype_data = (struct fsl_mmdc_devtype_data *)of_id->data; @@ -523,9 +527,10 @@ static int imx_mmdc_perf_init(struct platform_device *pdev, void __iomem *mmdc_b
pmu_register_err: pr_warn("MMDC Perf PMU failed (%d), disabled\n", ret); - ida_simple_remove(&mmdc_ida, pmu_mmdc->id); cpuhp_state_remove_instance_nocalls(cpuhp_mmdc_state, &pmu_mmdc->node); hrtimer_cancel(&pmu_mmdc->hrtimer); +pmu_release_id: + ida_simple_remove(&mmdc_ida, pmu_mmdc->id); pmu_free:
Thanks, Kunwu On 2023/11/21 17:37, Ahmad Fatoum wrote: > Hello Kunwu, > > On 21.11.23 10:25, Kunwu Chan wrote: >> devm_kasprintf() returns a pointer to dynamically allocated memory >> which can be NULL upon failure. Ensure the allocation was successful >> by checking the pointer validity. >> >> Fixes: ebeb49f43c89 ("ARM: imx: Call ida_simple_remove() for ida_simple_get") > > This commit only moves the allocation around, but it didn't introduce it. > Please reference the first commit that added the allocation. > >> Signed-off-by: Kunwu Chan <chentao@kylinos.cn> >> --- >> arch/arm/mach-imx/mmdc.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/arch/arm/mach-imx/mmdc.c b/arch/arm/mach-imx/mmdc.c >> index 2157493b78a9..7c471d6a851d 100644 >> --- a/arch/arm/mach-imx/mmdc.c >> +++ b/arch/arm/mach-imx/mmdc.c >> @@ -501,6 +501,10 @@ static int imx_mmdc_perf_init(struct platform_device *pdev, void __iomem *mmdc_b >> >> name = devm_kasprintf(&pdev->dev, >> GFP_KERNEL, "mmdc%d", ret); >> + if (!name) { >> + ret = -ENOMEM; >> + goto pmu_free; > > Cleanup is incomplete if you goto pmu_free, e.g. ida_simple_remove() > isn't called. pmu_register_err does too much cleanup, so you'll need > to add a new cleanup label. > > Cheers, > Ahmad > > >> + } >> >> pmu_mmdc->mmdc_ipg_clk = mmdc_ipg_clk; >> pmu_mmdc->devtype_data = (struct fsl_mmdc_devtype_data *)of_id->data; >
| |