Xen devel - Nov 2012 - [PATCH] x86: fix hypercall continuation cancellation in XENMAPSPACE_gmfn_range compat wrapper

When no continuation was established, there must also not be an attempt
to cancel it - hypercall_cancel_continuation(), in the non-HVM, non-
multicall case, adjusts the guest mode return address in a way assuming
that an earlier call hypercall_create_continuation() took place.

Once touching this code, also restructure it slightly to improve
readability and switch to using the more relaxed copy function (copying
from the same guest memory already validated the virtual address
range).

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

--- a/xen/arch/x86/x86_64/compat/mm.c
+++ b/xen/arch/x86/x86_64/compat/mm.c
@@ -66,21 +66,20 @@ int compat_arch_memory_op(int op, XEN_GU
         XLAT_add_to_physmap(nat, &cmp);
         rc = arch_memory_op(op, guest_handle_from_ptr(nat, void));
 
-        if ( cmp.space == XENMAPSPACE_gmfn_range )
+        if ( !rc || cmp.space != XENMAPSPACE_gmfn_range )
+            break;
+
+        XLAT_add_to_physmap(&cmp, nat);
+        if ( __copy_to_guest(arg, &cmp, 1) )
         {
-            if ( rc )
-            {
-                XLAT_add_to_physmap(&cmp, nat);
-                if ( copy_to_guest(arg, &cmp, 1) )
-                {
-                    hypercall_cancel_continuation();
-                    return -EFAULT;
-                }
-            }
             if ( rc == __HYPERVISOR_memory_op )
-                hypercall_xlat_continuation(NULL, 0x2, nat, arg);
+                hypercall_cancel_continuation();
+            return -EFAULT;
         }
 
+        if ( rc == __HYPERVISOR_memory_op )
+            hypercall_xlat_continuation(NULL, 0x2, nat, arg);
+
         break;
     }
 





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

On 28/11/2012 07:43, "Jan Beulich" <JBeulich@suse.com> wrote:
> When no continuation was established, there must also not be an attempt
> to cancel it - hypercall_cancel_continuation(), in the non-HVM, non-
> multicall case, adjusts the guest mode return address in a way assuming
> that an earlier call hypercall_create_continuation() took place.
> 
> Once touching this code, also restructure it slightly to improve
> readability and switch to using the more relaxed copy function (copying
> from the same guest memory already validated the virtual address
> range).
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Keir Fraser <keir@xen.org>
> --- a/xen/arch/x86/x86_64/compat/mm.c
> +++ b/xen/arch/x86/x86_64/compat/mm.c
> @@ -66,21 +66,20 @@ int compat_arch_memory_op(int op, XEN_GU
>          XLAT_add_to_physmap(nat, &cmp);
>          rc = arch_memory_op(op, guest_handle_from_ptr(nat, void));
>  
> -        if ( cmp.space == XENMAPSPACE_gmfn_range )
> +        if ( !rc || cmp.space != XENMAPSPACE_gmfn_range )
> +            break;
> +
> +        XLAT_add_to_physmap(&cmp, nat);
> +        if ( __copy_to_guest(arg, &cmp, 1) )
>          {
> -            if ( rc )
> -            {
> -                XLAT_add_to_physmap(&cmp, nat);
> -                if ( copy_to_guest(arg, &cmp, 1) )
> -                {
> -                    hypercall_cancel_continuation();
> -                    return -EFAULT;
> -                }
> -            }
>              if ( rc == __HYPERVISOR_memory_op )
> -                hypercall_xlat_continuation(NULL, 0x2, nat, arg);
> +                hypercall_cancel_continuation();
> +            return -EFAULT;
>          }
>  
> +        if ( rc == __HYPERVISOR_memory_op )
> +            hypercall_xlat_continuation(NULL, 0x2, nat, arg);
> +
>          break;
>      }
>  
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel