Patchwork ubifs: error unwinding trouble

login
register
mail settings
Submitter Adrian Hunter
Date July 24, 2009, 10:49 a.m.
Message ID <4A6991A5.4020105@nokia.com>
Download mbox | patch
Permalink /patch/30198/
State New, archived
Headers show

Comments

Adrian Hunter - July 24, 2009, 10:49 a.m.
Daniel Mack wrote:
> On a recent git kernel, the error unwinding for UBIFS seems to have some
> problem, most probably a double-free or something similar.
> 
> When UBI is pointed to the right mtd partition (using command line
> arguments) , everything is fine. But when it's (accidentionally) set to
> some very small mtd, the attach process fails. Which wouldn't be a bad
> thing by itself, but it somehow messes up the slub/slab allocators then
> which causes very strange memory corruption effects - see the backtrace
> below.
> 
> The Ooops itself is unreleated to UBI, but it does not occur when UBI
> succeeds in attaching the volume.
> 
> Any idea? I searched for awhile but couldn't see anything obvious.

Looks like a double free of the eba_tbl

This might help:
Artem Bityutskiy - July 24, 2009, 12:17 p.m.
On Fri, 2009-07-24 at 13:49 +0300, Adrian Hunter wrote:
> Daniel Mack wrote:
> > On a recent git kernel, the error unwinding for UBIFS seems to have some
> > problem, most probably a double-free or something similar.
> > 
> > When UBI is pointed to the right mtd partition (using command line
> > arguments) , everything is fine. But when it's (accidentionally) set to
> > some very small mtd, the attach process fails. Which wouldn't be a bad
> > thing by itself, but it somehow messes up the slub/slab allocators then
> > which causes very strange memory corruption effects - see the backtrace
> > below.
> > 
> > The Ooops itself is unreleated to UBI, but it does not occur when UBI
> > succeeds in attaching the volume.
> > 
> > Any idea? I searched for awhile but couldn't see anything obvious.
> 
> Looks like a double free of the eba_tbl
> 
> This might help:
> 
> diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c
> index 0f2034c..e4d9ef0 100644
> --- a/drivers/mtd/ubi/eba.c
> +++ b/drivers/mtd/ubi/eba.c
> @@ -1254,6 +1254,7 @@ out_free:
>                 if (!ubi->volumes[i])
>                         continue;
>                 kfree(ubi->volumes[i]->eba_tbl);
> +               ubi->volumes[i]->eba_tbl = NULL;
>         }
>         return err;
>  }

You are right. I've just pushed your patch to ubi-2.6.git/master.
Daniel Mack - July 24, 2009, 3:46 p.m.
On Fri, Jul 24, 2009 at 03:17:46PM +0300, Artem Bityutskiy wrote:
> On Fri, 2009-07-24 at 13:49 +0300, Adrian Hunter wrote:
> > diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c
> > index 0f2034c..e4d9ef0 100644
> > --- a/drivers/mtd/ubi/eba.c
> > +++ b/drivers/mtd/ubi/eba.c
> > @@ -1254,6 +1254,7 @@ out_free:
> >                 if (!ubi->volumes[i])
> >                         continue;
> >                 kfree(ubi->volumes[i]->eba_tbl);
> > +               ubi->volumes[i]->eba_tbl = NULL;
> >         }
> >         return err;
> >  }
> 
> You are right. I've just pushed your patch to ubi-2.6.git/master.

Great. Thanks for the quick response!
Is there any merge cycle outstanding for ubifs in 2.6.31?

Daniel
Artem Bityutskiy - July 24, 2009, 3:47 p.m.
On 07/24/2009 06:46 PM, Daniel Mack wrote:
> On Fri, Jul 24, 2009 at 03:17:46PM +0300, Artem Bityutskiy wrote:
>> On Fri, 2009-07-24 at 13:49 +0300, Adrian Hunter wrote:
>>> diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c
>>> index 0f2034c..e4d9ef0 100644
>>> --- a/drivers/mtd/ubi/eba.c
>>> +++ b/drivers/mtd/ubi/eba.c
>>> @@ -1254,6 +1254,7 @@ out_free:
>>>                  if (!ubi->volumes[i])
>>>                          continue;
>>>                  kfree(ubi->volumes[i]->eba_tbl);
>>> +               ubi->volumes[i]->eba_tbl = NULL;
>>>          }
>>>          return err;
>>>   }
>> You are right. I've just pushed your patch to ubi-2.6.git/master.
>
> Great. Thanks for the quick response!
> Is there any merge cycle outstanding for ubifs in 2.6.31?

It is UBI, not UBIFS. I've created ubi-2.6.git/for-linus branch
with the stuff to merge for 2.6.31.

But I anyway always encouredge people to use the linux-next stuff
which has the latest UBI/UBIFS changes.

See http://www.linux-mtd.infradead.org/doc/ubifs.html#L_source

Patch

diff --git a/drivers/mtd/ubi/eba.c b/drivers/mtd/ubi/eba.c
index 0f2034c..e4d9ef0 100644
--- a/drivers/mtd/ubi/eba.c
+++ b/drivers/mtd/ubi/eba.c
@@ -1254,6 +1254,7 @@  out_free:
                if (!ubi->volumes[i])
                        continue;
                kfree(ubi->volumes[i]->eba_tbl);
+               ubi->volumes[i]->eba_tbl = NULL;
        }
        return err;
 }