On Wed, 20 Apr 2022 17:25:05 -0500, frowand.list@xxxxxxxxx wrote: > From: Frank Rowand <frank.rowand@xxxxxxxx> > > Fix various kfree() issues related to of_overlay_apply(). > - Double kfree() of fdt and tree when init_overlay_changeset() > returns an error. > - free_overlay_changeset() free the root of the unflattened > overlay (variable tree) instead of the memory that contains > the unflattened overlay. > - For the case of a failure during applying an overlay, move kfree() > of new_fdt and overlay_mem into free_overlay_changeset(), which > is called by the function that allocated them. > - For the case of removing an overlay, the kfree() of new_fdt and > overlay_mem remains in free_overlay_changeset(). > - Check return value of of_fdt_unflatten_tree() for error instead > of checking the returned value of overlay_root. > - When storing pointers to allocated objects in ovcs, do so as > near to the allocation as possible instead of in deeply layered > function. > > More clearly document policy related to lifetime of pointers into > overlay memory. > > Double kfree() > Reported-by: Slawomir Stepien <slawomir.stepien@xxxxxxxxx> > > Signed-off-by: Frank Rowand <frank.rowand@xxxxxxxx> > > --- > > Chose value of zero for OF_OVERLAY_INIT instead of -1, so that the > value of ovcs->notify_state would be properly initialed by the > kzalloc() of ovcs. > > My first reply to this email will be a diff of this patch vs > v3 of this patch. > > Changes since v3: > - Instead of field kfree_unsafe in struct overlay_changeset, add > field notify_state to track action of most recent notification, > allowing the free_overlay_changeset() code to choose whether > to free overlay_mem and new_fdt. > - Add new value OF_OVERLAY_INIT to enum of_overlay_notify_action > > Changes since v2: > - A version 2 review comment correctly said "This screams hack". > Restructure as listed below in response to the comment. > - Quit passing kfree_unsafe in function parameters, move it to > be a field of ovcs > - Quit passing a bunch of objects as function parameters, which > were used only for saving in ovcs > - Save pointers to allocated objects as near to the allocation as > possible instead of in a different function. > - Move object allocation as early in the calling stack (starting > at of_overlay_fdt_apply()) and move object freeing more fully > into free_overlay_changeset(), which is called in two places: > (1) on error path of applying overlay and (2) removal of an > overlay by of_overlay_remove(). > - Additional notes in the overlay documentation regarding pointers > into overlay nodes and data. > > Changes since v1: > - Move kfree()s from init_overlay_changeset() to of_overlay_fdt_apply() > - Better document lifetime of pointers into overlay, both in overlay.c > and Documentation/devicetree/overlay-notes.rst > > Documentation/devicetree/overlay-notes.rst | 30 ++- > drivers/of/overlay.c | 263 ++++++++++----------- > include/linux/of.h | 3 +- > 3 files changed, 153 insertions(+), 143 deletions(-) > Applied, thanks!
