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

[Xen-devel] [PATCH] fix c/s 17968



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>

--- 2009-03-27.orig/xen/arch/x86/cpu/mcheck/mce.c       2009-03-27 
16:50:39.000000000 +0100
+++ 2009-03-27/xen/arch/x86/cpu/mcheck/mce.c    2009-03-27 16:50:58.000000000 
+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,39 +1136,39 @@ 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);
                        log_cpus = xmalloc_array(xen_mc_logical_cpu_t, nlcpu);
                        if (log_cpus == NULL)
                                return x86_mcerr("do_mca cpuinfo", -ENOMEM);
 
+                       nlcpu = min(nlcpu, (int)mc_physcpuinfo.nat->ncpus);
                        if (on_each_cpu(do_mc_get_cpu_info, log_cpus,
                            1, 1) != 0) {
                                xfree(log_cpus);
                                return x86_mcerr("do_mca cpuinfo", -EIO);
                        }
+                       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->ncpus = nlcpu;
+               mc_physcpuinfo.nat->ncpus = nlcpu;
 
-               if (copy_to_guest(u_xen_mc, op, 1)) {
-                       if (log_cpus != NULL)
-                               xfree(log_cpus);
+               if (copy_to_guest(u_xen_mc, op, 1))
                        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))
-                               ret = -EFAULT;
-                       xfree(log_cpus);
-               }
                break;
 
        case XEN_MC_msrinject:
--- 2009-03-27.orig/xen/arch/x86/cpu/mcheck/x86_mca.h   2009-03-27 
16:50:39.000000000 +0100
+++ 2009-03-27/xen/arch/x86/cpu/mcheck/x86_mca.h        2009-03-27 
16:40:50.000000000 +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
--- 2009-03-27.orig/xen/include/public/arch-x86/xen.h   2009-03-27 
16:50:39.000000000 +0100
+++ 2009-03-27/xen/include/public/arch-x86/xen.h        2009-03-27 
16:37:22.000000000 +0100
@@ -76,10 +76,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__
 
 typedef unsigned long xen_ulong_t;
--- 2009-03-27.orig/xen/include/public/arch-x86/xen-mca.h       2009-03-27 
16:50:39.000000000 +0100
+++ 2009-03-27/xen/include/public/arch-x86/xen-mca.h    2009-03-27 
16:47:30.000000000 +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 */
-    uint32_t mc_apicid;
     uint16_t mc_core_threadid; /* core thread of physical core */
-    uint16_t mc_vcpuid; /* virtual cpu scheduled for mc_domid */
-    uint64_t mc_gstatus; /* global status */
+    uint32_t mc_apicid;
     uint32_t mc_flags;
+    uint64_t mc_gstatus; /* global status */
 };
 
 /* 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 mcinfo_recovery
 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 @@ DEFINE_XEN_GUEST_HANDLE(xen_mc_notifydom
 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);
--- 2009-03-27.orig/xen/include/xlat.lst        2009-03-27 16:50:39.000000000 
+0100
+++ 2009-03-27/xen/include/xlat.lst     2009-03-27 16:47:51.000000000 +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
--- 2009-03-27.orig/xen/tools/get-fields.sh     2009-03-27 16:50:39.000000000 
+0100
+++ 2009-03-27/xen/tools/get-fields.sh  2009-03-27 14:28:29.000000000 +0100
@@ -328,7 +328,7 @@ check_field ()
                                struct|union)
                                        ;;
                                [a-zA-Z_]*)
-                                       echo -n "    CHECK_$n"
+                                       echo -n "    CHECK_${n#xen_}"
                                        break
                                        ;;
                                *)



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


 


Rackspace

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