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

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

[niam]

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 <niam.niam@gmail.com>
---
 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);

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

[Joakim Tjernlund]

niam <niam.niam@gmail.com> 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 <niam.niam@gmail.com>
> ---
>  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);
>

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

[niam]

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
<joakim.tjernlund@transmode.se> wrote:
> niam <niam.niam@gmail.com> 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 <niam.niam@gmail.com>
>> ---
>>  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);
>>
>
>