[PATCH] UBIFS: add missing znode freeing in tcn_insert()

[Florian Fainelli]

2014-03-08 2:46 GMT-08:00 Richard Weinberger <richard.weinberger at gmail.com>:
> On Sat, Mar 8, 2014 at 1:11 AM, Florian Fainelli <f.fainelli at gmail.com> wrote:
>> In case the zi allocation fails in the do_split label, we will fail
>> freeing zn that we allocated before, add a missing kfree.
>>
>> Signed-off-by: Florian Fainelli <f.fainelli at gmail.com>
>> ---
>>  fs/ubifs/tnc.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/ubifs/tnc.c b/fs/ubifs/tnc.c
>> index 9083bc7ed4ae..9b84d91ea530 100644
>> --- a/fs/ubifs/tnc.c
>> +++ b/fs/ubifs/tnc.c
>> @@ -2105,8 +2105,10 @@ do_split:
>>         dbg_tnc("creating new zroot at level %d", znode->level + 1);
>>
>>         zi = kzalloc(c->max_znode_sz, GFP_NOFS);
>> -       if (!zi)
>> +       if (!zi) {
>> +               kfree(zn);
>>                 return -ENOMEM;
>> +       }
>>
>
> I'm not sure whether this is correct.
>
> Around line 2050 we have:
> ... else {
>                 /* Insert into new znode */
>                 zi = zn; <---------
>                 n -= keep;
>                 /* Re-parent */
>                 if (zn->level != 0)
>                         zbr->znode->parent = zn;
>         }
>
> And later:
> insert_zbranch(zi, zbr, n);
>
> By freeing zn you may introduce a use after free bug.

Yes, you are right, I cannot find a good way to make sure this is not
the case, variables re-use in this function makes it particularly hard
to read BTW.
-- 
Florian