[PATCH] x86/PV: address odd UB in I/O emulation

[Jan Beulich <jbeulich@xxxxxxxx>]

Compilers are certainly right in detecting UB here, given that fully
parenthesized (to express precedence) the original offending expression
was (((stub_va + p) - ctxt->io_emul_stub) + 5), which in fact exhibits
two overflows in pointer calculations. We really want to calculate
(p - ctxt->io_emul_stub) first, which is guaranteed to not overflow.

The issue was observed with clang 9 on 4.13.

The oddities are
- the issue was detected on APPEND_CALL(save_guest_gprs), despite the
  earlier similar APPEND_CALL(load_guest_gprs),
- merely casting the original offending expression to long was reported
  to also help.

While at it also avoid converting guaranteed (with our current address
space layout) negative values to unsigned long (which has implementation
defined behavior): Have stub_va be of pointer type. And since it's on an
immediately adjacent line, also constify this_stubs.

Fixes: d89e5e65f305 ("x86/ioemul: Rewrite stub generation to be shadow stack 
compatible")
Reported-by: Franklin Shen <2284696125@xxxxxx>
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
I'm not going to insist on the part avoiding implementation defined
behavior here. If I am to drop that, it is less clear whether
constifying this_stubs would then still be warranted.

--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -89,8 +89,8 @@ static io_emul_stub_t *io_emul_stub_setu
         0xc3,       /* ret       */
     };
 
-    struct stubs *this_stubs = &this_cpu(stubs);
-    unsigned long stub_va = this_stubs->addr + STUB_BUF_SIZE / 2;
+    const struct stubs *this_stubs = &this_cpu(stubs);
+    const void *stub_va = (void *)this_stubs->addr + STUB_BUF_SIZE / 2;
     unsigned int quirk_bytes = 0;
     char *p;
 
@@ -98,7 +98,7 @@ static io_emul_stub_t *io_emul_stub_setu
 #define APPEND_BUFF(b) ({ memcpy(p, b, sizeof(b)); p += sizeof(b); })
 #define APPEND_CALL(f)                                                  \
     ({                                                                  \
-        long disp = (long)(f) - (stub_va + p - ctxt->io_emul_stub + 5); \
+        long disp = (void *)(f) - (stub_va + (p - ctxt->io_emul_stub) + 5); \
         BUG_ON((int32_t)disp != disp);                                  \
         *p++ = 0xe8;                                                    \
         *(int32_t *)p = disp; p += 4;                                   \
@@ -106,7 +106,7 @@ static io_emul_stub_t *io_emul_stub_setu
 
     if ( !ctxt->io_emul_stub )
         ctxt->io_emul_stub =
-            map_domain_page(_mfn(this_stubs->mfn)) + (stub_va & ~PAGE_MASK);
+            map_domain_page(_mfn(this_stubs->mfn)) + PAGE_OFFSET(stub_va);
 
     p = ctxt->io_emul_stub;
 
@@ -141,7 +141,7 @@ static io_emul_stub_t *io_emul_stub_setu
     block_speculation(); /* SCSB */
 
     /* Handy function-typed pointer to the stub. */
-    return (void *)stub_va;
+    return stub_va;
 
 #undef APPEND_CALL
 #undef APPEND_BUFF