WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: [Xen-devel] [PATCH] fix c/s 17968
From: "Jan Beulich" <jbeulich@xxxxxxxxxx>
Date: Fri, 27 Mar 2009 16:32:27 +0000
Delivery-date: Fri, 27 Mar 2009 09:31:53 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
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

<Prev in Thread] Current Thread [Next in Thread>
  • [Xen-devel] [PATCH] fix c/s 17968, Jan Beulich <=