On kwi 07, 2022 14:53, Frank Rowand wrote: > Hi Slawomir, Hello > 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@xxxxxxxxx> > >>> > >>> 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. That sounds great! Thank you! > -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 > > > -- Slawomir Stepien