* [PATCH] x86/xenoprof: fix 32-bit guest stack handling after c/s 24537:3c0a533d3af0
@ 2012-03-06 15:04 Jan Beulich
  2012-03-06 15:15 ` Keir Fraser
  0 siblings, 1 reply; 2+ messages in thread
From: Jan Beulich @ 2012-03-06 15:04 UTC (permalink / raw)
  To: xen-devel; +Cc: George Dunlap, Marcus Granado
[-- Attachment #1: Type: text/plain, Size: 3408 bytes --]
32-bit guests don't have 64-bit precudrure return addresses - both
elements of struct frame_head_32bit should be 32 bits wide, not just
the frame link pointer.
Further, consolidate the whole handling here (also in the native size
guest case) to properly use guest handles and guest memory accessors.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/oprofile/backtrace.c
+++ b/xen/arch/x86/oprofile/backtrace.c
@@ -13,18 +13,22 @@
 #include<xen/types.h>
 #include<asm/page.h>
 #include<xen/xenoprof.h>
-#include<asm/guest_access.h>
+#include<xen/guest_access.h>
 
 struct frame_head {
     struct frame_head * ebp;
     unsigned long ret;
 } __attribute__((packed));
+typedef struct frame_head frame_head_t;
+DEFINE_XEN_GUEST_HANDLE(frame_head_t);
 
 #ifdef CONFIG_X86_64
 struct frame_head_32bit {
     uint32_t ebp;
-    unsigned long ret;
+    uint32_t ret;
 } __attribute__((packed));
+typedef struct frame_head_32bit frame_head32_t;
+DEFINE_COMPAT_HANDLE(frame_head32_t);
 #endif
 
 static struct frame_head *
@@ -54,44 +58,47 @@ static inline int is_32bit_vcpu(struct v
 
 static struct frame_head *
 dump_guest_backtrace(struct domain *d, struct vcpu *vcpu, 
-		     struct frame_head * head, int mode)
+		     const struct frame_head *head, int mode)
 {
-    struct frame_head bufhead[2];
-    XEN_GUEST_HANDLE(char) guest_head = guest_handle_from_ptr(head, char);
+    frame_head_t bufhead;
 
 #ifdef CONFIG_X86_64
     if ( is_32bit_vcpu(vcpu) )
     {
-        struct frame_head_32bit bufhead32[2];
+        __compat_handle_const_frame_head32_t guest_head =
+            { .c = (unsigned long)head };
+        frame_head32_t bufhead32;
+
         /* Also check accessibility of one struct frame_head beyond */
-        if (!guest_handle_okay(guest_head, sizeof(bufhead32)))
+        if (!compat_handle_okay(guest_head, 2))
             return 0;
-        if (__copy_from_guest_offset((char *)bufhead32, guest_head, 0, 
-                                     sizeof(bufhead32)))
+        if (__copy_from_compat(&bufhead32, guest_head, 1))
             return 0;
-        bufhead[0].ebp=(struct frame_head *)(unsigned long)bufhead32[0].ebp;
-        bufhead[0].ret=bufhead32[0].ret;
+        bufhead.ebp = (struct frame_head *)(unsigned long)bufhead32.ebp;
+        bufhead.ret = bufhead32.ret;
     }
     else
 #endif
     {
+        XEN_GUEST_HANDLE(const_frame_head_t) guest_head =
+            const_guest_handle_from_ptr(head, frame_head_t);
+
         /* Also check accessibility of one struct frame_head beyond */
-        if (!guest_handle_okay(guest_head, sizeof(bufhead)))
+        if (!guest_handle_okay(guest_head, 2))
             return 0;
-        if (__copy_from_guest_offset((char *)bufhead, guest_head, 0, 
-                                     sizeof(bufhead)))
+        if (__copy_from_guest(&bufhead, guest_head, 1))
             return 0;
     }
     
-    if (!xenoprof_add_trace(d, vcpu, bufhead[0].ret, mode))
+    if (!xenoprof_add_trace(d, vcpu, bufhead.ret, mode))
         return 0;
     
     /* frame pointers should strictly progress back up the stack
      * (towards higher addresses) */
-    if (head >= bufhead[0].ebp)
+    if (head >= bufhead.ebp)
         return NULL;
     
-    return bufhead[0].ebp;
+    return bufhead.ebp;
 }
 
 /*
[-- Attachment #2: x86-oprof-fix-24537.patch --]
[-- Type: text/plain, Size: 3481 bytes --]
x86/xenoprof: fix 32-bit guest stack handling after c/s 24537:3c0a533d3af0 
32-bit guests don't have 64-bit precudrure return addresses - both
elements of struct frame_head_32bit should be 32 bits wide, not just
the frame link pointer.
Further, consolidate the whole handling here (also in the native size
guest case) to properly use guest handles and guest memory accessors.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/oprofile/backtrace.c
+++ b/xen/arch/x86/oprofile/backtrace.c
@@ -13,18 +13,22 @@
 #include<xen/types.h>
 #include<asm/page.h>
 #include<xen/xenoprof.h>
-#include<asm/guest_access.h>
+#include<xen/guest_access.h>
 
 struct frame_head {
     struct frame_head * ebp;
     unsigned long ret;
 } __attribute__((packed));
+typedef struct frame_head frame_head_t;
+DEFINE_XEN_GUEST_HANDLE(frame_head_t);
 
 #ifdef CONFIG_X86_64
 struct frame_head_32bit {
     uint32_t ebp;
-    unsigned long ret;
+    uint32_t ret;
 } __attribute__((packed));
+typedef struct frame_head_32bit frame_head32_t;
+DEFINE_COMPAT_HANDLE(frame_head32_t);
 #endif
 
 static struct frame_head *
@@ -54,44 +58,47 @@ static inline int is_32bit_vcpu(struct v
 
 static struct frame_head *
 dump_guest_backtrace(struct domain *d, struct vcpu *vcpu, 
-		     struct frame_head * head, int mode)
+		     const struct frame_head *head, int mode)
 {
-    struct frame_head bufhead[2];
-    XEN_GUEST_HANDLE(char) guest_head = guest_handle_from_ptr(head, char);
+    frame_head_t bufhead;
 
 #ifdef CONFIG_X86_64
     if ( is_32bit_vcpu(vcpu) )
     {
-        struct frame_head_32bit bufhead32[2];
+        __compat_handle_const_frame_head32_t guest_head =
+            { .c = (unsigned long)head };
+        frame_head32_t bufhead32;
+
         /* Also check accessibility of one struct frame_head beyond */
-        if (!guest_handle_okay(guest_head, sizeof(bufhead32)))
+        if (!compat_handle_okay(guest_head, 2))
             return 0;
-        if (__copy_from_guest_offset((char *)bufhead32, guest_head, 0, 
-                                     sizeof(bufhead32)))
+        if (__copy_from_compat(&bufhead32, guest_head, 1))
             return 0;
-        bufhead[0].ebp=(struct frame_head *)(unsigned long)bufhead32[0].ebp;
-        bufhead[0].ret=bufhead32[0].ret;
+        bufhead.ebp = (struct frame_head *)(unsigned long)bufhead32.ebp;
+        bufhead.ret = bufhead32.ret;
     }
     else
 #endif
     {
+        XEN_GUEST_HANDLE(const_frame_head_t) guest_head =
+            const_guest_handle_from_ptr(head, frame_head_t);
+
         /* Also check accessibility of one struct frame_head beyond */
-        if (!guest_handle_okay(guest_head, sizeof(bufhead)))
+        if (!guest_handle_okay(guest_head, 2))
             return 0;
-        if (__copy_from_guest_offset((char *)bufhead, guest_head, 0, 
-                                     sizeof(bufhead)))
+        if (__copy_from_guest(&bufhead, guest_head, 1))
             return 0;
     }
     
-    if (!xenoprof_add_trace(d, vcpu, bufhead[0].ret, mode))
+    if (!xenoprof_add_trace(d, vcpu, bufhead.ret, mode))
         return 0;
     
     /* frame pointers should strictly progress back up the stack
      * (towards higher addresses) */
-    if (head >= bufhead[0].ebp)
+    if (head >= bufhead.ebp)
         return NULL;
     
-    return bufhead[0].ebp;
+    return bufhead.ebp;
 }
 
 /*
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply	[flat|nested] 2+ messages in thread
* Re: [PATCH] x86/xenoprof: fix 32-bit guest stack handling after c/s 24537:3c0a533d3af0
  2012-03-06 15:04 [PATCH] x86/xenoprof: fix 32-bit guest stack handling after c/s 24537:3c0a533d3af0 Jan Beulich
@ 2012-03-06 15:15 ` Keir Fraser
  0 siblings, 0 replies; 2+ messages in thread
From: Keir Fraser @ 2012-03-06 15:15 UTC (permalink / raw)
  To: Jan Beulich, xen-devel; +Cc: George Dunlap, Marcus Granado
On 06/03/2012 15:04, "Jan Beulich" <JBeulich@suse.com> wrote:
> 32-bit guests don't have 64-bit precudrure return addresses - both
> elements of struct frame_head_32bit should be 32 bits wide, not just
> the frame link pointer.
> 
> Further, consolidate the whole handling here (also in the native size
> guest case) to properly use guest handles and guest memory accessors.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Keir Fraser <keir@xen.org>
> --- a/xen/arch/x86/oprofile/backtrace.c
> +++ b/xen/arch/x86/oprofile/backtrace.c
> @@ -13,18 +13,22 @@
>  #include<xen/types.h>
>  #include<asm/page.h>
>  #include<xen/xenoprof.h>
> -#include<asm/guest_access.h>
> +#include<xen/guest_access.h>
>  
>  struct frame_head {
>      struct frame_head * ebp;
>      unsigned long ret;
>  } __attribute__((packed));
> +typedef struct frame_head frame_head_t;
> +DEFINE_XEN_GUEST_HANDLE(frame_head_t);
>  
>  #ifdef CONFIG_X86_64
>  struct frame_head_32bit {
>      uint32_t ebp;
> -    unsigned long ret;
> +    uint32_t ret;
>  } __attribute__((packed));
> +typedef struct frame_head_32bit frame_head32_t;
> +DEFINE_COMPAT_HANDLE(frame_head32_t);
>  #endif
>  
>  static struct frame_head *
> @@ -54,44 +58,47 @@ static inline int is_32bit_vcpu(struct v
>  
>  static struct frame_head *
>  dump_guest_backtrace(struct domain *d, struct vcpu *vcpu,
> -       struct frame_head * head, int mode)
> +       const struct frame_head *head, int mode)
>  {
> -    struct frame_head bufhead[2];
> -    XEN_GUEST_HANDLE(char) guest_head = guest_handle_from_ptr(head, char);
> +    frame_head_t bufhead;
>  
>  #ifdef CONFIG_X86_64
>      if ( is_32bit_vcpu(vcpu) )
>      {
> -        struct frame_head_32bit bufhead32[2];
> +        __compat_handle_const_frame_head32_t guest_head =
> +            { .c = (unsigned long)head };
> +        frame_head32_t bufhead32;
> +
>          /* Also check accessibility of one struct frame_head beyond */
> -        if (!guest_handle_okay(guest_head, sizeof(bufhead32)))
> +        if (!compat_handle_okay(guest_head, 2))
>              return 0;
> -        if (__copy_from_guest_offset((char *)bufhead32, guest_head, 0,
> -                                     sizeof(bufhead32)))
> +        if (__copy_from_compat(&bufhead32, guest_head, 1))
>              return 0;
> -        bufhead[0].ebp=(struct frame_head *)(unsigned long)bufhead32[0].ebp;
> -        bufhead[0].ret=bufhead32[0].ret;
> +        bufhead.ebp = (struct frame_head *)(unsigned long)bufhead32.ebp;
> +        bufhead.ret = bufhead32.ret;
>      }
>      else
>  #endif
>      {
> +        XEN_GUEST_HANDLE(const_frame_head_t) guest_head =
> +            const_guest_handle_from_ptr(head, frame_head_t);
> +
>          /* Also check accessibility of one struct frame_head beyond */
> -        if (!guest_handle_okay(guest_head, sizeof(bufhead)))
> +        if (!guest_handle_okay(guest_head, 2))
>              return 0;
> -        if (__copy_from_guest_offset((char *)bufhead, guest_head, 0,
> -                                     sizeof(bufhead)))
> +        if (__copy_from_guest(&bufhead, guest_head, 1))
>              return 0;
>      }
>      
> -    if (!xenoprof_add_trace(d, vcpu, bufhead[0].ret, mode))
> +    if (!xenoprof_add_trace(d, vcpu, bufhead.ret, mode))
>          return 0;
>      
>      /* frame pointers should strictly progress back up the stack
>       * (towards higher addresses) */
> -    if (head >= bufhead[0].ebp)
> +    if (head >= bufhead.ebp)
>          return NULL;
>      
> -    return bufhead[0].ebp;
> +    return bufhead.ebp;
>  }
>  
>  /*
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2012-03-06 15:15 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-06 15:04 [PATCH] x86/xenoprof: fix 32-bit guest stack handling after c/s 24537:3c0a533d3af0 Jan Beulich
2012-03-06 15:15 ` Keir Fraser
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).