[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] Libxc: fix xc_translate_foreign_address()



On 04/04/2017 07:08 PM, Tamas K Lengyel wrote:
> 
> 
> On Tue, Apr 4, 2017 at 9:58 AM, Andrew Cooper <andrew.cooper3@xxxxxxxxxx
> <mailto:andrew.cooper3@xxxxxxxxxx>> wrote:
> 
>     On 04/04/17 16:39, Tamas K Lengyel wrote:
>>
>>
>>     On Tue, Apr 4, 2017 at 9:11 AM, Andrew Cooper
>>     <andrew.cooper3@xxxxxxxxxx <mailto:andrew.cooper3@xxxxxxxxxx>> wrote:
>>
>>         On 04/04/17 13:14, Razvan Cojocaru wrote:
>>         > Currently xc_translate_foreign_address only checks for PSE
>>         bit only on
>>         > level 2 entries (that's 2 MB pages on x64 and 32-bit with
>>         PAE, and 4 MB
>>         > pages on 32-bit). But linux kernel sometimes uses 1 GB
>>         pages. This patch
>>         > fixes that, and checks the PSE bit on level 3 entries if the
>>         guest has 4
>>         > translation levels (that means 64-bit guests only).
>>         >
>>         > Signed-off-by: Cristian-Bogdan Sirb <csirb@xxxxxxxxxxxxxxx
>>         <mailto:csirb@xxxxxxxxxxxxxxx>>
>>
>>         This function is in a rather tragic state.  Lucky it isn't
>>         used by code
>>         covered by Xen's security statement.
>>
>>         This patch reintroduces XSA-176, and the existing code doesn't
>>         work for
>>         4M superpages, or guests using PSE36.  (I might be acutely
>>         aware of
>>         pagetable issues, following c/s
>>         4c5d78a10dc89).  Furthermore, the map/unmap overhead must be a
>>         large
>>         overhead.
>>
>>
>>     Indeed it is, that's why in LibVMI there is actually a cache
>>     implemented for mapped pages so we throw them away only after they
>>     have been idle for a while.
>>      
>>
>>
>>         How often is this used by introspection?  To properly walk the
>>         pagetables, you need access to the CPUID information (as well
>>         as the
>>         control register state), and that isn't yet available to the
>>         toolstack
>>         in Xen.
>>
>>
>>     Control register state is certainly available.
>>      
>>
>>
>>         I'm wondering whether it might be better to have a way of
>>         asking Xen to
>>         perform a virtual to linear translation in the context of a
>>         specific
>>         vcpu.  My gut feeling is that it might be quicker than running
>>         this
>>         logic, and is definitely more simple than trying to fix this
>>         code not to
>>         have vulnerabilities in it.
>>
>>
>>     I don't think it would be necessary. IMHO doing this in Xen
>>     wouldn't really net us much performance-wise. Furthermore, there
>>     are certain PTE bits that are OS specific and Xen wouldn't
>>     necessary have the required information to do the translation
>>     properly (ie. present bit unset but transition bits used for
>>     Windows guests).
> 
>     Ok.  Xen needs to care only about the behaviour of real pointers in
>     guest context, and whether they raise #PF.
> 
>     It sounds like the best thing to do is to provide userspace with all
>     information it needs to actually perform the walk safely, and let
>     the library apply guest-specific knowledge if it wants.
> 
>     Off the top of my head, the control information needed is:
> 
>     Hap/Shadow, hardwares views towards page1gb and pse36, whether
>     EFER.NX is leaked from Xen, EFER.NX, CR0.{PG,WP},
>     CR4.{PAE,PSE,PCID,SMEP,SMAP,PKE}, and the PDPTRs for the 32bit PAE
>     case, because the translation in use isn't necessary the value you
>     would read by following CR3 in memory.
> 
> 
> The userspace already has access to these informations and we use them
> in LibVMI already (see
> https://github.com/libvmi/libvmi/blob/master/libvmi/memory.c#L37). It's
> only this libxc function that has not really been touched in a long time
> because I don't think anyone uses it..

Should it then be removed altogether, or at least be marked with a
#warning or a comment?


Thanks,
Razvan

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.