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 of the root of the unflattened
overlay (variable tree) instead of the memory that contains
the unflattened overlay.
- Move similar kfree()s from multiple error locations to a
common error path (err_free_tree_unlocked:).
Double kfree()
Reported-by: Slawomir Stepien <slawomir.stepien@xxxxxxxxx>
Signed-off-by: Frank Rowand <frank.rowand@xxxxxxxx>
---
drivers/of/overlay.c | 64 ++++++++++++++++++++++++++++----------------
1 file changed, 41 insertions(+), 23 deletions(-)
diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index d80160cf34bb..1b9a90d61496 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -58,6 +58,7 @@ struct fragment {
* @id: changeset identifier
* @ovcs_list: list on which we are located
* @fdt: base of memory allocated to hold aligned FDT that was unflattened to create @overlay_tree
+ * @overlay_mem: the memory chunk that contains @overlay_tree
* @overlay_tree: expanded device tree that contains the fragment nodes
* @count: count of fragment structures
* @fragments: fragment nodes in the overlay expanded device tree
@@ -68,6 +69,7 @@ struct overlay_changeset {
int id;
struct list_head ovcs_list;
const void *fdt;
+ const void *overlay_mem;
struct device_node *overlay_tree;
int count;
struct fragment *fragments;
@@ -720,6 +722,7 @@ static struct device_node *find_target(struct device_node *info_node)
* init_overlay_changeset() - initialize overlay changeset from overlay tree
* @ovcs: Overlay changeset to build
* @fdt: base of memory allocated to hold aligned FDT that was unflattened to create @tree
+ * @tree_mem: Memory that contains @tree
* @tree: Contains the overlay fragments and overlay fixup nodes
*
* Initialize @ovcs. Populate @ovcs->fragments with node information from
@@ -730,13 +733,23 @@ static struct device_node *find_target(struct device_node *info_node)
* detected in @tree, or -ENOSPC if idr_alloc() error.
*/
static int init_overlay_changeset(struct overlay_changeset *ovcs,
- const void *fdt, struct device_node *tree)
+ const void *fdt, const void *tree_mem, struct device_node *tree)
{
struct device_node *node, *overlay_node;
struct fragment *fragment;
struct fragment *fragments;
int cnt, id, ret;
+ /*
+ * Must set these fields before any error return. fdt and tree_mem
+ * will be freed by free_overlay_changeset(), which is called if
+ * init_overlay_changeset() returns an error.
+ */
+
+ ovcs->fdt = fdt;
+ ovcs->overlay_mem = tree_mem;
+ ovcs->overlay_tree = tree;
+
/*
* Warn for some issues. Can not return -EINVAL for these until
* of_unittest_apply_overlay() is fixed to pass these checks.
@@ -750,9 +763,6 @@ static int init_overlay_changeset(struct overlay_changeset *ovcs,
if (!of_node_is_root(tree))
pr_debug("%s() tree is not root\n", __func__);
- ovcs->overlay_tree = tree;
- ovcs->fdt = fdt;
-
INIT_LIST_HEAD(&ovcs->ovcs_list);
of_changeset_init(&ovcs->cset);
@@ -865,7 +875,7 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs)
* ovcs->fdt due to the policy that overlay notifiers are not allowed
* to retain pointers into the overlay devicetree.
*/
- kfree(ovcs->overlay_tree);
+ kfree(ovcs->overlay_mem);
kfree(ovcs->fdt);
kfree(ovcs);
}
@@ -875,6 +885,7 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs)
*
* of_overlay_apply() - Create and apply an overlay changeset
* @fdt: base of memory allocated to hold the aligned FDT
+ * @tree_mem: Memory that contains @tree
* @tree: Expanded overlay device tree
* @ovcs_id: Pointer to overlay changeset id
*
@@ -913,31 +924,27 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs)
* id is returned to *ovcs_id.
*/
-static int of_overlay_apply(const void *fdt, struct device_node *tree,
- int *ovcs_id)
+static int of_overlay_apply(const void *fdt, void *tree_mem,
+ struct device_node *tree, int *ovcs_id)
{
struct overlay_changeset *ovcs;
int ret = 0, ret_revert, ret_tmp;
/*
- * As of this point, fdt and tree belong to the overlay changeset.
+ * As of this point, fdt and tree_mem belong to the overlay changeset.
* overlay changeset code is responsible for freeing them.
*/
if (devicetree_corrupt()) {
pr_err("devicetree state suspect, refuse to apply overlay\n");
- kfree(fdt);
- kfree(tree);
ret = -EBUSY;
- goto out;
+ goto err_free_tree_unlocked;
}
ovcs = kzalloc(sizeof(*ovcs), GFP_KERNEL);
if (!ovcs) {
- kfree(fdt);
- kfree(tree);
ret = -ENOMEM;
- goto out;
+ goto err_free_tree_unlocked;
}
of_overlay_mutex_lock();
@@ -947,9 +954,14 @@ static int of_overlay_apply(const void *fdt, struct device_node *tree,
if (ret)
goto err_free_tree;
- ret = init_overlay_changeset(ovcs, fdt, tree);
+ /*
+ * init_overlay_changeset() promises to add tree_mem and tree to ovcs
+ * even in the case of an early error return, so they can be freed by
+ * free_overlay_changeset().
+ */
+ ret = init_overlay_changeset(ovcs, fdt, tree_mem, tree);
if (ret)
- goto err_free_tree;
+ goto err_free_overlay_changeset;
/*
* after overlay_notify(), ovcs->overlay_tree related pointers may have
@@ -999,7 +1011,7 @@ static int of_overlay_apply(const void *fdt, struct device_node *tree,
err_free_tree:
kfree(fdt);
- kfree(tree);
+ kfree(tree_mem);
err_free_overlay_changeset:
free_overlay_changeset(ovcs);
@@ -1008,9 +1020,14 @@ static int of_overlay_apply(const void *fdt, struct device_node *tree,
mutex_unlock(&of_mutex);
of_overlay_mutex_unlock();
-out:
pr_debug("%s() err=%d\n", __func__, ret);
+ return ret;
+
+err_free_tree_unlocked:
+ kfree(fdt);
+ kfree(tree_mem);
+
return ret;
}
@@ -1019,6 +1036,7 @@ int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
{
void *new_fdt;
void *new_fdt_align;
+ void *overlay_mem;
int ret;
u32 size;
struct device_node *overlay_root = NULL;
@@ -1046,18 +1064,17 @@ int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
new_fdt_align = PTR_ALIGN(new_fdt, FDT_ALIGN_SIZE);
memcpy(new_fdt_align, overlay_fdt, size);
- of_fdt_unflatten_tree(new_fdt_align, NULL, &overlay_root);
- if (!overlay_root) {
+ overlay_mem = of_fdt_unflatten_tree(new_fdt_align, NULL, &overlay_root);
+ if (!overlay_mem) {
pr_err("unable to unflatten overlay_fdt\n");
ret = -EINVAL;
goto out_free_new_fdt;
}
- ret = of_overlay_apply(new_fdt, overlay_root, ovcs_id);
+ ret = of_overlay_apply(new_fdt, overlay_mem, overlay_root, ovcs_id);
if (ret < 0) {
/*
- * new_fdt and overlay_root now belong to the overlay
- * changeset.
+ * new_fdt and overlay_mem now belong to the overlay changeset.
* overlay changeset code is responsible for freeing them.
*/
goto out;
@@ -1067,6 +1084,7 @@ int of_overlay_fdt_apply(const void *overlay_fdt, u32 overlay_fdt_size,
out_free_new_fdt:
+ kfree(overlay_mem);
kfree(new_fdt);
out:
--
Frank Rowand <frank.rowand@xxxxxxxx>
