[Lguest] [PATCH] fix lguest rmmod "bad pgd"

[Balaji Rao R]

On Jan 4, 2008 6:22 AM, Rusty Russell <rusty at rustcorp.com.au> wrote:
>
> On Friday 04 January 2008 01:17:34 Balaji Rao wrote:
> > On Thursday 03 January 2008 14:05:28 Ingo Molnar wrote:
> > > * Rusty Russell <rusty at rustcorp.com.au> wrote:
> > > > On Thursday 03 January 2008 00:27:10 pageexec at freemail.hu wrote:
> > > > > (sorry for breaking the thread but i didn't get the original headers)
> > > > >
> > > > > > After 17d57a9206b4de6ad082ac9f2d2346985abbd2aa
> > > > > > (x86: fix x86-32 early fixmap initialization.) removing lg.ko
> > > > > > caused a printk from vunmap:
> > > > > >
> > > > > >  mm/memory.c:115: bad pgd 004b3027.
> > > > > >
> > > > > > On the second use after module load, the kernel crashes.
> > > > > >
> > > > > > This fixes the immediate problem (accessed and dirty bits not set
> > > > > > as expected in pmd_none_or_clear_bad).  I can't see why this would
> > > > > > cause a crash, but I haven't been able to reproduce it once this is
> > > > > > applied.
> > > > >
> > > > > it's the 'clear_bad' part that zero's out the pmd and hence destroys
> > > > > all the fixmap mappings in that 4 MB range leading to page faults at
> > > > > probably the most unexpected times.
> > > >
> > > > Thanks, I had misread the code as clearing it normally anyway.  This
> > > > is a nasty bug because we'd hit it if we ever filled vmalloc space,
> > > > but now Linus has applied that patch we should all be merry...
> > >
> > > wanna send a patch for the p?d_bad() bug as well? (it should ignore any
> > > differences in the dirty/accessed bits)
> >
> > Hi Ingo,
> > I just tried this and I stop getting the 'bad pgd' error. Is it the right
> > way to do it ?
> >
> > Index: linux/include/asm/pgtable_32.h
> > ===================================================================
> > --- linux.orig/include/asm/pgtable_32.h       2008-01-03 19:16:29.000000000 +0530
> > +++ linux/include/asm/pgtable_32.h    2008-01-03 19:16:49.000000000 +0530
> > @@ -206,7 +206,8 @@
> >  /* To avoid harmful races, pmd_none(x) should check only the lower when
> > PAE */ #define pmd_none(x)    (!(unsigned long)pmd_val(x))
> >  #define pmd_present(x)       (pmd_val(x) & _PAGE_PRESENT)
> > -#define      pmd_bad(x)      ((pmd_val(x) & (~PAGE_MASK & ~_PAGE_USER)) !=
> > _KERNPG_TABLE) +#define       pmd_bad(x) \
> > +     (!(pmd_val(x) & (~PAGE_MASK & ~_PAGE_USER & ~_KERNPG_TABLE)) &
> > (_PAGE_ACCESSED | _PAGE_DIRTY))
>
> I don't agree with this idea: catching these errors is a nice warning, but I
> wonder if we should simply try to fix it up in this case rather than zeroing
> it.
>
May be we can put the original line in a BUG_ON() ? I am not sure, am
just thinking..

-- 
warm regards

Balaji Rao
NITK Surathkal