From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: [PATCH] fix c/s 17968 Date: Fri, 27 Mar 2009 16:32:27 +0000 Message-ID: <49CD0DAB.76E4.0078.0@novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: xen-devel@lists.xensource.com List-Id: xen-devel@lists.xenproject.org 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 --- 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.0000000= 00 +0100 @@ -984,14 +984,76 @@ static void x86_mc_mceinject(void *data) #error BITS_PER_LONG definition absent #endif =20 +#ifdef CONFIG_COMPAT +# include + +# 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 =3D 0; struct xen_mc curop, *op =3D &curop; struct vcpu *v =3D 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 =3D NULL; @@ -1009,8 +1071,8 @@ long do_mca(XEN_GUEST_HANDLE(xen_mc_t) u =20 switch (op->cmd) { case XEN_MC_fetch: - mc_fetch =3D &op->u.mc_fetch; - cmdflags =3D mc_fetch->flags; + mc_fetch.nat =3D &op->u.mc_fetch; + cmdflags =3D mc_fetch.nat->flags; =20 /* 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 =3D XEN_MC_OK; =20 if (cmdflags & XEN_MC_ACK) { - mctelem_cookie_t cookie =3D ID2COOKIE(mc_fetch->fet= ch_id); + mctelem_cookie_t cookie =3D 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); =20 if ((mctc =3D mctelem_consume_oldest_begin(which)))= { struct mc_info *mcip =3D mctelem_dataptr(mc= tc); - 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 =3D -EFAULT; flags |=3D XEN_MC_FETCHFAILED; - mc_fetch->fetch_id =3D 0; + mc_fetch.nat->fetch_id =3D 0; } else { - mc_fetch->fetch_id =3D COOKIE2ID(mc= tc); + mc_fetch.nat->fetch_id =3D = COOKIE2ID(mctc); } mctelem_consume_oldest_end(mctc); } else { /* There is no data */ flags |=3D XEN_MC_NODATA; - mc_fetch->fetch_id =3D 0; + mc_fetch.nat->fetch_id =3D 0; } =20 - mc_fetch->flags =3D flags; + mc_fetch.nat->flags =3D flags; if (copy_to_guest(u_xen_mc, op, 1) !=3D 0) ret =3D -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); =20 - mc_physcpuinfo =3D &op->u.mc_physcpuinfo; + mc_physcpuinfo.nat =3D &op->u.mc_physcpuinfo; nlcpu =3D num_online_cpus(); =20 - if (!guest_handle_is_null(mc_physcpuinfo->info)) { - if (mc_physcpuinfo->ncpus <=3D 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 <=3D 0) return x86_mcerr("do_mca cpuinfo: ncpus = <=3D 0", -EINVAL); - nlcpu =3D min(nlcpu, (int)mc_physcpuinfo->ncpus); log_cpus =3D xmalloc_array(xen_mc_logical_cpu_t, = nlcpu); if (log_cpus =3D=3D NULL) return x86_mcerr("do_mca cpuinfo", = -ENOMEM); =20 + nlcpu =3D min(nlcpu, (int)mc_physcpuinfo.nat->ncpus= ); if (on_each_cpu(do_mc_get_cpu_info, log_cpus, 1, 1) !=3D 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 =3D -EFAULT; + xfree(log_cpus); } =20 - mc_physcpuinfo->ncpus =3D nlcpu; + mc_physcpuinfo.nat->ncpus =3D nlcpu; =20 - if (copy_to_guest(u_xen_mc, op, 1)) { - if (log_cpus !=3D NULL) - xfree(log_cpus); + if (copy_to_guest(u_xen_mc, op, 1)) return x86_mcerr("do_mca cpuinfo", -EFAULT); - } =20 - if (!guest_handle_is_null(mc_physcpuinfo->info)) { - if (copy_to_guest(mc_physcpuinfo->info, - log_cpus, nlcpu)) - ret =3D -EFAULT; - xfree(log_cpus); - } break; =20 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 @@ */ =20 #ifndef X86_MCA_H - #define X86_MCA_H =20 +#include =20 /* 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 =20 - -/* Machine check support */ -#include "xen-mca.h" - #ifndef __ASSEMBLY__ =20 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 =20 /* IN: Dom0 calls hypercall to retrieve nonurgent telemetry */ #define XEN_MC_NONURGENT 0x0001 @@ -125,13 +125,13 @@ struct mcinfo_global { =20 /* 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 */ }; =20 /* contains bank local x86 mc information */ @@ -216,8 +216,9 @@ struct cpu_offline_action }; =20 #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; }; =20 -struct mcinfo_recovery -{ - struct mcinfo_common common; - struct mc_recovery mc_action; -}; - =20 #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) = */ =20 -typedef struct mcinfo_logical_cpu { +struct mcinfo_logical_cpu { uint32_t mc_cpunr; =20 uint32_t mc_chipid;=20 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); =20 =20 @@ -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)) =20 /* 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 */ =20 /* 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]; }; =20 @@ -406,18 +403,16 @@ struct xen_mc_mceinject { unsigned int mceinj_cpunr; /* target processor id */ }; =20 -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.0000000= 00 +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=20 ! 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.0000000= 00 +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 ;; *)