Skip to contentSkip to conversation

mtd: parsers: ofpart: fix OF node refcount leak in parse_fixed_partitions()

PATCH Applied Weigang He 3 participants 3 messages Jan 23 – Jan 29

Subsystems (2)
Kernel janitors Memory technology devices (MTD)
lore
Share & tools
lore

Weigang He

author
#0 lore raw

PATCH 1/1 mtd: parsers: ofpart: fix OF node refcount leak in parse_fixed_partitions()

of_get_child_by_name() returns a node pointer with refcount incremented, which must be released with of_node_put() when done. However, in parse_fixed_partitions(), when dedicated is true (i.e., a "partitions" subnode was found), the ofpart_node obtained from of_get_child_by_name() is never released on any code path.

Add of_node_put(ofpart_node) calls on all exit paths when dedicated is true to fix the reference count leak.

This bug was detected by our static analysis tool.

Fixes 562b4e91d3b2 ("mtd: parsers: ofpart: fix parsing subpartitions")
Signed-off-by Weigang He <[email protected]>
---
 drivers/mtd/parsers/ofpart_core.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)
drivers/mtd/parsers/ofpart_core.c +14 -2
index abfa687989182..09961c6f39496 100644
@@ -77,6 +77,7 @@ static int parse_fixed_partitions(struct mtd_info *master,
 	of_id = of_match_node(parse_ofpart_match_table, ofpart_node);
 	if (dedicated && !of_id) {
 		/* The 'partitions' subnode might be used by another parser */
+		of_node_put(ofpart_node);
 		return 0;
 	}
 

@@ -91,12 +92,18 @@ static int parse_fixed_partitions(struct mtd_info *master,
 		nr_parts++;
 	}
 
-	if (nr_parts == 0)
+	if (nr_parts == 0) {
+		if (dedicated)
+			of_node_put(ofpart_node);
 		return 0;
+	}
 
 	parts = kcalloc(nr_parts, sizeof(*parts), GFP_KERNEL);
-	if (!parts)
+	if (!parts) {
+		if (dedicated)
+			of_node_put(ofpart_node);
 		return -ENOMEM;
+	}
 
 	i = 0;
 	for_each_child_of_node(ofpart_node,  pp) {

@@ -175,6 +182,9 @@ static int parse_fixed_partitions(struct mtd_info *master,
 	if (quirks && quirks->post_parse)
 		quirks->post_parse(master, parts, nr_parts);
 
+	if (dedicated)
+		of_node_put(ofpart_node);
+
 	*pparts = parts;
 	return nr_parts;
 

@@ -183,6 +193,8 @@ static int parse_fixed_partitions(struct mtd_info *master,
 	       master->name, pp, mtd_node);
 	ret = -EINVAL;
 ofpart_none:
+	if (dedicated)
+		of_node_put(ofpart_node);
 	of_node_put(pp);
 	kfree(parts);
 	return ret;
Signature · 1 line
2.34.1
#1 lore raw

…

Quoted 4 lines

Add of_node_put(ofpart_node) calls on all exit paths when dedicated is true to fix the reference count leak.

This bug was detected by our static analysis tool.

Will any additional background information become more helpful here?

Regards,
Markus

maintainer Applied
#2 lore raw
In reply to Weigang He
Quoted 10 lines · Weigang He

of_get_child_by_name() returns a node pointer with refcount incremented, which must be released with of_node_put() when done. However, in parse_fixed_partitions(), when dedicated is true (i.e., a "partitions" subnode was found), the ofpart_node obtained from of_get_child_by_name() is never released on any code path.

Add of_node_put(ofpart_node) calls on all exit paths when dedicated is true to fix the reference count leak.

[...]

Applied to mtd/next, thanks!

[1/1] mtd: parsers: ofpart: fix OF node refcount leak in parse_fixed_partitions()

      commit: 7cce81df7d26d44123bd7620715c8349d96793d7

Patche(s) should be available on mtd/linux.git and will be part of the next PR (provided that no robot complains by then).

Kind regards,
Miquèl