[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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |