linux-kernel - Re: [PATCH v2] clk: hisilicon: Free clk_data and unmap region obtained by of

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <3252231f-dfb9-789f-e164-11d0e579defe@huawei.com>
Date:   Tue, 10 Nov 2020 19:58:15 +0800
From:   Dongjiu Geng <gengdongjiu@...wei.com>
To:     Markus Elfring <Markus.Elfring@....de>, <linux-clk@...r.kernel.org>
CC:     <linux-kernel@...r.kernel.org>, <kernel-janitors@...r.kernel.org>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>
Subject: Re: [PATCH v2] clk: hisilicon: Free clk_data and unmap region
 obtained by of_iomap



On 2020/11/10 1:54, Markus Elfring wrote:
> …
>> +++ b/drivers/clk/hisilicon/clk-hi3620.c
> …
>> @@ -478,6 +478,10 @@ static void __init hi3620_mmc_clk_init(struct device_node *node)
>>
>>  	clk_data->clk_num = num;
>>  	of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
>> +free_clk_data:
>> +	kfree(clk_data);
> …
> 
> * Should any system resources be kept allocated if the execution
>   of this function implementation succeeded?
> 
> * How do you think about to add the statement “return;”
>   after the call of the function “of_clk_add_provider”?
> 
> * Should another return value be also checked here?

sure.

> 
>   See also:
>   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/clk.c?id=f8394f232b1eab649ce2df5c5f15b0e528c92091#n4414
>   https://elixir.bootlin.com/linux/v5.10-rc2/source/drivers/clk/clk.c#L4414
> 
> * Would you like to use the function “of_clk_add_hw_provider” instead?

    How about we still use of_clk_add_provider()? It doesn't seem to make difference using of_clk_add_hw_provider().

> 
> Regards,
> Markus