Xen devel - Oct 2009 - [PATCH] Update cr3 in PAE mode when guest walk succeed but shadow walk fails

When running in PAE mode, Windows 7 (apparently) will occasionally
switch cr3 with one of the L3 entries invalid, make it valid, and then
expect the hardware to load the new value.  (This behavior is
explicitly not promised in the hardware manuals.)  This leads to a
situation where on a shadow fault, the guest walk succeeds but the
shadow walk fails.  The code assumes this can only happen when the
domain is dying, and makes an ASSERT() to that effect.  So currently,
in debug mode, this will cause the host to crash; in non-debug mode,
this will cause a page-fault loop.

The attached patch solves the problem by calling update_cr3() in that
path when the guest is in PAE mode, and only ASSERT()ing when the
guest is not in PAE mode.  The guest will get one spurious page fault,
but subsequent accesses will succeed.

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Fri, Oct 16, 2009 at 02:09:20PM +0100, George Dunlap wrote:
> domain is dying, and makes an ASSERT() to that effect.  So currently,
> in debug mode, this will cause the host to crash; in non-debug mode,
> this will cause a page-fault loop.
Isn''t this a security hole then?

regards
john

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 17/10/2009 14:51, "John Levon" <levon@movementarian.org>
wrote:
> On Fri, Oct 16, 2009 at 02:09:20PM +0100, George Dunlap wrote:
> 
>> domain is dying, and makes an ASSERT() to that effect.  So currently,
>> in debug mode, this will cause the host to crash; in non-debug mode,
>> this will cause a page-fault loop.
> 
> Isn''t this a security hole then?
Debug builds shouldn''t be used in production environments. The
page-fault
loop, in the non-debug case, is preemptible, so the offending guest can only
waste its own scheduling quantum.

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Sat, Oct 17, 2009 at 03:15:35PM +0100, Keir Fraser wrote:
> > On Fri, Oct 16, 2009 at 02:09:20PM +0100, George Dunlap wrote:
> > 
> >> domain is dying, and makes an ASSERT() to that effect.  So
currently,
> >> in debug mode, this will cause the host to crash; in non-debug
mode,
> >> this will cause a page-fault loop.
> > 
> > Isn''t this a security hole then?
> 
> Debug builds shouldn''t be used in production environments. The
page-fault
> loop, in the non-debug case, is preemptible, so the offending guest can
only
> waste its own scheduling quantum.
OK, makes sense.

regards
john

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

> The attached patch solves the problem by calling update_cr3() in that
> path when the guest is in PAE mode, and only ASSERT()ing when the
> guest is not in PAE mode.  The guest will get one spurious page fault,
> but subsequent accesses will succeed.
Hi George,

I suppose this patch is also needed when running 32-on-64.  In that 
case, shouldn''t the test be

#if GUEST_PAGING_LEVELS >= 3

rather than "== 3"?

Thanks,

Paolo

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hi,

At 10:46 +0100 on 21 Oct (1256121960), Paolo Bonzini
wrote:
> I suppose this patch is also needed when running 32-on-64.  In that 
> case, shouldn''t the test be
> 
> #if GUEST_PAGING_LEVELS >= 3
> 
> rather than "== 3"?
No, ''=='' is correct.  GUEST_PAGING_LEVELS refers to the layout
of the
_guest''s_ pagetables, not the shadows.  GUEST_PAGING_LEVELS == 4 means
the guest is using full 64-bit pagetables, which don''t have this quirk.

Cheers, 

Tim,
 

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Citrix Systems (R&D) Ltd.
[Company #02300071, SL9 0DZ, UK.]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 10/21/2009 11:50 AM, Tim Deegan wrote:
> Hi,
>
> At 10:46 +0100 on 21 Oct (1256121960), Paolo Bonzini wrote:
>> I suppose this patch is also needed when running 32-on-64.  In that
>> case, shouldn''t the test be
>>
>> #if GUEST_PAGING_LEVELS>= 3
>>
>> rather than "== 3"?
>
> No, ''=='' is correct.  GUEST_PAGING_LEVELS refers to the
layout of the
> _guest''s_ pagetables, not the shadows.  GUEST_PAGING_LEVELS == 4
means
> the guest is using full 64-bit pagetables, which don''t have this
quirk.
Right, I should have read arch/x86/mm/shadow/Makefile.

Thanks,

Paolo

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Paolo Bonzini wrote:
> Right, I should have read arch/x86/mm/shadow/Makefile.
>   
Yes, the shadow code is as sane as it can possibly be... but still 
barking mad by any standard. :-)

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel