2010-02-22 16:40:35

by niam

Subject: [PATCH] jffs2: fix memory leak if the sector was successfully erased

Resending patch to proper people/mailing list.

Memory allocated for erase instruction is not freed if the sector was
successfully erased.

Signed-off-by: Dmytro Milinevskyy <[email protected]>
---
?fs/jffs2/erase.c | ? ?4 +++-
?1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c
index b47679b..c0a5604 100644
--- a/fs/jffs2/erase.c
+++ b/fs/jffs2/erase.c
@@ -74,8 +74,10 @@ static void jffs2_erase_block(struct jffs2_sb_info *c,
? ? ? ?((struct erase_priv_struct *)instr->priv)->c = c;

? ? ? ?ret = c->mtd->erase(c->mtd, instr);
- ? ? ? if (!ret)
+ ? ? ? if (!ret) {
+ ? ? ? ?kfree(instr);
? ? ? ? ? ? ? ?return;
+ ? ?}

? ? ? ?bad_offset = instr->fail_addr;
? ? ? ?kfree(instr);

2010-02-22 17:57:54

by niam

Subject: Re: [PATCH] jffs2: fix memory leak if the sector was successfully erased

Yes, you are right.
Michael Trimarchi also responded that it's freed by the callback.

My concern now that it's not obvious to free erase instruction in
jffs2_erase_callback.
Why not to free it the same piece of code where it was allocated?

--Dima

On Mon, Feb 22, 2010 at 7:49 PM, Joakim Tjernlund
<[email protected]> wrote:
> niam <[email protected]> wrote on 2010/02/22 17:40:11:
>>
>> Resending patch to proper people/mailing list.
>>
>> Memory allocated for erase instruction is not freed if the sector was
>> successfully erased.
>
> NAK, jffs2_erase_callback() will free it so
> you have added a double free I think. Did you measure a memory leak?
>
>>
>> Signed-off-by: Dmytro Milinevskyy <[email protected]>
>> ---
>> ?fs/jffs2/erase.c | ? ?4 +++-
>> ?1 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c
>> index b47679b..c0a5604 100644
>> --- a/fs/jffs2/erase.c
>> +++ b/fs/jffs2/erase.c
>> @@ -74,8 +74,10 @@ static void jffs2_erase_block(struct jffs2_sb_info *c,
>> ? ? ? ?((struct erase_priv_struct *)instr->priv)->c = c;
>>
>> ? ? ? ?ret = c->mtd->erase(c->mtd, instr);
>> - ? ? ? if (!ret)
>> + ? ? ? if (!ret) {
>> + ? ? ? ?kfree(instr);
>> ? ? ? ? ? ? ? ?return;
>> + ? ?}
>>
>> ? ? ? ?bad_offset = instr->fail_addr;
>> ? ? ? ?kfree(instr);
>>
>
>

2010-02-22 18:02:02

by Joakim Tjernlund

Subject: Re: [PATCH] jffs2: fix memory leak if the sector was successfully erased

niam <[email protected]> wrote on 2010/02/22 17:40:11:
>
> Resending patch to proper people/mailing list.
>
> Memory allocated for erase instruction is not freed if the sector was
> successfully erased.

NAK, jffs2_erase_callback() will free it so
you have added a double free I think. Did you measure a memory leak?

>
> Signed-off-by: Dmytro Milinevskyy <[email protected]>
> ---
> ?fs/jffs2/erase.c | ? ?4 +++-
> ?1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/fs/jffs2/erase.c b/fs/jffs2/erase.c
> index b47679b..c0a5604 100644
> --- a/fs/jffs2/erase.c
> +++ b/fs/jffs2/erase.c
> @@ -74,8 +74,10 @@ static void jffs2_erase_block(struct jffs2_sb_info *c,
> ? ? ? ?((struct erase_priv_struct *)instr->priv)->c = c;
>
> ? ? ? ?ret = c->mtd->erase(c->mtd, instr);
> - ? ? ? if (!ret)
> + ? ? ? if (!ret) {
> + ? ? ? ?kfree(instr);
> ? ? ? ? ? ? ? ?return;
> + ? ?}
>
> ? ? ? ?bad_offset = instr->fail_addr;
> ? ? ? ?kfree(instr);
>