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

["Aravindh Puthiyaparambil (aravindp)" <aravindp@xxxxxxxxx>]

>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.

Yes, I agree. In fact using the p2m-pt implementation is not going to help 
getting around the hypercall continuation issue.

>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).

Do you mean the p2m_access_t enum when you say encoding? 0 now mean 
p2m_access_n. Are you saying that 0 should now mean p2m_access_rwx? 

Thanks,
Aravindh


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