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

From: niam.niam@gmail.com (niam)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] jffs2: fix memory leak if the sector was successfully erased
Date: Mon, 22 Feb 2010 19:40:43 +0200	[thread overview]
Message-ID: <a9e22dff1002220940v529faaa3j92ee0fdf7edf8d38@mail.gmail.com> (raw)
In-Reply-To: <4B82BEC5.2040306@gandalf.sssup.it>

Yes, indeed, you are right!

Thank you Michael!

Thank you to everybody and I'm sorry for doing that much noise here.

Next time I will try to send more correct patches.

However freeing memory in the callback is not obvious. Isn't better to
do this out of it?

-- Dima

On Mon, Feb 22, 2010 at 7:28 PM, Michael Trimarchi
<trimarchi@gandalf.sssup.it> wrote:
> Hi,
>
> Daniel Mack wrote:
>>
>> On Mon, Feb 22, 2010 at 06:26:22PM +0200, Ni at m wrote:
>>
>>>
>>> Memory allocated for erase instruction is not freed if the sector was
>>> successfully erased.
>>>
>>> Signed-off-by: Dmytro Milinevskyy <niam.niam@gmail.com>
>>>
>>
>> Can you forward that to the right people please? Call
>> scripts/get_maintainer.pl to get a list of maintainers.
>>
>> Daniel
>>
>>
>>
>>
>>>
>>> ---
>>> ?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);
>>>
>>> _______________________________________________
>>> linux-arm-kernel mailing list
>>> linux-arm-kernel at lists.infradead.org
>>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>>
>>
>>
>
> static void jffs2_erase_callback(struct erase_info *instr)
> {
> ? ? ? struct erase_priv_struct *priv = (void *)instr->priv;
>
> ? ? ? if(instr->state != MTD_ERASE_DONE) {
> printk(KERN_WARNING "Erase at 0x%08x finished, but state != MTD_ERASE_DONE.
> State is 0x%x instead.\n", instr->addr, instr->state);
> ? ? ? ? ? ? ? jffs2_erase_failed(priv->c, priv->jeb, instr->fail_addr);
> ? ? ? } else {
> ? ? ? ? ? ? ? jffs2_erase_succeeded(priv->c, priv->jeb);
> ? ? ? }
> ? ? ? kfree(instr);
> }
>
> instr->callback = jffs2_erase_callback;
>
> Maybe the free is here ?:)
>
> Michael
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>>
>
>