* [BUG] physmap_flash_probe() frees memory still in use
@ 2009-01-30 13:43 ` Matthias Kaehlcke
0 siblings, 0 replies; 6+ messages in thread
From: Matthias Kaehlcke @ 2009-01-30 13:43 UTC (permalink / raw)
To: Atsushi Nemoto
Cc: Andrew Morton, linux-mtd, David Woodhouse, Mike Frysinger,
linux-kernel
as i reported earlier in this month
(http://lkml.org/lkml/2009/1/12/141) MTD partition names are screwed
up in 2.6.29-rcX.
commit 176bf2e0f10ecf1d20a97db3bd5bb2e6ba0b5668 deals with a memory
leak and frees the pointer array of mtd_partition after the call to
add_mtd_partitions(). the problem is that mtd_table[x]->name still
points to the freed memory.
--
Matthias Kaehlcke
Embedded Linux Engineer
Barcelona
For to be free is not merely to cast off
one's chains, but to live in a way that
respects and enhances the freedom of others
(Nelson Mandela)
.''`.
using free software / Debian GNU/Linux | http://debian.org : :' :
`. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4 `-
^ permalink raw reply [flat|nested] 6+ messages in thread* [BUG] physmap_flash_probe() frees memory still in use @ 2009-01-30 13:43 ` Matthias Kaehlcke 0 siblings, 0 replies; 6+ messages in thread From: Matthias Kaehlcke @ 2009-01-30 13:43 UTC (permalink / raw) To: Atsushi Nemoto Cc: Andrew Morton, David Woodhouse, Mike Frysinger, linux-mtd, linux-kernel as i reported earlier in this month (http://lkml.org/lkml/2009/1/12/141) MTD partition names are screwed up in 2.6.29-rcX. commit 176bf2e0f10ecf1d20a97db3bd5bb2e6ba0b5668 deals with a memory leak and frees the pointer array of mtd_partition after the call to add_mtd_partitions(). the problem is that mtd_table[x]->name still points to the freed memory. -- Matthias Kaehlcke Embedded Linux Engineer Barcelona For to be free is not merely to cast off one's chains, but to live in a way that respects and enhances the freedom of others (Nelson Mandela) .''`. using free software / Debian GNU/Linux | http://debian.org : :' : `. `'` gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4 `- ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [BUG] physmap_flash_probe() frees memory still in use 2009-01-30 13:43 ` Matthias Kaehlcke @ 2009-01-30 15:52 ` Atsushi Nemoto -1 siblings, 0 replies; 6+ messages in thread From: Atsushi Nemoto @ 2009-01-30 15:52 UTC (permalink / raw) To: matthias; +Cc: akpm, linux-mtd, David.Woodhouse, vapier.adi, linux-kernel On Fri, 30 Jan 2009 14:43:18 +0100, Matthias Kaehlcke <matthias@kaehlcke.net> wrote: > as i reported earlier in this month > (http://lkml.org/lkml/2009/1/12/141) MTD partition names are screwed > up in 2.6.29-rcX. > > commit 176bf2e0f10ecf1d20a97db3bd5bb2e6ba0b5668 deals with a memory > leak and frees the pointer array of mtd_partition after the call to > add_mtd_partitions(). the problem is that mtd_table[x]->name still > points to the freed memory. Oh Yes, I had missed that point. And looking at physmap driver again, I found that del_mtd_partition() and del_mtd_device() are abused. Though these APIs are robust enough, deleting right things in right order would be better. Could you test this patch? I cannot test it until Monday. ------------------------------------------------------ Subject: [MTD] physmap: Fix wrong free and del_mtd_{partition,device} From: Atsushi Nemoto <anemo@mba.ocn.ne.jp> commit 176bf2e0f10ecf1d20a97db3bd5bb2e6ba0b5668 deals with a memory leak and frees the pointer array of mtd_partition after the call to add_mtd_partitions(). the problem is that mtd_table[x]->name still points to the freed memory. Aldo physmap_flash_remove() should call del_mtd_partitions() or del_mtd_device() only once. Reported-by: Matthias Kaehlcke <matthias@kaehlcke.net> Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp> --- diff --git a/drivers/mtd/maps/physmap.c b/drivers/mtd/maps/physmap.c index 8774366..4b122e7 100644 --- a/drivers/mtd/maps/physmap.c +++ b/drivers/mtd/maps/physmap.c @@ -29,6 +29,7 @@ struct physmap_flash_info { struct map_info map[MAX_RESOURCES]; #ifdef CONFIG_MTD_PARTITIONS int nr_parts; + struct mtd_partition *parts; #endif }; @@ -45,25 +46,26 @@ static int physmap_flash_remove(struct platform_device *dev) physmap_data = dev->dev.platform_data; -#ifdef CONFIG_MTD_CONCAT - if (info->cmtd != info->mtd[0]) { +#ifdef CONFIG_MTD_PARTITIONS + if (info->nr_parts) { + del_mtd_partitions(info->cmtd); + kfree(info->parts); + } else if (physmap_data->nr_parts) + del_mtd_partitions(info->cmtd); + else del_mtd_device(info->cmtd); +#else + del_mtd_device(info->cmtd); +#endif + +#ifdef CONFIG_MTD_CONCAT + if (info->cmtd != info->mtd[0]) mtd_concat_destroy(info->cmtd); - } #endif for (i = 0; i < MAX_RESOURCES; i++) { - if (info->mtd[i] != NULL) { -#ifdef CONFIG_MTD_PARTITIONS - if (info->nr_parts || physmap_data->nr_parts) - del_mtd_partitions(info->mtd[i]); - else - del_mtd_device(info->mtd[i]); -#else - del_mtd_device(info->mtd[i]); -#endif + if (info->mtd[i] != NULL) map_destroy(info->mtd[i]); - } } return 0; } @@ -86,9 +88,6 @@ 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) @@ -167,10 +166,11 @@ 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, &parts, 0); + err = parse_mtd_partitions(info->cmtd, part_probe_types, + &info->parts, 0); if (err > 0) { - add_mtd_partitions(info->cmtd, parts, err); - kfree(parts); + add_mtd_partitions(info->cmtd, info->parts, err); + info->nr_parts = err; return 0; } ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [BUG] physmap_flash_probe() frees memory still in use @ 2009-01-30 15:52 ` Atsushi Nemoto 0 siblings, 0 replies; 6+ messages in thread From: Atsushi Nemoto @ 2009-01-30 15:52 UTC (permalink / raw) To: matthias; +Cc: akpm, David.Woodhouse, vapier.adi, linux-mtd, linux-kernel On Fri, 30 Jan 2009 14:43:18 +0100, Matthias Kaehlcke <matthias@kaehlcke.net> wrote: > as i reported earlier in this month > (http://lkml.org/lkml/2009/1/12/141) MTD partition names are screwed > up in 2.6.29-rcX. > > commit 176bf2e0f10ecf1d20a97db3bd5bb2e6ba0b5668 deals with a memory > leak and frees the pointer array of mtd_partition after the call to > add_mtd_partitions(). the problem is that mtd_table[x]->name still > points to the freed memory. Oh Yes, I had missed that point. And looking at physmap driver again, I found that del_mtd_partition() and del_mtd_device() are abused. Though these APIs are robust enough, deleting right things in right order would be better. Could you test this patch? I cannot test it until Monday. ------------------------------------------------------ Subject: [MTD] physmap: Fix wrong free and del_mtd_{partition,device} From: Atsushi Nemoto <anemo@mba.ocn.ne.jp> commit 176bf2e0f10ecf1d20a97db3bd5bb2e6ba0b5668 deals with a memory leak and frees the pointer array of mtd_partition after the call to add_mtd_partitions(). the problem is that mtd_table[x]->name still points to the freed memory. Aldo physmap_flash_remove() should call del_mtd_partitions() or del_mtd_device() only once. Reported-by: Matthias Kaehlcke <matthias@kaehlcke.net> Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp> --- diff --git a/drivers/mtd/maps/physmap.c b/drivers/mtd/maps/physmap.c index 8774366..4b122e7 100644 --- a/drivers/mtd/maps/physmap.c +++ b/drivers/mtd/maps/physmap.c @@ -29,6 +29,7 @@ struct physmap_flash_info { struct map_info map[MAX_RESOURCES]; #ifdef CONFIG_MTD_PARTITIONS int nr_parts; + struct mtd_partition *parts; #endif }; @@ -45,25 +46,26 @@ static int physmap_flash_remove(struct platform_device *dev) physmap_data = dev->dev.platform_data; -#ifdef CONFIG_MTD_CONCAT - if (info->cmtd != info->mtd[0]) { +#ifdef CONFIG_MTD_PARTITIONS + if (info->nr_parts) { + del_mtd_partitions(info->cmtd); + kfree(info->parts); + } else if (physmap_data->nr_parts) + del_mtd_partitions(info->cmtd); + else del_mtd_device(info->cmtd); +#else + del_mtd_device(info->cmtd); +#endif + +#ifdef CONFIG_MTD_CONCAT + if (info->cmtd != info->mtd[0]) mtd_concat_destroy(info->cmtd); - } #endif for (i = 0; i < MAX_RESOURCES; i++) { - if (info->mtd[i] != NULL) { -#ifdef CONFIG_MTD_PARTITIONS - if (info->nr_parts || physmap_data->nr_parts) - del_mtd_partitions(info->mtd[i]); - else - del_mtd_device(info->mtd[i]); -#else - del_mtd_device(info->mtd[i]); -#endif + if (info->mtd[i] != NULL) map_destroy(info->mtd[i]); - } } return 0; } @@ -86,9 +88,6 @@ 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) @@ -167,10 +166,11 @@ 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, &parts, 0); + err = parse_mtd_partitions(info->cmtd, part_probe_types, + &info->parts, 0); if (err > 0) { - add_mtd_partitions(info->cmtd, parts, err); - kfree(parts); + add_mtd_partitions(info->cmtd, info->parts, err); + info->nr_parts = err; return 0; } ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [BUG] physmap_flash_probe() frees memory still in use 2009-01-30 15:52 ` Atsushi Nemoto @ 2009-01-30 16:44 ` Matthias Kaehlcke -1 siblings, 0 replies; 6+ messages in thread From: Matthias Kaehlcke @ 2009-01-30 16:44 UTC (permalink / raw) To: Atsushi Nemoto; +Cc: akpm, linux-mtd, David.Woodhouse, vapier.adi, linux-kernel El Sat, Jan 31, 2009 at 12:52:50AM +0900 Atsushi Nemoto ha dit: > On Fri, 30 Jan 2009 14:43:18 +0100, Matthias Kaehlcke <matthias@kaehlcke.net> wrote: > > as i reported earlier in this month > > (http://lkml.org/lkml/2009/1/12/141) MTD partition names are screwed > > up in 2.6.29-rcX. > > > > commit 176bf2e0f10ecf1d20a97db3bd5bb2e6ba0b5668 deals with a memory > > leak and frees the pointer array of mtd_partition after the call to > > add_mtd_partitions(). the problem is that mtd_table[x]->name still > > points to the freed memory. > > Oh Yes, I had missed that point. And looking at physmap driver again, > I found that del_mtd_partition() and del_mtd_device() are abused. > Though these APIs are robust enough, deleting right things in right > order would be better. > > Could you test this patch? I cannot test it until Monday. looks good! Tested-By: Matthias Kaehlcke <matthias@kaehlcke.net> -- Matthias Kaehlcke Embedded Linux Engineer Barcelona If you don't know where you are going, you will probably end up somewhere else (Laurence J. Peter) .''`. using free software / Debian GNU/Linux | http://debian.org : :' : `. `'` gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4 `- ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [BUG] physmap_flash_probe() frees memory still in use @ 2009-01-30 16:44 ` Matthias Kaehlcke 0 siblings, 0 replies; 6+ messages in thread From: Matthias Kaehlcke @ 2009-01-30 16:44 UTC (permalink / raw) To: Atsushi Nemoto; +Cc: akpm, David.Woodhouse, vapier.adi, linux-mtd, linux-kernel El Sat, Jan 31, 2009 at 12:52:50AM +0900 Atsushi Nemoto ha dit: > On Fri, 30 Jan 2009 14:43:18 +0100, Matthias Kaehlcke <matthias@kaehlcke.net> wrote: > > as i reported earlier in this month > > (http://lkml.org/lkml/2009/1/12/141) MTD partition names are screwed > > up in 2.6.29-rcX. > > > > commit 176bf2e0f10ecf1d20a97db3bd5bb2e6ba0b5668 deals with a memory > > leak and frees the pointer array of mtd_partition after the call to > > add_mtd_partitions(). the problem is that mtd_table[x]->name still > > points to the freed memory. > > Oh Yes, I had missed that point. And looking at physmap driver again, > I found that del_mtd_partition() and del_mtd_device() are abused. > Though these APIs are robust enough, deleting right things in right > order would be better. > > Could you test this patch? I cannot test it until Monday. looks good! Tested-By: Matthias Kaehlcke <matthias@kaehlcke.net> -- Matthias Kaehlcke Embedded Linux Engineer Barcelona If you don't know where you are going, you will probably end up somewhere else (Laurence J. Peter) .''`. using free software / Debian GNU/Linux | http://debian.org : :' : `. `'` gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4 `- ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-01-30 16:44 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-01-30 13:43 [BUG] physmap_flash_probe() frees memory still in use Matthias Kaehlcke 2009-01-30 13:43 ` Matthias Kaehlcke 2009-01-30 15:52 ` Atsushi Nemoto 2009-01-30 15:52 ` Atsushi Nemoto 2009-01-30 16:44 ` Matthias Kaehlcke 2009-01-30 16:44 ` Matthias Kaehlcke
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.