From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tamas K Lengyel Subject: Re: [PATCH] Libxc: fix xc_translate_foreign_address() Date: Tue, 4 Apr 2017 09:39:38 -0600 Message-ID: References: <1491308099-4751-1-git-send-email-rcojocaru@bitdefender.com> <45ad0664-1405-7355-08dd-c16c2b5fa127@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============7126772958327515401==" Return-path: In-Reply-To: <45ad0664-1405-7355-08dd-c16c2b5fa127@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Andrew Cooper Cc: "wei.liu2@citrix.com" , Cristian-Bogdan Sirb , Ian Jackson , Razvan Cojocaru , Xen-devel List-Id: xen-devel@lists.xenproject.org --===============7126772958327515401== Content-Type: multipart/alternative; boundary=001a1148ea8cd57daf054c59170b --001a1148ea8cd57daf054c59170b Content-Type: text/plain; charset=UTF-8 On Tue, Apr 4, 2017 at 9:11 AM, Andrew Cooper 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 > > 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). Tamas --001a1148ea8cd57daf054c59170b Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable


On Tue, Apr 4, 2017 at 9:11 AM, Andrew Cooper <andrew.cooper3@= citrix.com> 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 pat= ch
> 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@bitdefender.com>

This function is in a rather tragic state.=C2=A0 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.=C2=A0 (I might be acutely aware of pagetable issues, following c/s
4c5d78a10dc89).=C2=A0 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 thro= w them away only after they have been idle for a while.
=C2= =A0

How often is this used by introspection?=C2=A0 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<= br> in Xen.

Control register state is certa= inly available.
=C2=A0

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.=C2=A0 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 thi= nk 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 d= o the translation properly (ie. present bit unset but transition bits used = for Windows guests).

Tamas

--001a1148ea8cd57daf054c59170b-- --===============7126772958327515401== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5v cmcveGVuLWRldmVsCg== --===============7126772958327515401==--