From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 2/3] x86/HVM: clobber hypercall arguments just like for PV Date: Thu, 8 Jan 2015 17:20:04 +0000 Message-ID: <54AEBC44.9070702@citrix.com> References: <54AEAD2C0200007800052B56@mail.emea.novell.com> <54AEAEDE0200007800052B7D@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6994251016793208980==" Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Y9GzH-0007gp-Gr for xen-devel@lists.xenproject.org; Thu, 08 Jan 2015 17:35:11 +0000 In-Reply-To: <54AEAEDE0200007800052B7D@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , xen-devel Cc: Keir Fraser List-Id: xen-devel@lists.xenproject.org --===============6994251016793208980== Content-Type: multipart/alternative; boundary="------------090401090303000508030503" --------------090401090303000508030503 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: quoted-printable On 08/01/15 15:22, Jan Beulich wrote: > Unused arguments get clobbered before the call (not affecting caller > visible state), while used arguments get clobbered afterwards unless > a continuation is needed (affecting caller visible state). > > Signed-off-by: Jan Beulich After a long time pouring over the Microsoft register calling conventions documentation, and the Windows PV driver code, I am now convinced that they are performing appropriate parameter saving. Reviewed-by: Andrew Cooper > > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -4818,6 +4818,8 @@ static hvm_hypercall_t *const pvh_hyperc > [ __HYPERVISOR_arch_1 ] =3D (hvm_hypercall_t *)paging_domctl_conti= nuation > }; > =20 > +extern const uint8_t hypercall_args_table[], compat_hypercall_args_tab= le[]; > + > int hvm_do_hypercall(struct cpu_user_regs *regs) > { > struct vcpu *curr =3D current; > @@ -4856,36 +4858,95 @@ int hvm_do_hypercall(struct cpu_user_reg > =20 > if ( mode =3D=3D 8 ) > { > + unsigned long rdi =3D regs->rdi; > + unsigned long rsi =3D regs->rsi; > + unsigned long rdx =3D regs->rdx; > + unsigned long r10 =3D regs->r10; > + unsigned long r8 =3D regs->r8; > + unsigned long r9 =3D regs->r9; > + > HVM_DBG_LOG(DBG_LEVEL_HCALL, "hcall%u(%lx, %lx, %lx, %lx, %lx,= %lx)", > - eax, regs->rdi, regs->rsi, regs->rdx, > - regs->r10, regs->r8, regs->r9); > + eax, rdi, rsi, rdx, r10, r8, r9); > + > +#ifndef NDEBUG > + /* Deliberately corrupt parameter regs not used by this hyperc= all. */ > + switch ( hypercall_args_table[eax] ) > + { > + case 0: rdi =3D 0xdeadbeefdeadf00dUL; > + case 1: rsi =3D 0xdeadbeefdeadf00dUL; > + case 2: rdx =3D 0xdeadbeefdeadf00dUL; > + case 3: r10 =3D 0xdeadbeefdeadf00dUL; > + case 4: r8 =3D 0xdeadbeefdeadf00dUL; > + case 5: r9 =3D 0xdeadbeefdeadf00dUL; > + } > +#endif > =20 > curr->arch.hvm_vcpu.hcall_64bit =3D 1; > - if ( is_pvh_vcpu(curr) ) > - regs->rax =3D pvh_hypercall64_table[eax](regs->rdi, regs->= rsi, > - regs->rdx, regs->r1= 0, > - regs->r8, regs->r9)= ; > - else > - regs->rax =3D hvm_hypercall64_table[eax](regs->rdi, regs->= rsi, > - regs->rdx, regs->r1= 0, > - regs->r8, regs->r9)= ; > + regs->rax =3D (is_pvh_vcpu(curr) > + ? pvh_hypercall64_table > + : hvm_hypercall64_table)[eax](rdi, rsi, rdx, r10,= r8, r9); > curr->arch.hvm_vcpu.hcall_64bit =3D 0; > + > +#ifndef NDEBUG > + if ( !curr->arch.hvm_vcpu.hcall_preempted ) > + { > + /* Deliberately corrupt parameter regs used by this hyperc= all. */ > + switch ( hypercall_args_table[eax] ) > + { > + case 6: regs->r9 =3D 0xdeadbeefdeadf00dUL; > + case 5: regs->r8 =3D 0xdeadbeefdeadf00dUL; > + case 4: regs->r10 =3D 0xdeadbeefdeadf00dUL; > + case 3: regs->edx =3D 0xdeadbeefdeadf00dUL; > + case 2: regs->esi =3D 0xdeadbeefdeadf00dUL; > + case 1: regs->edi =3D 0xdeadbeefdeadf00dUL; > + } > + } > +#endif > } > else if ( unlikely(is_pvh_vcpu(curr)) ) > regs->_eax =3D -ENOSYS; /* PVH 32bitfixme. */ > else > { > + unsigned int ebx =3D regs->_ebx; > + unsigned int ecx =3D regs->_ecx; > + unsigned int edx =3D regs->_edx; > + unsigned int esi =3D regs->_esi; > + unsigned int edi =3D regs->_edi; > + unsigned int ebp =3D regs->_ebp; > + > HVM_DBG_LOG(DBG_LEVEL_HCALL, "hcall%u(%x, %x, %x, %x, %x, %x)"= , eax, > - (uint32_t)regs->ebx, (uint32_t)regs->ecx, > - (uint32_t)regs->edx, (uint32_t)regs->esi, > - (uint32_t)regs->edi, (uint32_t)regs->ebp); > - > - regs->eax =3D hvm_hypercall32_table[eax]((uint32_t)regs->ebx, > - (uint32_t)regs->ecx, > - (uint32_t)regs->edx, > - (uint32_t)regs->esi, > - (uint32_t)regs->edi, > - (uint32_t)regs->ebp); > + ebx, ecx, edx, esi, edi, ebp); > + > +#ifndef NDEBUG > + /* Deliberately corrupt parameter regs not used by this hyperc= all. */ > + switch ( compat_hypercall_args_table[eax] ) > + { > + case 0: ebx =3D 0xdeadf00d; > + case 1: ecx =3D 0xdeadf00d; > + case 2: edx =3D 0xdeadf00d; > + case 3: esi =3D 0xdeadf00d; > + case 4: edi =3D 0xdeadf00d; > + case 5: ebp =3D 0xdeadf00d; > + } > +#endif > + > + regs->_eax =3D hvm_hypercall32_table[eax](ebx, ecx, edx, esi, = edi, ebp); > + > +#ifndef NDEBUG > + if ( !curr->arch.hvm_vcpu.hcall_preempted ) > + { > + /* Deliberately corrupt parameter regs used by this hyperc= all. */ > + switch ( compat_hypercall_args_table[eax] ) > + { > + case 6: regs->ebp =3D 0xdeadf00d; > + case 5: regs->edi =3D 0xdeadf00d; > + case 4: regs->esi =3D 0xdeadf00d; > + case 3: regs->edx =3D 0xdeadf00d; > + case 2: regs->ecx =3D 0xdeadf00d; > + case 1: regs->ebx =3D 0xdeadf00d; > + } > + } > +#endif > } > =20 > HVM_DBG_LOG(DBG_LEVEL_HCALL, "hcall%u -> %lx", > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel --------------090401090303000508030503 Content-Type: text/html; charset="windows-1252" Content-Transfer-Encoding: 8bit
On 08/01/15 15:22, Jan Beulich wrote:
Unused arguments get clobbered before the call (not affecting caller
visible state), while used arguments get clobbered afterwards unless
a continuation is needed (affecting caller visible state).

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

After a long time pouring over the Microsoft register calling conventions documentation, and the Windows PV driver code, I am now convinced that they are performing appropriate parameter saving.

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>


--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4818,6 +4818,8 @@ static hvm_hypercall_t *const pvh_hyperc
     [ __HYPERVISOR_arch_1 ] = (hvm_hypercall_t *)paging_domctl_continuation
 };
 
+extern const uint8_t hypercall_args_table[], compat_hypercall_args_table[];
+
 int hvm_do_hypercall(struct cpu_user_regs *regs)
 {
     struct vcpu *curr = current;
@@ -4856,36 +4858,95 @@ int hvm_do_hypercall(struct cpu_user_reg
 
     if ( mode == 8 )
     {
+        unsigned long rdi = regs->rdi;
+        unsigned long rsi = regs->rsi;
+        unsigned long rdx = regs->rdx;
+        unsigned long r10 = regs->r10;
+        unsigned long r8 = regs->r8;
+        unsigned long r9 = regs->r9;
+
         HVM_DBG_LOG(DBG_LEVEL_HCALL, "hcall%u(%lx, %lx, %lx, %lx, %lx, %lx)",
-                    eax, regs->rdi, regs->rsi, regs->rdx,
-                    regs->r10, regs->r8, regs->r9);
+                    eax, rdi, rsi, rdx, r10, r8, r9);
+
+#ifndef NDEBUG
+        /* Deliberately corrupt parameter regs not used by this hypercall. */
+        switch ( hypercall_args_table[eax] )
+        {
+        case 0: rdi = 0xdeadbeefdeadf00dUL;
+        case 1: rsi = 0xdeadbeefdeadf00dUL;
+        case 2: rdx = 0xdeadbeefdeadf00dUL;
+        case 3: r10 = 0xdeadbeefdeadf00dUL;
+        case 4: r8 = 0xdeadbeefdeadf00dUL;
+        case 5: r9 = 0xdeadbeefdeadf00dUL;
+        }
+#endif
 
         curr->arch.hvm_vcpu.hcall_64bit = 1;
-        if ( is_pvh_vcpu(curr) )
-            regs->rax = pvh_hypercall64_table[eax](regs->rdi, regs->rsi,
-                                                   regs->rdx, regs->r10,
-                                                   regs->r8, regs->r9);
-        else
-            regs->rax = hvm_hypercall64_table[eax](regs->rdi, regs->rsi,
-                                                   regs->rdx, regs->r10,
-                                                   regs->r8, regs->r9);
+        regs->rax = (is_pvh_vcpu(curr)
+                     ? pvh_hypercall64_table
+                     : hvm_hypercall64_table)[eax](rdi, rsi, rdx, r10, r8, r9);
         curr->arch.hvm_vcpu.hcall_64bit = 0;
+
+#ifndef NDEBUG
+        if ( !curr->arch.hvm_vcpu.hcall_preempted )
+        {
+            /* Deliberately corrupt parameter regs used by this hypercall. */
+            switch ( hypercall_args_table[eax] )
+            {
+            case 6: regs->r9  = 0xdeadbeefdeadf00dUL;
+            case 5: regs->r8  = 0xdeadbeefdeadf00dUL;
+            case 4: regs->r10 = 0xdeadbeefdeadf00dUL;
+            case 3: regs->edx = 0xdeadbeefdeadf00dUL;
+            case 2: regs->esi = 0xdeadbeefdeadf00dUL;
+            case 1: regs->edi = 0xdeadbeefdeadf00dUL;
+            }
+        }
+#endif
     }
     else if ( unlikely(is_pvh_vcpu(curr)) )
         regs->_eax = -ENOSYS; /* PVH 32bitfixme. */
     else
     {
+        unsigned int ebx = regs->_ebx;
+        unsigned int ecx = regs->_ecx;
+        unsigned int edx = regs->_edx;
+        unsigned int esi = regs->_esi;
+        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,
-                    (uint32_t)regs->ebx, (uint32_t)regs->ecx,
-                    (uint32_t)regs->edx, (uint32_t)regs->esi,
-                    (uint32_t)regs->edi, (uint32_t)regs->ebp);
-
-        regs->eax = hvm_hypercall32_table[eax]((uint32_t)regs->ebx,
-                                               (uint32_t)regs->ecx,
-                                               (uint32_t)regs->edx,
-                                               (uint32_t)regs->esi,
-                                               (uint32_t)regs->edi,
-                                               (uint32_t)regs->ebp);
+                    ebx, ecx, edx, esi, edi, ebp);
+
+#ifndef NDEBUG
+        /* Deliberately corrupt parameter regs not used by this hypercall. */
+        switch ( compat_hypercall_args_table[eax] )
+        {
+        case 0: ebx = 0xdeadf00d;
+        case 1: ecx = 0xdeadf00d;
+        case 2: edx = 0xdeadf00d;
+        case 3: esi = 0xdeadf00d;
+        case 4: edi = 0xdeadf00d;
+        case 5: ebp = 0xdeadf00d;
+        }
+#endif
+
+        regs->_eax = hvm_hypercall32_table[eax](ebx, ecx, edx, esi, edi, ebp);
+
+#ifndef NDEBUG
+        if ( !curr->arch.hvm_vcpu.hcall_preempted )
+        {
+            /* Deliberately corrupt parameter regs used by this hypercall. */
+            switch ( compat_hypercall_args_table[eax] )
+            {
+            case 6: regs->ebp = 0xdeadf00d;
+            case 5: regs->edi = 0xdeadf00d;
+            case 4: regs->esi = 0xdeadf00d;
+            case 3: regs->edx = 0xdeadf00d;
+            case 2: regs->ecx = 0xdeadf00d;
+            case 1: regs->ebx = 0xdeadf00d;
+            }
+        }
+#endif
     }
 
     HVM_DBG_LOG(DBG_LEVEL_HCALL, "hcall%u -> %lx",





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

--------------090401090303000508030503-- --===============6994251016793208980== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============6994251016793208980==--