x86/pagewalk: Read guest PTEs with ACCESS_ONCE()

This has been a plain C read for as far back as I can trace in history.

Research into invented-loads has flagged it as a possible vulnerability.
After analysis, it is believed to be a bug only, not a security vulnerability.

The analysis suggests that we can read one value out of the guest, operate on
another, and that this could be an in-guest privliege escalation.  Any entity
in the guest able to modify the pagetables already has full privilege, so
while Xen can potentially malfunction, the effects don't cross a privilege
boundary.

The analysis also suggests that this is worse for shadow guests because we may
put the TOCTOU entry in the shadows, but this is inaccurate.  What we put in
the shadows is still translated under the P2M and refers to guest physical
address space.

Either way, the code fits the pattern for invented loads, and it is a risk;
harden the accesses.

Link: https://github.com/xoreaxeaxeax/schrodingers-toctou/blob/main/observer-effect/audits/audit-xen-ptwalk-RELEASE-4.21.1.md#86-per-candidate-finding
Fixes: 49f7c7364e ("Replace shadow pagetable code with shadow2.")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Roger Pau Monné <roger@xenproject.org>

xen/arch/x86/mm/guest_walk.c

@@ -129,7 +129,7 @@ guest_walk_tables(const struct vcpu *v, struct p2m_domain *p2m,
             guest_l4_table_offset(va) * sizeof(gw->l4e);
     if ( !hvmemul_read_cache(v, l4gpa, &gw->l4e, sizeof(gw->l4e)) )
     {
-        gw->l4e = l4p[guest_l4_table_offset(va)];
+        gw->l4e = (guest_l4e_t){ ACCESS_ONCE(l4p[guest_l4_table_offset(va)].l4) };
         hvmemul_write_cache(v, l4gpa, &gw->l4e, sizeof(gw->l4e));
     }
     gflags = guest_l4e_get_flags(gw->l4e);
@@ -164,7 +164,7 @@ guest_walk_tables(const struct vcpu *v, struct p2m_domain *p2m,
             guest_l3_table_offset(va) * sizeof(gw->l3e);
     if ( !hvmemul_read_cache(v, l3gpa, &gw->l3e, sizeof(gw->l3e)) )
     {
-        gw->l3e = l3p[guest_l3_table_offset(va)];
+        gw->l3e = (guest_l3e_t){ ACCESS_ONCE(l3p[guest_l3_table_offset(va)].l3) };
         hvmemul_write_cache(v, l3gpa, &gw->l3e, sizeof(gw->l3e));
     }
     gflags = guest_l3e_get_flags(gw->l3e);
@@ -264,7 +264,7 @@ guest_walk_tables(const struct vcpu *v, struct p2m_domain *p2m,
     l2gpa += guest_l2_table_offset(va) * sizeof(gw->l2e);
     if ( !hvmemul_read_cache(v, l2gpa, &gw->l2e, sizeof(gw->l2e)) )
     {
-        gw->l2e = l2p[guest_l2_table_offset(va)];
+        gw->l2e = (guest_l2e_t){ ACCESS_ONCE(l2p[guest_l2_table_offset(va)].l2) };
         hvmemul_write_cache(v, l2gpa, &gw->l2e, sizeof(gw->l2e));
     }
 
@@ -353,7 +353,7 @@ guest_walk_tables(const struct vcpu *v, struct p2m_domain *p2m,
             guest_l1_table_offset(va) * sizeof(gw->l1e);
     if ( !hvmemul_read_cache(v, l1gpa, &gw->l1e, sizeof(gw->l1e)) )
     {
-        gw->l1e = l1p[guest_l1_table_offset(va)];
+        gw->l1e = (guest_l1e_t){ ACCESS_ONCE(l1p[guest_l1_table_offset(va)].l1) };
         hvmemul_write_cache(v, l1gpa, &gw->l1e, sizeof(gw->l1e));
     }