Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <fc106a76-c661-b133-9ce0-2470ef03fefa@gmail.com>
Date:   Thu, 1 Dec 2022 16:54:33 -0600
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.2
Subject: Re: [PATCH] of: overlay: fix memory leak in add_changeset_node()
Content-Language: en-US
To:     Zeng Heng <zengheng4@huawei.com>, pantelis.antoniou@konsulko.com,
        grant.likely@linaro.org, robh+dt@kernel.org
Cc:     liwei391@huawei.com, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20221118105308.370474-1-zengheng4@huawei.com>
From:   Frank Rowand <frowand.list@gmail.com>
In-Reply-To: <20221118105308.370474-1-zengheng4@huawei.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 11/18/22 04:53, Zeng Heng wrote:
> In of_changeset_action(), we have called of_node_get() to increase
> refcount of a node.
> 
> Therefore, when tchild (duplicated by __of_node_dup()) is done,
> of_node_put() needs to call and release the device_node.
> 
> Otherwise, there are some memory leak reported about the node:
> 
> unreferenced object 0xffff88810cd1e800 (size 256):
>   backtrace:
>     kmalloc_trace
>     __of_node_dup
>     add_changeset_node (inlined)
>     build_changeset_next_level
> 
> unreferenced object 0xffff888113721240 (size 16):
>   backtrace:
>     __kmalloc_node_track_caller
>     kstrdup
>     __of_node_dup
>     add_changeset_node (inlined)
>     build_changeset_next_level
> 
> unreferenced object 0xffff88810a38d400 (size 128):
>   backtrace:
>     kmalloc_trace
>     __of_prop_dup
>     add_changeset_property
>     build_changeset_next_level
> 
> Fixes: 7518b5890d8a ("of/overlay: Introduce DT overlay support")
> Signed-off-by: Zeng Heng <zengheng4@huawei.com>
> ---
>  drivers/of/overlay.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index bd8ff4df723d..a5189a0ec0a3 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -436,8 +436,10 @@ static int add_changeset_node(struct overlay_changeset *ovcs,
>  		of_node_set_flag(tchild, OF_OVERLAY);
>  
>  		ret = of_changeset_attach_node(&ovcs->cset, tchild);
> -		if (ret)
> +		if (ret) {
> +			of_node_put(tchild);
>  			return ret;
> +		}
>  
>  		target_child.np = tchild;
>  		target_child.in_livetree = false;

Pending updated Fixes: tag (mentioned in a previous reply).

Reviewed-by: Frank Rowand <frowand.list@gmail.com>
Tested-by: Frank Rowand <frowand.list@gmail.com>

The testing was my normal testing, but did not replicate the triggered warning
to verify that this patch eliminated the warning.  I am depending upon Zeng having
verified the elimination of the warning.

-Frank