[PATCH] mtd: fix memory leak

Mathias Krause minipli at googlemail.com
Sun Jan 30 12:35:48 EST 2011


On 30.01.2011 at 15:52, Artem Bityutskiy wrote:
> Hi,
> 
> On Sun, 2011-01-30 at 10:31 +0100, Mathias Krause wrote:
>> diff --git a/drivers/mtd/devices/phram.c b/drivers/mtd/devices/phram.c
>> index 5239328..8d28fa0 100644
>> --- a/drivers/mtd/devices/phram.c
>> +++ b/drivers/mtd/devices/phram.c
>> @@ -117,6 +117,7 @@ static void unregister_devices(void)
>> 	list_for_each_entry_safe(this, safe, &phram_list, list) {
>> 		del_mtd_device(&this->mtd);
>> 		iounmap(this->mtd.priv);
>> +		kfree(this->mtd.name);
>> 		kfree(this);
>> 	}
> 
> Since register_device() did not allocate it, unregister_devices() should
> not free it.

I agree with you, the internal API is a little quirky regarding that point. register_device() should strdup() the name and not just blindly use it. But since the memory for name was already allocated via kmalloc() in phram_setup() it seems a little nitpicky to copy it once again in register_device().

> Hence, I think it is better to free(name) just after
> calling unregister_devices().

This is not possible because unregister_devices() unregisters all devices, not just a single instance. Though name must be freed for every object in the list. After unregister_devices() returns the list is empty and no pointer to the memory locations do exist any more. So my patch was the straight forward fix for the memory leak.


Regards,
Mathias


More information about the linux-mtd mailing list