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? Yes, that would be much cleaner and proper. v2 of my follow on patch set will do so. > > 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. new_fdt is correct instead of new_fdt_align. It is only used by of_overlay_apply() and children for the purpose of kfree(). The only place that new_fdt_align is derefenced to access the data in the FDT is in of_fdt_unflatten_tree() and children, which is called by of_overlay_fdt_apply. > > Rob
