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

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





On Tue, Apr 4, 2017 at 9:58 AM, Andrew Cooper <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> 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>

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

Tamas
_______________________________________________
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®.