linux-kernel - Re: [PATCH] clk: Skip clk provider registration when np is NULL

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1a9f6d35-2c0e-df6b-f759-70edffa9c5b6@microchip.com>
Date:   Fri, 23 Apr 2021 19:16:30 +0000
From:   <Tudor.Ambarus@...rochip.com>
To:     <mturquette@...libre.com>, <sboyd@...nel.org>, <nsaenz@...nel.org>,
        <maxime@...no.tech>, <gregkh@...uxfoundation.org>,
        <rafael@...nel.org>, <khilman@...nel.org>,
        <ulf.hansson@...aro.org>, <len.brown@...el.com>, <pavel@....cz>,
        <robh+dt@...nel.org>, <frowand.list@...il.com>, <maz@...nel.org>,
        <tglx@...utronix.de>, <saravanak@...gle.com>,
        <geert@...ux-m68k.org>, <nsaenzjulienne@...e.de>,
        <linux@...ck-us.net>, <guillaume.tucker@...labora.com>
CC:     <linux-clk@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <corbet@....net>, <Nicolas.Ferre@...rochip.com>,
        <Claudiu.Beznea@...rochip.com>, <linux-doc@...r.kernel.org>,
        <linux-pm@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-acpi@...r.kernel.org>, <kernel-team@...roid.com>,
        <linux-rpi-kernel@...ts.infradead.org>, <m.szyprowski@...sung.com>
Subject: Re: [PATCH] clk: Skip clk provider registration when np is NULL

On 4/23/21 10:12 PM, Tudor Ambarus wrote:
> commit 6579c8d97ad7 ("clk: Mark fwnodes when their clock provider is added")
> revealed that clk/bcm/clk-raspberrypi.c driver calls
> devm_of_clk_add_hw_provider(), with a NULL dev->of_node, which resulted in a
> NULL pointer dereference in of_clk_add_provider() when calling
s/of_clk_add_provider()/of_clk_add_hw_provider()
> fwnode_dev_initialized().
> 
> Returning 0 is reducing the if conditions in driver code and is being
> consistent with the CONFIG_OF=n inline stub that returns 0 when CONFIG_OF
> is disabled. The downside is that drivers will maybe register clkdev lookups
> when they don't need to and waste some memory.
> 
> Reported-by: Marek Szyprowski <m.szyprowski@...sung.com>
> Fixes: 6579c8d97ad7 ("clk: Mark fwnodes when their clock provider is added")
> Signed-off-by: Tudor Ambarus <tudor.ambarus@...rochip.com>
> ---
> This would be the second approach, where we don't return an error when
> one calls devm_of_clk_add_hw_provider with a NULL of_node, but instead
> we just return 0 and skip the logic in the core and the drivers.
> 
>  drivers/clk/clk.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index e2ec1b745243..5d10da3519ac 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -4540,6 +4540,9 @@ int of_clk_add_provider(struct device_node *np,
>  	struct of_clk_provider *cp;
>  	int ret;
>  
> +	if (!np)
> +		return 0;
> +
>  	cp = kzalloc(sizeof(*cp), GFP_KERNEL);
>  	if (!cp)
>  		return -ENOMEM;
> @@ -4579,6 +4582,9 @@ int of_clk_add_hw_provider(struct device_node *np,
>  	struct of_clk_provider *cp;
>  	int ret;
>  
> +	if (!np)
> +		return 0;
> +
>  	cp = kzalloc(sizeof(*cp), GFP_KERNEL);
>  	if (!cp)
>  		return -ENOMEM;
> @@ -4676,6 +4682,9 @@ void of_clk_del_provider(struct device_node *np)
>  {
>  	struct of_clk_provider *cp;
>  
> +	if (!np)
> +		return 0;
> +
>  	mutex_lock(&of_clk_mutex);
>  	list_for_each_entry(cp, &of_clk_providers, link) {
>  		if (cp->node == np) {
>