linux-kernel - Re: [PATCH] driver: of: overlay: demote message to warning

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]

Date:   Mon, 12 Sep 2022 01:45:40 -0500
From:   Frank Rowand <frowand.list@...il.com>
To:     Daniel Walker <danielwa@...co.com>
Cc:     Pantelis Antoniou <pantelis.antoniou@...sulko.com>,
        Rob Herring <robh+dt@...nel.org>, xe-linux-external@...co.com,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] driver: of: overlay: demote message to warning

On 9/8/22 12:55, Frank Rowand wrote:
> On 9/7/22 19:35, Daniel Walker wrote:
>> On Wed, Sep 07, 2022 at 06:54:02PM -0500, Frank Rowand wrote:
>>> On 9/7/22 18:07, Daniel Walker wrote:
>>>> This warning message shows by default on the vast majority of overlays
>>>> applied. Despite the text identifying this as a warning it is marked
>>>> with the loglevel for error. At Cisco we filter the loglevels to only
>>>> show error messages. We end up seeing this message but it's not really
>>>> an error.
>>>>
>>>> For this reason it makes sense to demote the message to the warning
>>>> loglevel.
>>>>
>>>> Cc: xe-linux-external@...co.com
>>>> Signed-off-by: Daniel Walker <danielwa@...co.com>
>>>> ---
>>>>  drivers/of/overlay.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
>>>> index bd8ff4df723d..4ae276ed9a65 100644
>>>> --- a/drivers/of/overlay.c
>>>> +++ b/drivers/of/overlay.c
>>>> @@ -358,7 +358,7 @@ static int add_changeset_property(struct overlay_changeset *ovcs,
>>>>  	}
>>>>  
>>>>  	if (!of_node_check_flag(target->np, OF_OVERLAY))
>>>> -		pr_err("WARNING: memory leak will occur if overlay removed, property: %pOF/%s\n",
>>>> +		pr_warn("WARNING: memory leak will occur if overlay removed, property: %pOF/%s\n",
>>>>  		       target->np, new_prop->name);
>>>>  
>>>>  	if (ret) {
>>>
>>> NACK
>>>
>>> This is showing a real problem with the overlay.
>>
>> What's the real problem ?
>>
>> Daniel
> 
> A memory leak when the overlay is removed.
> 
> I'll send a patch to update the overlay file in Documumentation/devicetree/ to provide
> more information about this.  If you don't see a patch by tomorrow, feel free to
> ping me.
> 
> -Frank

The good news is that your question prodded me to start improving the in kernel documentation
of overlays.  The promised patch is a rough start at:

   https://lore.kernel.org/all/20220912062615.3727029-1-frowand.list@gmail.com/

The bad news is that what I wrote doesn't explain the memory leak in any more detail.
If an overlay adds a property to a node in the base device tree then the memory
allocated to do the add will not be freed when the overlay is removed.  Since it is
possible to add and remove overlays multiple times, the ensuing size of the memory
leak is potentially unbounded.

-Frank