From patchwork Tue Feb 24 13:35:05 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: physmap: Fix leak of memory returned by parse_mtd_partitions Date: Tue, 24 Feb 2009 03:35:05 -0000 From: Sascha Hauer X-Patchwork-Id: 23617 Message-Id: <20090224133505.GJ21900@pengutronix.de> To: Atsushi Nemoto Cc: David Woodhouse , linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org Hi, Sorry to reply to such an old thread, but... On Wed, Nov 12, 2008 at 11:57:33PM +0900, Atsushi Nemoto wrote: > The mtd partition parser returns an allocated pointer array of > mtd_partition. The caller must free it. The array is used only for > add_mtd_partitions(), so free it just after the call. This patch breaks command line parsing support. With command line partition parsing the struct mtd_partition array is allocated, but only once. On my board with NAND and NOR (both with command line partition parsing) It fails badly in parse_cmdline_partitions() when the second device gets parsed. The following patch fixes it, but I don't know if this is the correct solution. Does anybody have more insights on this? Sascha > > Signed-off-by: Atsushi Nemoto > --- > drivers/mtd/maps/physmap.c | 17 ++++++++--------- > 1 files changed, 8 insertions(+), 9 deletions(-) > > diff --git a/drivers/mtd/maps/physmap.c b/drivers/mtd/maps/physmap.c > index 7ca048d..cc26b41 100644 > --- a/drivers/mtd/maps/physmap.c > +++ b/drivers/mtd/maps/physmap.c > @@ -29,7 +29,6 @@ struct physmap_flash_info { > struct map_info map[MAX_RESOURCES]; > #ifdef CONFIG_MTD_PARTITIONS > int nr_parts; > - struct mtd_partition *parts; > #endif > }; > > @@ -56,14 +55,10 @@ static int physmap_flash_remove(struct platform_device *dev) > for (i = 0; i < MAX_RESOURCES; i++) { > if (info->mtd[i] != NULL) { > #ifdef CONFIG_MTD_PARTITIONS > - if (info->nr_parts) { > + if (info->nr_parts || physmap_data->nr_parts) > del_mtd_partitions(info->mtd[i]); > - kfree(info->parts); > - } else if (physmap_data->nr_parts) { > - del_mtd_partitions(info->mtd[i]); > - } else { > + else > del_mtd_device(info->mtd[i]); > - } > #else > del_mtd_device(info->mtd[i]); > #endif > @@ -86,6 +81,9 @@ static int physmap_flash_probe(struct platform_device *dev) > int err = 0; > int i; > int devices_found = 0; > +#ifdef CONFIG_MTD_PARTITIONS > + struct mtd_partition *parts; > +#endif > > physmap_data = dev->dev.platform_data; > if (physmap_data == NULL) > @@ -166,9 +164,10 @@ static int physmap_flash_probe(struct platform_device *dev) > goto err_out; > > #ifdef CONFIG_MTD_PARTITIONS > - err = parse_mtd_partitions(info->cmtd, part_probe_types, &info->parts, 0); > + err = parse_mtd_partitions(info->cmtd, part_probe_types, &parts, 0); > if (err > 0) { > - add_mtd_partitions(info->cmtd, info->parts, err); > + add_mtd_partitions(info->cmtd, parts, err); > + kfree(parts); > return 0; > } > > -- > 1.5.6.3 > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/ > diff --git a/drivers/mtd/cmdlinepart.c b/drivers/mtd/cmdlinepart.c index 50a3403..14c00dd 100644 --- a/drivers/mtd/cmdlinepart.c +++ b/drivers/mtd/cmdlinepart.c @@ -335,7 +335,13 @@ static int parse_cmdline_partitions(struct mtd_info *master, } offset += part->parts[i].size; } - *pparts = part->parts; + + *pparts = kmalloc(sizeof (struct mtd_partition) * part->num_parts, GFP_KERNEL); + if (!*pparts) + return -ENOMEM; + + memcpy(*pparts, part->parts, sizeof (struct mtd_partition) * part->num_parts); + return part->num_parts; } }