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

[Rusty Russell]

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.

Cheers,
Rusty.