From 067c098766c6af667a9002d4e33cf1f3c998abbe Mon Sep 17 00:00:00 2001 From: Frank Rowand Date: Wed, 20 Apr 2022 17:25:05 -0500 Subject: 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: Slawomir Stepien Signed-off-by: Frank Rowand Signed-off-by: Rob Herring Link: https://lore.kernel.org/r/20220420222505.928492-3-frowand.list@gmail.com --- include/linux/of.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'include') diff --git a/include/linux/of.h b/include/linux/of.h index 04971e85fbc9..17741eee0ca4 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -1543,7 +1543,8 @@ static inline bool of_device_is_system_power_controller(const struct device_node */ 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_PRE_REMOVE, OF_OVERLAY_POST_REMOVE, -- cgit v1.2.3 From 1ac17586c950a2c129393f8a92901a2b357acf24 Mon Sep 17 00:00:00 2001 From: Frank Rowand Date: Mon, 2 May 2022 13:17:40 -0500 Subject: of: overlay: add entry to of_overlay_action_name[] The values of enum of_overlay_notify_action are used to index into array of_overlay_action_name. Add an entry to of_overlay_action_name for the value recently added to of_overlay_notify_action. Array of_overlay_action_name[] is moved into include/linux/of.h adjacent to enum of_overlay_notify_action to make the connection between the two more obvious if either is modified in the future. The only use of of_overlay_action_name is for error reporting in overlay_notify(). All callers of overlay_notify() report the same error, but with fewer details. Remove the redundant error reports in the callers. Fixes: 067c098766c6 ("of: overlay: rework overlay apply and remove kfree()s") Reported-by: Dan Carpenter Signed-off-by: Frank Rowand Signed-off-by: Rob Herring Link: https://lore.kernel.org/r/20220502181742.1402826-2-frowand.list@gmail.com --- drivers/of/overlay.c | 27 +++++---------------------- include/linux/of.h | 13 +++++++++++++ 2 files changed, 18 insertions(+), 22 deletions(-) (limited to 'include') diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index bd522da7f76b..ae5ea5b1079b 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -152,13 +152,6 @@ int of_overlay_notifier_unregister(struct notifier_block *nb) } EXPORT_SYMBOL_GPL(of_overlay_notifier_unregister); -static char *of_overlay_action_name[] = { - "pre-apply", - "post-apply", - "pre-remove", - "post-remove", -}; - static int overlay_notify(struct overlay_changeset *ovcs, enum of_overlay_notify_action action) { @@ -178,7 +171,7 @@ static int overlay_notify(struct overlay_changeset *ovcs, if (notifier_to_errno(ret)) { ret = notifier_to_errno(ret); pr_err("overlay changeset %s notifier error %d, target: %pOF\n", - of_overlay_action_name[action], ret, nd.target); + of_overlay_action_name(action), ret, nd.target); return ret; } } @@ -925,10 +918,8 @@ static int of_overlay_apply(struct overlay_changeset *ovcs) goto out; ret = overlay_notify(ovcs, OF_OVERLAY_PRE_APPLY); - if (ret) { - pr_err("overlay changeset pre-apply notify error %d\n", ret); + if (ret) goto out; - } ret = build_changeset(ovcs); if (ret) @@ -951,12 +942,9 @@ static int of_overlay_apply(struct overlay_changeset *ovcs) /* notify failure is not fatal, continue */ ret_tmp = overlay_notify(ovcs, OF_OVERLAY_POST_APPLY); - if (ret_tmp) { - pr_err("overlay changeset post-apply notify error %d\n", - ret_tmp); + if (ret_tmp) if (!ret) ret = ret_tmp; - } out: pr_debug("%s() err=%d\n", __func__, ret); @@ -1192,10 +1180,8 @@ int of_overlay_remove(int *ovcs_id) } ret = overlay_notify(ovcs, OF_OVERLAY_PRE_REMOVE); - if (ret) { - pr_err("overlay changeset pre-remove notify error %d\n", ret); + if (ret) goto err_unlock; - } ret_apply = 0; ret = __of_changeset_revert_entries(&ovcs->cset, &ret_apply); @@ -1218,12 +1204,9 @@ int of_overlay_remove(int *ovcs_id) * OF_OVERLAY_POST_REMOVE returns an error. */ ret_tmp = overlay_notify(ovcs, OF_OVERLAY_POST_REMOVE); - if (ret_tmp) { - pr_err("overlay changeset post-remove notify error %d\n", - ret_tmp); + if (ret_tmp) if (!ret) ret = ret_tmp; - } free_overlay_changeset(ovcs); diff --git a/include/linux/of.h b/include/linux/of.h index 17741eee0ca4..f0a5d6b10c5a 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -1550,6 +1550,19 @@ enum of_overlay_notify_action { OF_OVERLAY_POST_REMOVE, }; +static inline char *of_overlay_action_name(enum of_overlay_notify_action action) +{ + static char *of_overlay_action_name[] = { + "init", + "pre-apply", + "post-apply", + "pre-remove", + "post-remove", + }; + + return of_overlay_action_name[action]; +} + struct of_overlay_notify_data { struct device_node *overlay; struct device_node *target; -- cgit v1.2.3