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

[Xen-changelog] [xen-unstable] x86 mce: fix c/s 17968 for 32-on-64



# HG changeset patch
# User Keir Fraser <keir.fraser@xxxxxxxxxx>
# Date 1238501516 -3600
# Node ID 37f67d8224b7486c3f6e268855ec151e5528e249
# Parent  f2cf89a4e7620dbdd084a6f2b22dc1606387909a
x86 mce: fix c/s 17968 for 32-on-64

32-on-64 aspects were not properly considered. Add respective
checking, and adjust structure layouts for the cases where the
checking pointed out issues.

Also,
- fix a potential memory corruption issue (do_mca() could write beyond
  log_cpus' end if the guest specified less than the number of online
  CPUs
- there is no reason to make the (not even properly prefixed)
  definitions in xen/public/arch-x86/xen-mca.h globally visible by
  including the file from xen/public/arch-x86/xen.h.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxxxx>
---
 xen/arch/x86/cpu/mcheck/mce.c         |  123 ++++++++++++++++++++++++++--------
 xen/arch/x86/cpu/mcheck/x86_mca.h     |    2 
 xen/include/public/arch-x86/xen-mca.h |   53 ++++++--------
 xen/include/public/arch-x86/xen.h     |    4 -
 xen/include/xlat.lst                  |   16 ++++
 xen/tools/get-fields.sh               |    2 
 6 files changed, 137 insertions(+), 63 deletions(-)

diff -r f2cf89a4e762 -r 37f67d8224b7 xen/arch/x86/cpu/mcheck/mce.c
--- a/xen/arch/x86/cpu/mcheck/mce.c     Tue Mar 31 12:06:57 2009 +0100
+++ b/xen/arch/x86/cpu/mcheck/mce.c     Tue Mar 31 13:11:56 2009 +0100
@@ -984,14 +984,76 @@ static void x86_mc_mceinject(void *data)
 #error BITS_PER_LONG definition absent
 #endif
 
+#ifdef CONFIG_COMPAT
+# include <compat/arch-x86/xen-mca.h>
+
+# define xen_mcinfo_msr              mcinfo_msr
+CHECK_mcinfo_msr;
+# undef xen_mcinfo_msr
+# undef CHECK_mcinfo_msr
+# define CHECK_mcinfo_msr            struct mcinfo_msr
+
+# define xen_mcinfo_common           mcinfo_common
+CHECK_mcinfo_common;
+# undef xen_mcinfo_common
+# undef CHECK_mcinfo_common
+# define CHECK_mcinfo_common         struct mcinfo_common
+
+CHECK_FIELD_(struct, mc_fetch, flags);
+CHECK_FIELD_(struct, mc_fetch, fetch_id);
+# define CHECK_compat_mc_fetch       struct mc_fetch
+
+CHECK_FIELD_(struct, mc_physcpuinfo, ncpus);
+# define CHECK_compat_mc_physcpuinfo struct mc_physcpuinfo
+
+CHECK_mc;
+# undef CHECK_compat_mc_fetch
+# undef CHECK_compat_mc_physcpuinfo
+
+# define xen_mc_info                 mc_info
+CHECK_mc_info;
+# undef xen_mc_info
+
+# define xen_mcinfo_global           mcinfo_global
+CHECK_mcinfo_global;
+# undef xen_mcinfo_global
+
+# define xen_mcinfo_bank             mcinfo_bank
+CHECK_mcinfo_bank;
+# undef xen_mcinfo_bank
+
+# define xen_mcinfo_extended         mcinfo_extended
+CHECK_mcinfo_extended;
+# undef xen_mcinfo_extended
+
+# define xen_mcinfo_recovery         mcinfo_recovery
+# define xen_cpu_offline_action      cpu_offline_action
+# define xen_page_offline_action     page_offline_action
+CHECK_mcinfo_recovery;
+# undef xen_cpu_offline_action
+# undef xen_page_offline_action
+# undef xen_mcinfo_recovery
+#else
+# define compat_mc_fetch xen_mc_fetch
+# define compat_mc_physcpuinfo xen_mc_physcpuinfo
+# define compat_handle_is_null guest_handle_is_null
+# define copy_to_compat copy_to_guest
+#endif
+
 /* Machine Check Architecture Hypercall */
 long do_mca(XEN_GUEST_HANDLE(xen_mc_t) u_xen_mc)
 {
        long ret = 0;
        struct xen_mc curop, *op = &curop;
        struct vcpu *v = current;
-       struct xen_mc_fetch *mc_fetch;
-       struct xen_mc_physcpuinfo *mc_physcpuinfo;
+       union {
+               struct xen_mc_fetch *nat;
+               struct compat_mc_fetch *cmp;
+       } mc_fetch;
+       union {
+               struct xen_mc_physcpuinfo *nat;
+               struct compat_mc_physcpuinfo *cmp;
+       } mc_physcpuinfo;
        uint32_t flags, cmdflags;
        int nlcpu;
        xen_mc_logical_cpu_t *log_cpus = NULL;
@@ -1009,8 +1071,8 @@ long do_mca(XEN_GUEST_HANDLE(xen_mc_t) u
 
        switch (op->cmd) {
        case XEN_MC_fetch:
-               mc_fetch = &op->u.mc_fetch;
-               cmdflags = mc_fetch->flags;
+               mc_fetch.nat = &op->u.mc_fetch;
+               cmdflags = mc_fetch.nat->flags;
 
                /* This hypercall is for Dom0 only */
                if (!IS_PRIV(v->domain) )
@@ -1032,30 +1094,35 @@ long do_mca(XEN_GUEST_HANDLE(xen_mc_t) u
                flags = XEN_MC_OK;
 
                if (cmdflags & XEN_MC_ACK) {
-                       mctelem_cookie_t cookie = ID2COOKIE(mc_fetch->fetch_id);
+                       mctelem_cookie_t cookie = 
ID2COOKIE(mc_fetch.nat->fetch_id);
                        mctelem_ack(which, cookie);
                } else {
-                       if (guest_handle_is_null(mc_fetch->data))
+                       if (!is_pv_32on64_vcpu(v)
+                           ? guest_handle_is_null(mc_fetch.nat->data)
+                           : compat_handle_is_null(mc_fetch.cmp->data))
                                return x86_mcerr("do_mca fetch: guest buffer "
                                    "invalid", -EINVAL);
 
                        if ((mctc = mctelem_consume_oldest_begin(which))) {
                                struct mc_info *mcip = mctelem_dataptr(mctc);
-                               if (copy_to_guest(mc_fetch->data, mcip, 1)) {
+                               if (!is_pv_32on64_vcpu(v)
+                                   ? copy_to_guest(mc_fetch.nat->data, mcip, 1)
+                                   : copy_to_compat(mc_fetch.cmp->data,
+                                                    mcip, 1)) {
                                        ret = -EFAULT;
                                        flags |= XEN_MC_FETCHFAILED;
-                                       mc_fetch->fetch_id = 0;
+                                       mc_fetch.nat->fetch_id = 0;
                                } else {
-                                       mc_fetch->fetch_id = COOKIE2ID(mctc);
+                                       mc_fetch.nat->fetch_id = 
COOKIE2ID(mctc);
                                }
                                mctelem_consume_oldest_end(mctc);
                        } else {
                                /* There is no data */
                                flags |= XEN_MC_NODATA;
-                               mc_fetch->fetch_id = 0;
+                               mc_fetch.nat->fetch_id = 0;
                        }
 
-                       mc_fetch->flags = flags;
+                       mc_fetch.nat->flags = flags;
                        if (copy_to_guest(u_xen_mc, op, 1) != 0)
                                ret = -EFAULT;
                }
@@ -1069,14 +1136,16 @@ long do_mca(XEN_GUEST_HANDLE(xen_mc_t) u
                if ( !IS_PRIV(v->domain) )
                        return x86_mcerr("do_mca cpuinfo", -EPERM);
 
-               mc_physcpuinfo = &op->u.mc_physcpuinfo;
+               mc_physcpuinfo.nat = &op->u.mc_physcpuinfo;
                nlcpu = num_online_cpus();
 
-               if (!guest_handle_is_null(mc_physcpuinfo->info)) {
-                       if (mc_physcpuinfo->ncpus <= 0)
+               if (!is_pv_32on64_vcpu(v)
+                   ? !guest_handle_is_null(mc_physcpuinfo.nat->info)
+                   : !compat_handle_is_null(mc_physcpuinfo.cmp->info)) {
+                       if (mc_physcpuinfo.nat->ncpus <= 0)
                                return x86_mcerr("do_mca cpuinfo: ncpus <= 0",
                                    -EINVAL);
-                       nlcpu = min(nlcpu, (int)mc_physcpuinfo->ncpus);
+                       nlcpu = min(nlcpu, (int)mc_physcpuinfo.nat->ncpus);
                        log_cpus = xmalloc_array(xen_mc_logical_cpu_t, nlcpu);
                        if (log_cpus == NULL)
                                return x86_mcerr("do_mca cpuinfo", -ENOMEM);
@@ -1086,22 +1155,20 @@ long do_mca(XEN_GUEST_HANDLE(xen_mc_t) u
                                xfree(log_cpus);
                                return x86_mcerr("do_mca cpuinfo", -EIO);
                        }
-               }
-
-               mc_physcpuinfo->ncpus = nlcpu;
-
-               if (copy_to_guest(u_xen_mc, op, 1)) {
-                       if (log_cpus != NULL)
-                               xfree(log_cpus);
-                       return x86_mcerr("do_mca cpuinfo", -EFAULT);
-               }
-
-               if (!guest_handle_is_null(mc_physcpuinfo->info)) {
-                       if (copy_to_guest(mc_physcpuinfo->info,
-                           log_cpus, nlcpu))
+                       if (!is_pv_32on64_vcpu(v)
+                           ? copy_to_guest(mc_physcpuinfo.nat->info,
+                                           log_cpus, nlcpu)
+                           : copy_to_compat(mc_physcpuinfo.cmp->info,
+                                           log_cpus, nlcpu))
                                ret = -EFAULT;
                        xfree(log_cpus);
                }
+
+               mc_physcpuinfo.nat->ncpus = nlcpu;
+
+               if (copy_to_guest(u_xen_mc, op, 1))
+                       return x86_mcerr("do_mca cpuinfo", -EFAULT);
+
                break;
 
        case XEN_MC_msrinject:
diff -r f2cf89a4e762 -r 37f67d8224b7 xen/arch/x86/cpu/mcheck/x86_mca.h
--- a/xen/arch/x86/cpu/mcheck/x86_mca.h Tue Mar 31 12:06:57 2009 +0100
+++ b/xen/arch/x86/cpu/mcheck/x86_mca.h Tue Mar 31 13:11:56 2009 +0100
@@ -18,9 +18,9 @@
  */
 
 #ifndef X86_MCA_H
-
 #define X86_MCA_H
 
+#include <public/arch-x86/xen-mca.h>
 
 /* The MCA/MCE MSRs should not be used anywhere else.
  * They are cpu family/model specific and are only for use
diff -r f2cf89a4e762 -r 37f67d8224b7 xen/include/public/arch-x86/xen-mca.h
--- a/xen/include/public/arch-x86/xen-mca.h     Tue Mar 31 12:06:57 2009 +0100
+++ b/xen/include/public/arch-x86/xen-mca.h     Tue Mar 31 13:11:56 2009 +0100
@@ -62,7 +62,7 @@
  * choose a different version number range that is numerically less
  * than that used in xen-unstable.
  */
-#define XEN_MCA_INTERFACE_VERSION 0x01ecc002
+#define XEN_MCA_INTERFACE_VERSION 0x01ecc003
 
 /* IN: Dom0 calls hypercall to retrieve nonurgent telemetry */
 #define XEN_MC_NONURGENT  0x0001
@@ -125,13 +125,13 @@ struct mcinfo_global {
 
     /* running domain at the time in error (most likely the impacted one) */
     uint16_t mc_domid;
+    uint16_t mc_vcpuid; /* virtual cpu scheduled for mc_domid */
     uint32_t mc_socketid; /* physical socket of the physical core */
     uint16_t mc_coreid; /* physical impacted core */
+    uint16_t mc_core_threadid; /* core thread of physical core */
     uint32_t mc_apicid;
-    uint16_t mc_core_threadid; /* core thread of physical core */
-    uint16_t mc_vcpuid; /* virtual cpu scheduled for mc_domid */
+    uint32_t mc_flags;
     uint64_t mc_gstatus; /* global status */
-    uint32_t mc_flags;
 };
 
 /* contains bank local x86 mc information */
@@ -216,8 +216,9 @@ struct cpu_offline_action
 };
 
 #define MAX_UNION_SIZE 16
-struct mc_recovery
+struct mcinfo_recovery
 {
+    struct mcinfo_common common;
     uint16_t mc_bank; /* bank nr */
     uint8_t action_flags;
     uint8_t action_types;
@@ -228,12 +229,6 @@ struct mc_recovery
     } action_info;
 };
 
-struct mcinfo_recovery
-{
-    struct mcinfo_common common;
-    struct mc_recovery mc_action;
-};
-
 
 #define MCINFO_HYPERCALLSIZE   1024
 #define MCINFO_MAXSIZE         768
@@ -241,8 +236,8 @@ struct mc_info {
 struct mc_info {
     /* Number of mcinfo_* entries in mi_data */
     uint32_t mi_nentries;
-
-    uint8_t mi_data[MCINFO_MAXSIZE - sizeof(uint32_t)];
+    uint32_t _pad0;
+    uint64_t mi_data[(MCINFO_MAXSIZE - 1) / 8];
 };
 typedef struct mc_info mc_info_t;
 DEFINE_XEN_GUEST_HANDLE(mc_info_t);
@@ -258,7 +253,7 @@ DEFINE_XEN_GUEST_HANDLE(mc_info_t);
 #define MC_CAPS_VIA    5       /* cpuid level 0xc0000001 */
 #define MC_CAPS_AMD_ECX        6       /* cpuid level 0x80000001 (%ecx) */
 
-typedef struct mcinfo_logical_cpu {
+struct mcinfo_logical_cpu {
     uint32_t mc_cpunr;          
     uint32_t mc_chipid; 
     uint16_t mc_coreid;
@@ -280,7 +275,8 @@ typedef struct mcinfo_logical_cpu {
     uint32_t mc_cache_alignment;
     int32_t mc_nmsrvals;
     struct mcinfo_msr mc_msrvalues[__MC_MSR_ARRAYSIZE];
-} xen_mc_logical_cpu_t;
+};
+typedef struct mcinfo_logical_cpu xen_mc_logical_cpu_t;
 DEFINE_XEN_GUEST_HANDLE(xen_mc_logical_cpu_t);
 
 
@@ -299,12 +295,12 @@ DEFINE_XEN_GUEST_HANDLE(xen_mc_logical_c
  *    struct mcinfo_common *x86_mcinfo_first(struct mc_info *mi);
  */
 #define x86_mcinfo_first(_mi)       \
-    (struct mcinfo_common *)((_mi)->mi_data)
+    ((struct mcinfo_common *)(_mi)->mi_data)
 /* Prototype:
  *    struct mcinfo_common *x86_mcinfo_next(struct mcinfo_common *mic);
  */
 #define x86_mcinfo_next(_mic)       \
-    (struct mcinfo_common *)((uint8_t *)(_mic) + (_mic)->size)
+    ((struct mcinfo_common *)((uint8_t *)(_mic) + (_mic)->size))
 
 /* Prototype:
  *    void x86_mcinfo_lookup(void *ret, struct mc_info *mi, uint16_t type);
@@ -350,6 +346,7 @@ struct xen_mc_fetch {
                            XEN_MC_ACK if ack'ing an earlier fetch */
                        /* OUT: XEN_MC_OK, XEN_MC_FETCHFAILED,
                           XEN_MC_NODATA, XEN_MC_NOMATCH */
+    uint32_t _pad0;
     uint64_t fetch_id; /* OUT: id for ack, IN: id we are ack'ing */
 
     /* OUT variables. */
@@ -382,7 +379,7 @@ struct xen_mc_physcpuinfo {
 struct xen_mc_physcpuinfo {
        /* IN/OUT */
        uint32_t ncpus;
-       uint32_t pad0;
+       uint32_t _pad0;
        /* OUT */
        XEN_GUEST_HANDLE(xen_mc_logical_cpu_t) info;
 };
@@ -391,10 +388,10 @@ struct xen_mc_physcpuinfo {
 #define MC_MSRINJ_MAXMSRS       8
 struct xen_mc_msrinject {
        /* IN */
-       unsigned int mcinj_cpunr;       /* target processor id */
+       uint32_t mcinj_cpunr;           /* target processor id */
        uint32_t mcinj_flags;           /* see MC_MSRINJ_F_* below */
        uint32_t mcinj_count;           /* 0 .. count-1 in array are valid */
-       uint32_t mcinj_pad0;
+       uint32_t _pad0;
        struct mcinfo_msr mcinj_msr[MC_MSRINJ_MAXMSRS];
 };
 
@@ -406,18 +403,16 @@ struct xen_mc_mceinject {
        unsigned int mceinj_cpunr;      /* target processor id */
 };
 
-typedef union {
-    struct xen_mc_fetch        mc_fetch;
-    struct xen_mc_notifydomain mc_notifydomain;
-    struct xen_mc_physcpuinfo  mc_physcpuinfo;
-    struct xen_mc_msrinject    mc_msrinject;
-    struct xen_mc_mceinject    mc_mceinject;
-} xen_mc_arg_t;
-
 struct xen_mc {
     uint32_t cmd;
     uint32_t interface_version; /* XEN_MCA_INTERFACE_VERSION */
-    xen_mc_arg_t u;
+    union {
+        struct xen_mc_fetch        mc_fetch;
+        struct xen_mc_notifydomain mc_notifydomain;
+        struct xen_mc_physcpuinfo  mc_physcpuinfo;
+        struct xen_mc_msrinject    mc_msrinject;
+        struct xen_mc_mceinject    mc_mceinject;
+    } u;
 };
 typedef struct xen_mc xen_mc_t;
 DEFINE_XEN_GUEST_HANDLE(xen_mc_t);
diff -r f2cf89a4e762 -r 37f67d8224b7 xen/include/public/arch-x86/xen.h
--- a/xen/include/public/arch-x86/xen.h Tue Mar 31 12:06:57 2009 +0100
+++ b/xen/include/public/arch-x86/xen.h Tue Mar 31 13:11:56 2009 +0100
@@ -75,10 +75,6 @@ typedef unsigned long xen_pfn_t;
 
 /* Maximum number of virtual CPUs in multi-processor guests. */
 #define MAX_VIRT_CPUS 32
-
-
-/* Machine check support */
-#include "xen-mca.h"
 
 #ifndef __ASSEMBLY__
 
diff -r f2cf89a4e762 -r 37f67d8224b7 xen/include/xlat.lst
--- a/xen/include/xlat.lst      Tue Mar 31 12:06:57 2009 +0100
+++ b/xen/include/xlat.lst      Tue Mar 31 13:11:56 2009 +0100
@@ -10,6 +10,22 @@
 !      cpu_user_regs                   arch-x86/xen-@arch@.h
 !      trap_info                       arch-x86/xen.h
 !      vcpu_guest_context              arch-x86/xen.h
+?      cpu_offline_action              arch-x86/xen-mca.h
+?      mc                              arch-x86/xen-mca.h
+?      mcinfo_bank                     arch-x86/xen-mca.h
+?      mcinfo_common                   arch-x86/xen-mca.h
+?      mcinfo_extended                 arch-x86/xen-mca.h
+?      mcinfo_global                   arch-x86/xen-mca.h
+?      mcinfo_logical_cpu              arch-x86/xen-mca.h
+?      mcinfo_msr                      arch-x86/xen-mca.h
+?      mcinfo_recovery                 arch-x86/xen-mca.h
+!      mc_fetch                        arch-x86/xen-mca.h
+?      mc_info                         arch-x86/xen-mca.h
+?      mc_mceinject                    arch-x86/xen-mca.h
+?      mc_msrinject                    arch-x86/xen-mca.h
+?      mc_notifydomain                 arch-x86/xen-mca.h
+!      mc_physcpuinfo                  arch-x86/xen-mca.h
+?      page_offline_action             arch-x86/xen-mca.h
 ?      evtchn_alloc_unbound            event_channel.h
 ?      evtchn_bind_interdomain         event_channel.h
 ?      evtchn_bind_ipi                 event_channel.h
diff -r f2cf89a4e762 -r 37f67d8224b7 xen/tools/get-fields.sh
--- a/xen/tools/get-fields.sh   Tue Mar 31 12:06:57 2009 +0100
+++ b/xen/tools/get-fields.sh   Tue Mar 31 13:11:56 2009 +0100
@@ -328,7 +328,7 @@ check_field ()
                                struct|union)
                                        ;;
                                [a-zA-Z_]*)
-                                       echo -n "    CHECK_$n"
+                                       echo -n "    CHECK_${n#xen_}"
                                        break
                                        ;;
                                *)

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog


 


Rackspace

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