Commit 067c0987 authored by Frank Rowand's avatar Frank Rowand Committed by Rob Herring

of: overlay: rework overlay apply and remove kfree()s

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: default avatarSlawomir Stepien <slawomir.stepien@nokia.com>
Signed-off-by: default avatarFrank Rowand <frank.rowand@sony.com>
Signed-off-by: default avatarRob Herring <robh@kernel.org>
Link: https://lore.kernel.org/r/20220420222505.928492-3-frowand.list@gmail.com
parent 1e408966
...@@ -119,10 +119,32 @@ Finally, if you need to remove all overlays in one-go, just call ...@@ -119,10 +119,32 @@ Finally, if you need to remove all overlays in one-go, just call
of_overlay_remove_all() which will remove every single one in the correct of_overlay_remove_all() which will remove every single one in the correct
order. order.
In addition, there is the option to register notifiers that get called on There is the option to register notifiers that get called on
overlay operations. See of_overlay_notifier_register/unregister and overlay operations. See of_overlay_notifier_register/unregister and
enum of_overlay_notify_action for details. enum of_overlay_notify_action for details.
Note that a notifier callback is not supposed to store pointers to a device A notifier callback for OF_OVERLAY_PRE_APPLY, OF_OVERLAY_POST_APPLY, or
tree node or its content beyond OF_OVERLAY_POST_REMOVE corresponding to the OF_OVERLAY_PRE_REMOVE may store pointers to a device tree node in the overlay
respective node it received. or its content but these pointers must not persist past the notifier callback
for OF_OVERLAY_POST_REMOVE. The memory containing the overlay will be
kfree()ed after OF_OVERLAY_POST_REMOVE notifiers are called. Note that the
memory will be kfree()ed even if the notifier for OF_OVERLAY_POST_REMOVE
returns an error.
The changeset notifiers in drivers/of/dynamic.c are a second type of notifier
that could be triggered by applying or removing an overlay. These notifiers
are not allowed to store pointers to a device tree node in the overlay
or its content. The overlay code does not protect against such pointers
remaining active when the memory containing the overlay is freed as a result
of removing the overlay.
Any other code that retains a pointer to the overlay nodes or data is
considered to be a bug because after removing the overlay the pointer
will refer to freed memory.
Users of overlays must be especially aware of the overall operations that
occur on the system to ensure that other kernel code does not retain any
pointers to the overlay nodes or data. Any example of an inadvertent use
of such pointers is if a driver or subsystem module is loaded after an
overlay has been applied, and the driver or subsystem scans the entire
devicetree or a large portion of it, including the overlay nodes.
This diff is collapsed.
...@@ -1543,7 +1543,8 @@ static inline bool of_device_is_system_power_controller(const struct device_node ...@@ -1543,7 +1543,8 @@ static inline bool of_device_is_system_power_controller(const struct device_node
*/ */
enum of_overlay_notify_action { enum of_overlay_notify_action {
OF_OVERLAY_PRE_APPLY = 0, OF_OVERLAY_INIT = 0, /* kzalloc() of ovcs sets this value */
OF_OVERLAY_PRE_APPLY,
OF_OVERLAY_POST_APPLY, OF_OVERLAY_POST_APPLY,
OF_OVERLAY_PRE_REMOVE, OF_OVERLAY_PRE_REMOVE,
OF_OVERLAY_POST_REMOVE, OF_OVERLAY_POST_REMOVE,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment