[Xen-devel] [PATCH] RFC x86/hvm: Don't truncate the hvm hypercall index before range checking it

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

When the compat hypercall ABI was added for HVM guests (i.e. supporting 32bit
operating systems making hypercalls against a 64bit Xen), an ABI breakage was
introduced for non-compat guests, as the 64bit hypercall index became
truncated to 32 bits.

This has been the case for a very long time, but is not very obvious from the
code, and definitely counterintuitive, seeing as all other 64bit parameters
are passed without truncation.

However, the only supported method of making hypercalls is to call into the
hypercall page, which in practice means that only hypercall index up to 63 are
supported.

Therefore, take the opportunity to fix the ABI before it becomes impossible to
fix.

While tweaking this area, fix one piece of trailing whitespace.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>

RFC because this might manifest as an ABI change for guests making hypercalls
via an unsupported mechanism.  However, I feel that fixing the bug is the less
bad option.
---
 xen/arch/x86/hvm/hvm.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 3c90ecd..563f029 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4265,11 +4265,11 @@ int hvm_do_hypercall(struct cpu_user_regs *regs)
     struct domain *currd = curr->domain;
     struct segment_register sreg;
     int mode = hvm_guest_x86_mode(curr);
-    uint32_t eax = regs->eax;
+    unsigned long eax;
 
     switch ( mode )
     {
-    case 8:        
+    case 8:
     case 4:
     case 2:
         hvm_get_segment_register(curr, x86_seg_ss, &sreg);
@@ -4283,6 +4283,8 @@ int hvm_do_hypercall(struct cpu_user_regs *regs)
         break;
     }
 
+    eax = (mode == 8) ? regs->eax : regs->_eax;
+
     if ( (eax & 0x80000000) && is_viridian_domain(currd) )
         return viridian_hypercall(regs);
 
@@ -4307,7 +4309,7 @@ int hvm_do_hypercall(struct cpu_user_regs *regs)
         unsigned long r8 = regs->r8;
         unsigned long r9 = regs->r9;
 
-        HVM_DBG_LOG(DBG_LEVEL_HCALL, "hcall%u(%lx, %lx, %lx, %lx, %lx, %lx)",
+        HVM_DBG_LOG(DBG_LEVEL_HCALL, "hcall%lu(%lx, %lx, %lx, %lx, %lx, %lx)",
                     eax, rdi, rsi, rdx, r10, r8, r9);
 
 #ifndef NDEBUG
@@ -4354,7 +4356,7 @@ int hvm_do_hypercall(struct cpu_user_regs *regs)
         unsigned int edi = regs->_edi;
         unsigned int ebp = regs->_ebp;
 
-        HVM_DBG_LOG(DBG_LEVEL_HCALL, "hcall%u(%x, %x, %x, %x, %x, %x)", eax,
+        HVM_DBG_LOG(DBG_LEVEL_HCALL, "hcall%lu(%x, %x, %x, %x, %x, %x)", eax,
                     ebx, ecx, edx, esi, edi, ebp);
 
 #ifndef NDEBUG
@@ -4390,7 +4392,7 @@ int hvm_do_hypercall(struct cpu_user_regs *regs)
 #endif
     }
 
-    HVM_DBG_LOG(DBG_LEVEL_HCALL, "hcall%u -> %lx",
+    HVM_DBG_LOG(DBG_LEVEL_HCALL, "hcall%lu -> %lx",
                 eax, (unsigned long)regs->eax);
 
     if ( curr->arch.hvm_vcpu.hcall_preempted )
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel