| Message ID | 20240104081446.126540-1-miquel.raynal@bootlin.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v2] mtd: Fix possible refcounting issue when going through partition nodes | expand |
On Thu, 2024-01-04 at 08:14:46 UTC, Miquel Raynal wrote: > Under normal conditions, the loop goes over all child partitions, and > 'breaks' when the relevant partition is found. In this case we get a > reference to the partition node without ever releasing it. Indeed, right > after the mtd_check_of_node() function returns, we call of_node_get() > again over this very same node. It is probably safer to keep the > counters even in this helper and call of_node_put() before break-ing. > > Reported-by: kernel test robot <lkp@intel.com> > Reported-by: Julia Lawall <julia.lawall@inria.fr> > Closes: https://lore.kernel.org/r/202312250546.ISzglvM2-lkp@intel.com/ > Cc: Christian Marangi <ansuelsmth@gmail.com> > Cc: Rafał Miłecki <rafal@milecki.pl> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/fixes. Miquel
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c index bb0759ca12f1..ff6d03f57924 100644 --- a/drivers/mtd/mtdcore.c +++ b/drivers/mtd/mtdcore.c @@ -620,6 +620,7 @@ static void mtd_check_of_node(struct mtd_info *mtd) if (plen == mtd_name_len && !strncmp(mtd->name, pname + offset, plen)) { mtd_set_of_node(mtd, mtd_dn); + of_node_put(mtd_dn); break; } }
Under normal conditions, the loop goes over all child partitions, and 'breaks' when the relevant partition is found. In this case we get a reference to the partition node without ever releasing it. Indeed, right after the mtd_check_of_node() function returns, we call of_node_get() again over this very same node. It is probably safer to keep the counters even in this helper and call of_node_put() before break-ing. Reported-by: kernel test robot <lkp@intel.com> Reported-by: Julia Lawall <julia.lawall@inria.fr> Closes: https://lore.kernel.org/r/202312250546.ISzglvM2-lkp@intel.com/ Cc: Christian Marangi <ansuelsmth@gmail.com> Cc: Rafał Miłecki <rafal@milecki.pl> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> --- This is compile-tested only. v2: Don't move the of_node_put(partitions) but add an of_node_put(mtd_dn) instead, which looks more legitimate in this case. Indeed, the 'partitions' node is acquired before the loop and released after, which seems safe. However when we break the loop we apparently leak a reference over mtd_dn instead. --- drivers/mtd/mtdcore.c | 1 + 1 file changed, 1 insertion(+)