[PATCH 0/3] x86: Fixes to shadowing dom0

[PATCH 0/3] x86: Fixes to shadowing dom0

[Andrew Cooper]

It turns out there are some real bugs attempting to shadow dom0, and it is
because of these that XSA-273 went out with `pv-l1tf=` defaulting to not
shadowing dom0.  They aren't security issues themselves.

Patch 1 is a general problem with dom0 and 1G superpages, and wants to be
taken onto all releases.

Patches 2 and 3 are various backports of work already upstream, to fix the
shadowing of PV guests with 2M superpages.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 1/3] x86/dom0: Avoid using 1G superpages if shadowing may be necessary

[Andrew Cooper]

The shadow code doesn't support 1G superpages, and will hand #PF[RSVD] back to
guests.

For dom0's with 512GB of RAM or more (and subject to the P2M alignment), Xen's
domain builder might use 1G superpages.

Avoid using 1G superpages (falling back to 2M superpages instead) if there is
a reasonable chance that we may have to shadow dom0.  This assumes that there
are no circumstances where we will activate logdirty mode on dom0.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/pv/dom0_build.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
index 812b026..710c8bb 100644
--- a/xen/arch/x86/pv/dom0_build.c
+++ b/xen/arch/x86/pv/dom0_build.c
@@ -148,7 +148,14 @@ static __init void setup_pv_physmap(struct domain *d, unsigned long pgtbl_pfn,
         pl3e += l3_table_offset(vphysmap_start);
         if ( !l3e_get_intpte(*pl3e) )
         {
-            if ( cpu_has_page1gb &&
+            /*
+             * 1G superpages aren't supported by the shadow code.  Avoid using
+             * them we are liable to need to start shadowing dom0.  This
+             * assumes that there are no circumstances where we will activate
+             * logdirty mode on dom0.
+             */
+            if ( (!IS_ENABLED(CONFIG_SHADOW_PAGING) ||
+                  !d->arch.pv.check_l1tf) && cpu_has_page1gb &&
                  !(vphysmap_start & ((1UL << L3_PAGETABLE_SHIFT) - 1)) &&
                  vphysmap_end >= vphysmap_start + (1UL << L3_PAGETABLE_SHIFT) &&
                  (page = alloc_domheap_pages(d,
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/3] x86/dom0: Avoid using 1G superpages if shadowing may be necessary

[Jan Beulich]

>>> On 09.11.18 at 16:26, <andrew.cooper3@citrix.com> wrote:
> The shadow code doesn't support 1G superpages, and will hand #PF[RSVD] back to
> guests.

So this change is then instead of trying to make shadow code cope?

> --- a/xen/arch/x86/pv/dom0_build.c
> +++ b/xen/arch/x86/pv/dom0_build.c
> @@ -148,7 +148,14 @@ static __init void setup_pv_physmap(struct domain *d, unsigned long pgtbl_pfn,
>          pl3e += l3_table_offset(vphysmap_start);
>          if ( !l3e_get_intpte(*pl3e) )
>          {
> -            if ( cpu_has_page1gb &&
> +            /*
> +             * 1G superpages aren't supported by the shadow code.  Avoid using
> +             * them we are liable to need to start shadowing dom0.  This

I think there's a word missing here - "when" perhaps?

> +             * assumes that there are no circumstances where we will activate
> +             * logdirty mode on dom0.
> +             */
> +            if ( (!IS_ENABLED(CONFIG_SHADOW_PAGING) ||
> +                  !d->arch.pv.check_l1tf) && cpu_has_page1gb &&
>                   !(vphysmap_start & ((1UL << L3_PAGETABLE_SHIFT) - 1)) &&
>                   vphysmap_end >= vphysmap_start + (1UL << L3_PAGETABLE_SHIFT) &&
>                   (page = alloc_domheap_pages(d,

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/3] x86/dom0: Avoid using 1G superpages if shadowing may be necessary

[Andrew Cooper]

On 12/11/2018 10:05, Jan Beulich wrote:
>>>> On 09.11.18 at 16:26, <andrew.cooper3@citrix.com> wrote:
>> The shadow code doesn't support 1G superpages, and will hand #PF[RSVD] back to
>> guests.
> So this change is then instead of trying to make shadow code cope?

At the moment, that is an unknown quantity of work.  Ideally yes we
should get 1G superpage support working in the shadow code, but it
definitely isn't top of the priority list atm.

>
>> --- a/xen/arch/x86/pv/dom0_build.c
>> +++ b/xen/arch/x86/pv/dom0_build.c
>> @@ -148,7 +148,14 @@ static __init void setup_pv_physmap(struct domain *d, unsigned long pgtbl_pfn,
>>          pl3e += l3_table_offset(vphysmap_start);
>>          if ( !l3e_get_intpte(*pl3e) )
>>          {
>> -            if ( cpu_has_page1gb &&
>> +            /*
>> +             * 1G superpages aren't supported by the shadow code.  Avoid using
>> +             * them we are liable to need to start shadowing dom0.  This
> I think there's a word missing here - "when" perhaps?

Oops yes.

>
>> +             * assumes that there are no circumstances where we will activate
>> +             * logdirty mode on dom0.
>> +             */
>> +            if ( (!IS_ENABLED(CONFIG_SHADOW_PAGING) ||
>> +                  !d->arch.pv.check_l1tf) && cpu_has_page1gb &&
>>                   !(vphysmap_start & ((1UL << L3_PAGETABLE_SHIFT) - 1)) &&
>>                   vphysmap_end >= vphysmap_start + (1UL << L3_PAGETABLE_SHIFT) &&
>>                   (page = alloc_domheap_pages(d,
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Thanks,

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 2/3 Xen-4.9] x86/dom0: Fix shadowing of PV guests with 2M superpages

[Andrew Cooper]

This is a straight backport of c/s 28d9a9a2d41759b9e5163037b759ac557aea767c
but with a different justification.

Dom0 may have superpages (e.g. initial P2M), and may be shadowed
(e.g. PV-L1TF).  Because of this incorrect check, when PV superpages are
disallowed (which is the security supported configuration), attempting to
shadow the P2M with its superpages still intact will fail.  A #PF will be
handed back to the kernel, rather than the superpage being splintered and
shadowed.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
 xen/include/asm-x86/guest_pt.h | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/xen/include/asm-x86/guest_pt.h b/xen/include/asm-x86/guest_pt.h
index 72126d5..08031c8 100644
--- a/xen/include/asm-x86/guest_pt.h
+++ b/xen/include/asm-x86/guest_pt.h
@@ -205,15 +205,17 @@ static inline guest_l4e_t guest_l4e_from_gfn(gfn_t gfn, u32 flags)
 static inline bool guest_can_use_l2_superpages(const struct vcpu *v)
 {
     /*
+     * PV guests use Xen's paging settings.  Being 4-level, 2M
+     * superpages are unconditionally supported.
+     *
      * The L2 _PAGE_PSE bit must be honoured in HVM guests, whenever
      * CR4.PSE is set or the guest is in PAE or long mode.
      * It's also used in the dummy PT for vcpus with CR0.PG cleared.
      */
-    return (is_pv_vcpu(v)
-            ? opt_allow_superpage
-            : (GUEST_PAGING_LEVELS != 2
-               || !hvm_paging_enabled(v)
-               || (v->arch.hvm_vcpu.guest_cr[4] & X86_CR4_PSE)));
+    return (is_pv_vcpu(v) ||
+            GUEST_PAGING_LEVELS != 2 ||
+            !hvm_paging_enabled(v) ||
+            (v->arch.hvm_vcpu.guest_cr[4] & X86_CR4_PSE));
 }
 
 static inline bool guest_can_use_l3_superpages(const struct domain *d)
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 2/3 Xen-4.9] x86/dom0: Fix shadowing of PV guests with 2M superpages

[Jan Beulich]

>>> On 09.11.18 at 16:26, <andrew.cooper3@citrix.com> wrote:
> This is a straight backport of c/s 28d9a9a2d41759b9e5163037b759ac557aea767c
> but with a different justification.
> 
> Dom0 may have superpages (e.g. initial P2M), and may be shadowed
> (e.g. PV-L1TF).  Because of this incorrect check, when PV superpages are
> disallowed (which is the security supported configuration), attempting to
> shadow the P2M with its superpages still intact will fail.  A #PF will be
> handed back to the kernel, rather than the superpage being splintered and
> shadowed.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

It being a straight backport, tags from the original commit should
perhaps be retained?

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 3/3 Xen-4.8] x86/dom0: Fix shadowing of PV guests with 2M superpages

[Andrew Cooper]

This is a minimal backport of pieces of:

 c/s 28d9a9a2d41759b9e5163037b759ac557aea767c
 c/s 4c5d78a10dc89427140a50a1df5a0b8e9f073e82

to fix a PV shadowing problem which I hadn't anticipated at the time these
fixes were first accepted.

Having opt_allow_superpage disabled causes guest_supports_superpages() to
return false for PV guests.  Returning false causes guest_walk_tables() to
ignore L2 superpages, and read under them.

This ignoring behaviour is correct for 2-level paging when CR4.PSE is clear,
but isn't correct for 3- or 4-level paging.

When opt_allow_superpage is clear, PV domU's can't have superpages, but dom0
will still have its initial P2M constructed with 2M superpages.

The end result is that, if dom0 becomes shadowed (e.g. PV-L1TF), the next
memory access touching a P2M superpage will cause the shadow code to read
under the P2M superpage and attempt to shadow junk.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
 xen/arch/x86/mm/guest_walk.c   | 15 ++++++++++++++-
 xen/include/asm-x86/guest_pt.h | 20 ++++++++++++--------
 2 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/xen/arch/x86/mm/guest_walk.c b/xen/arch/x86/mm/guest_walk.c
index 868e909..f4c9921 100644
--- a/xen/arch/x86/mm/guest_walk.c
+++ b/xen/arch/x86/mm/guest_walk.c
@@ -324,9 +324,20 @@ guest_walk_tables(struct vcpu *v, struct p2m_domain *p2m,
         rc |= _PAGE_PRESENT;
         goto out;
     }
+
+    /*
+     * In 2-level paging without CR0.PSE, there are no reserved bits, and the
+     * PAT/PSE bit is ignored.
+     */
+    if ( GUEST_PAGING_LEVELS == 2 && !guest_supports_superpages(v) )
+    {
+        gw->l2e.l2 &= ~_PAGE_PSE;
+        gflags &= ~_PAGE_PSE;
+    }
+
     rc |= ((gflags & mflags) ^ mflags);
 
-    pse2M = (gflags & _PAGE_PSE) && guest_supports_superpages(v); 
+    pse2M = !!(gflags & _PAGE_PSE);
 
     if ( pse2M )
     {
@@ -348,6 +359,8 @@ guest_walk_tables(struct vcpu *v, struct p2m_domain *p2m,
             /* _PAGE_PSE_PAT not set: remove _PAGE_PAT from flags. */
             flags &= ~_PAGE_PAT;
 
+        if ( !guest_supports_superpages(v) )
+            rc |= _PAGE_PSE | _PAGE_INVALID_BIT;
         if ( gfn_x(start) & GUEST_L2_GFN_MASK & ~0x1 )
             rc |= _PAGE_INVALID_BITS;
 
diff --git a/xen/include/asm-x86/guest_pt.h b/xen/include/asm-x86/guest_pt.h
index 79ed4ff..6bdb7ca 100644
--- a/xen/include/asm-x86/guest_pt.h
+++ b/xen/include/asm-x86/guest_pt.h
@@ -179,14 +179,18 @@ static inline guest_l4e_t guest_l4e_from_gfn(gfn_t gfn, u32 flags)
 static inline int
 guest_supports_superpages(struct vcpu *v)
 {
-    /* The _PAGE_PSE bit must be honoured in HVM guests, whenever
-     * CR4.PSE is set or the guest is in PAE or long mode. 
-     * It's also used in the dummy PT for vcpus with CR4.PG cleared. */
-    return (is_pv_vcpu(v)
-            ? opt_allow_superpage
-            : (GUEST_PAGING_LEVELS != 2 
-               || !hvm_paging_enabled(v)
-               || (v->arch.hvm_vcpu.guest_cr[4] & X86_CR4_PSE)));
+    /*
+     * PV guests use Xen's paging settings.  Being 4-level, 2M
+     * superpages are unconditionally supported.
+     *
+     * The L2 _PAGE_PSE bit must be honoured in HVM guests, whenever
+     * CR4.PSE is set or the guest is in PAE or long mode.
+     * It's also used in the dummy PT for vcpus with CR0.PG cleared.
+     */
+    return (is_pv_vcpu(v) ||
+            GUEST_PAGING_LEVELS != 2 ||
+            !hvm_paging_enabled(v) ||
+            (v->arch.hvm_vcpu.guest_cr[4] & X86_CR4_PSE));
 }
 
 static inline int
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 3/3 Xen-4.8] x86/dom0: Fix shadowing of PV guests with 2M superpages

[Jan Beulich]

>>> On 09.11.18 at 16:26, <andrew.cooper3@citrix.com> wrote:
> This is a minimal backport of pieces of:
> 
>  c/s 28d9a9a2d41759b9e5163037b759ac557aea767c
>  c/s 4c5d78a10dc89427140a50a1df5a0b8e9f073e82
> 
> to fix a PV shadowing problem which I hadn't anticipated at the time these
> fixes were first accepted.
> 
> Having opt_allow_superpage disabled causes guest_supports_superpages() to
> return false for PV guests.  Returning false causes guest_walk_tables() to
> ignore L2 superpages, and read under them.
> 
> This ignoring behaviour is correct for 2-level paging when CR4.PSE is clear,
> but isn't correct for 3- or 4-level paging.
> 
> When opt_allow_superpage is clear, PV domU's can't have superpages, but dom0
> will still have its initial P2M constructed with 2M superpages.
> 
> The end result is that, if dom0 becomes shadowed (e.g. PV-L1TF), the next
> memory access touching a P2M superpage will cause the shadow code to read
> under the P2M superpage and attempt to shadow junk.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel