Re: [PATCH] of: overlay: set 'overlay_tree' and 'fdt' fields only on success

[Frank Rowand <frowand.list@gmail.com>]

Hi Slawomir,

 On 4/4/22 16:02, Frank Rowand wrote:
> On 4/4/22 15:38, Rob Herring wrote:
>> On Thu, Mar 31, 2022 at 11:56:33AM +0200, Slawomir Stepien wrote:
>>> From: Slawomir Stepien <slawomir.stepien@nokia.com>
>>>
>>> Before this change, the memory pointed by fields 'overlay_tree' and
>>> 'fdt' will be double freed by a call to free_overlay_changeset() from
>>> of_overlay_apply(), when the init_overlay_changeset() fails.
>>>
>>> The first free will happen under 'err_free_tree' label and for the
>>> second time under 'err_free_overlay_changeset' label, where we call
>>> free_overlay_changeset().
>>>
>>> This could happen for example, when you are applying an overlay to a
>>> target path that does not exists.
>>>
>>> By setting the pointers only when we are sure that
>>> init_overlay_changeset() will not fail, will prevent this double free.
>>
>> It looks to me like the free should just be moved from 
>> init_overlay_changeset() to of_overlay_fdt_apply() where the allocation 
>> is done. Frank?
> 
> This patch is next on my list to look over.

Thanks for finding this problem.  While investigating what you reported
I found that there are additional related issues.  I am in the process
of testing a patch to fix all of the issues.

-Frank

> 
> -Frank
> 
>>
>> Also, I believe there's a bug that of_overlay_apply() should be passed 
>> new_fdt_align rather than new_fdt. It's only a bug if we expect 
>> overlay_changeset.fdt to point to a valid fdt rather than the memory 
>> allocation.
>>
>> Rob
>