Re: [Xen-devel] [PATCH RFC v2 1/4] x86/mm: Shadow and p2m changes for PV mem_access

[Tim Deegan <tim@xxxxxxx>]

At 21:39 +0000 on 25 Jul (1406320788), Aravindh Puthiyaparambil (aravindp) 
wrote:
> >>>I wonder how well the window where you're running with CR0.WP clear
> >>>is bounded: The flag serves as a kind of security measure, and hence
> >>>shouldn't be left off for extended periods of time.
> >>
> >> I agree. Is there a way I can bound this?
> >
> >That's what you need to figure out. The simplistic solution (single
> >stepping just the critical instruction(s)) is probably not going to be
> >acceptable due to its fragility. I have no good other suggestions,
> >but I'm not eager to allow code in that weakens protection.
> 
> From the debugging I have done to get this working, this is what the flow 
> should be. Xen tries to write to guest page marked read only and page fault 
> occurs. So __copy_to_user_ll() -> handle_exception_saved->do_page_fault() and 
> CR0.WP is cleared. Once the fault is handled __copy_to_user_ll() is retried 
> and it goes through. At the end of which CR0.WP is turned on. So this is the 
> only window that pv_vcpu.need_cr0_wp_set should be true. Is there a spot 
> outside of this window that I check to see if it is set and if it is, turn it 
> back on again? Would that be a sufficient bound?
> 
> >>>>          pg[i].count_info = PGC_allocated | 1;
> >>>> +        if ( is_pv_domain(d) )
> >>>> +            shadow_set_access(&pg[i], p2m_get_hostp2m(d)-
> >>default_access);
> >>>
> >>>I don't think you should call shadow code from here.
> >>
> >> Should I add a p2m wrapper for this, so that it is valid only for x86 and a
> >> no-op for ARM?
> >
> >That's not necessarily enough, but at least presumably the right
> >route: You also need to avoid fiddling with struct page_info fields
> >that may be used (now or in the future) for other purposes, i.e.
> >you need to gate the setting of the flags by more than just
> >is_pv_domain().
> 
> Coupled with your response to the other thread, I am thinking I should move 
> away from using the shadow_flags for access permissions. Tim's other 
> suggestion was to try and re-use the p2m-pt implementation.
> 

I think you should be OK to use shadow_flags here -- after all the
shadow code relies on being able to use them for any guest data page
once shadow mode is enabled. 

To avoid touching them before shadow mode is actually enabled you
could reshuffle the encodings so that 0 is 'default' (shadow code
absolutely relies on this field being 0 when shadow more is enabled so
any other user will have to maintain that).

Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel