[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



commit c11b8d25fbe9c0155e91409594ea6701008409ed
Author:     Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Mon Oct 18 14:23:29 2021 +0200
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Mon Oct 18 14:23:29 2021 +0200

    x86/PV: address odd UB in I/O emulation
    
    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>
    Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
---
 xen/arch/x86/pv/emul-priv-op.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index d0df5bc616..7f4279a051 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -90,8 +90,8 @@ static io_emul_stub_t *io_emul_stub_setup(struct priv_op_ctxt 
*ctxt, u8 opcode,
         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;
 
@@ -99,7 +99,7 @@ static io_emul_stub_t *io_emul_stub_setup(struct priv_op_ctxt 
*ctxt, u8 opcode,
 #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;                                   \
@@ -107,7 +107,7 @@ static io_emul_stub_t *io_emul_stub_setup(struct 
priv_op_ctxt *ctxt, u8 opcode,
 
     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;
 
@@ -142,7 +142,7 @@ static io_emul_stub_t *io_emul_stub_setup(struct 
priv_op_ctxt *ctxt, u8 opcode,
     block_speculation(); /* SCSB */
 
     /* Handy function-typed pointer to the stub. */
-    return (void *)stub_va;
+    return stub_va;
 
 #undef APPEND_CALL
 #undef APPEND_BUFF
--
generated by git-patchbot for /home/xen/git/xen.git#master



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.