xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86/vMCE: MC{G,i}_CTL handling adjustments
@ 2012-02-13 11:24 Jan Beulich
  2012-02-13 19:52 ` Keir Fraser
  0 siblings, 1 reply; 2+ messages in thread
From: Jan Beulich @ 2012-02-13 11:24 UTC (permalink / raw)
  To: xen-devel@lists.xensource.com

[-- Attachment #1: Type: text/plain, Size: 3110 bytes --]

- g_mcg_cap was read to determine whether MCG_CTL exists before it got
  initialized
- h_mci_ctrl[] and dom_vmce()->mci_ctl[] both got initialized via
  memset() with an inappropriate size (hence causing a [minor?]
  information leak)

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -29,7 +29,7 @@ invbool_param("mce", mce_disabled);
 bool_t __read_mostly mce_broadcast = 0;
 bool_t is_mc_panic;
 unsigned int __read_mostly nr_mce_banks;
-int __read_mostly firstbank;
+unsigned int __read_mostly firstbank;
 
 static void intpose_init(void);
 static void mcinfo_clear(struct mc_info *);
@@ -650,7 +650,7 @@ int mce_available(struct cpuinfo_x86 *c)
  * Check if bank 0 is usable for MCE. It isn't for AMD K7,
  * and Intel P6 family before model 0x1a.
  */
-int mce_firstbank(struct cpuinfo_x86 *c)
+unsigned int mce_firstbank(struct cpuinfo_x86 *c)
 {
     if (c->x86 == 6) {
         if (c->x86_vendor == X86_VENDOR_AMD)
--- a/xen/arch/x86/cpu/mcheck/mce.h
+++ b/xen/arch/x86/cpu/mcheck/mce.h
@@ -52,7 +52,7 @@ int is_vmce_ready(struct mcinfo_bank *ba
 int unmmap_broken_page(struct domain *d, mfn_t mfn, unsigned long gfn);
 
 u64 mce_cap_init(void);
-extern int firstbank;
+extern unsigned int firstbank;
 
 int intel_mce_rdmsr(uint32_t msr, uint64_t *val);
 int intel_mce_wrmsr(uint32_t msr, uint64_t val);
@@ -61,7 +61,7 @@ struct mcinfo_extended *intel_get_extend
     struct mcinfo_global *mig, struct mc_info *mi);
 
 int mce_available(struct cpuinfo_x86 *c);
-int mce_firstbank(struct cpuinfo_x86 *c);
+unsigned int mce_firstbank(struct cpuinfo_x86 *c);
 /* Helper functions used for collecting error telemetry */
 struct mc_info *x86_mcinfo_getptr(void);
 void mc_panic(char *s);
--- a/xen/arch/x86/cpu/mcheck/vmce.c
+++ b/xen/arch/x86/cpu/mcheck/vmce.c
@@ -39,7 +39,7 @@ int vmce_init_msr(struct domain *d)
         return -ENOMEM;
     }
     memset(dom_vmce(d)->mci_ctl, ~0,
-           sizeof(dom_vmce(d)->mci_ctl));
+           nr_mce_banks * sizeof(*dom_vmce(d)->mci_ctl));
 
     dom_vmce(d)->mcg_status = 0x0;
     dom_vmce(d)->mcg_cap = g_mcg_cap;
@@ -438,7 +438,7 @@ int vmce_domain_inject(
 int vmce_init(struct cpuinfo_x86 *c)
 {
     u64 value;
-    int i;
+    unsigned int i;
 
     if ( !h_mci_ctrl )
     {
@@ -449,17 +449,17 @@ int vmce_init(struct cpuinfo_x86 *c)
             return -ENOMEM;
         }
         /* Don't care banks before firstbank */
-        memset(h_mci_ctrl, 0xff, sizeof(h_mci_ctrl));
+        memset(h_mci_ctrl, ~0,
+               min(firstbank, nr_mce_banks) * sizeof(*h_mci_ctrl));
         for (i = firstbank; i < nr_mce_banks; i++)
             rdmsrl(MSR_IA32_MCx_CTL(i), h_mci_ctrl[i]);
     }
 
-    if (g_mcg_cap & MCG_CTL_P)
-        rdmsrl(MSR_IA32_MCG_CTL, h_mcg_ctl);
-
     rdmsrl(MSR_IA32_MCG_CAP, value);
     /* For Guest vMCE usage */
     g_mcg_cap = value & ~MCG_CMCI_P;
+    if (value & MCG_CTL_P)
+        rdmsrl(MSR_IA32_MCG_CTL, h_mcg_ctl);
 
     return 0;
 }




[-- Attachment #2: x86-vmce-mcg_ctl.patch --]
[-- Type: text/plain, Size: 3150 bytes --]

x86/vMCE: MC{G,i}_CTL handling adjustments

- g_mcg_cap was read to determine whether MCG_CTL exists before it got
  initialized
- h_mci_ctrl[] and dom_vmce()->mci_ctl[] both got initialized via
  memset() with an inappropriate size (hence causing a [minor?]
  information leak)

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -29,7 +29,7 @@ invbool_param("mce", mce_disabled);
 bool_t __read_mostly mce_broadcast = 0;
 bool_t is_mc_panic;
 unsigned int __read_mostly nr_mce_banks;
-int __read_mostly firstbank;
+unsigned int __read_mostly firstbank;
 
 static void intpose_init(void);
 static void mcinfo_clear(struct mc_info *);
@@ -650,7 +650,7 @@ int mce_available(struct cpuinfo_x86 *c)
  * Check if bank 0 is usable for MCE. It isn't for AMD K7,
  * and Intel P6 family before model 0x1a.
  */
-int mce_firstbank(struct cpuinfo_x86 *c)
+unsigned int mce_firstbank(struct cpuinfo_x86 *c)
 {
     if (c->x86 == 6) {
         if (c->x86_vendor == X86_VENDOR_AMD)
--- a/xen/arch/x86/cpu/mcheck/mce.h
+++ b/xen/arch/x86/cpu/mcheck/mce.h
@@ -52,7 +52,7 @@ int is_vmce_ready(struct mcinfo_bank *ba
 int unmmap_broken_page(struct domain *d, mfn_t mfn, unsigned long gfn);
 
 u64 mce_cap_init(void);
-extern int firstbank;
+extern unsigned int firstbank;
 
 int intel_mce_rdmsr(uint32_t msr, uint64_t *val);
 int intel_mce_wrmsr(uint32_t msr, uint64_t val);
@@ -61,7 +61,7 @@ struct mcinfo_extended *intel_get_extend
     struct mcinfo_global *mig, struct mc_info *mi);
 
 int mce_available(struct cpuinfo_x86 *c);
-int mce_firstbank(struct cpuinfo_x86 *c);
+unsigned int mce_firstbank(struct cpuinfo_x86 *c);
 /* Helper functions used for collecting error telemetry */
 struct mc_info *x86_mcinfo_getptr(void);
 void mc_panic(char *s);
--- a/xen/arch/x86/cpu/mcheck/vmce.c
+++ b/xen/arch/x86/cpu/mcheck/vmce.c
@@ -39,7 +39,7 @@ int vmce_init_msr(struct domain *d)
         return -ENOMEM;
     }
     memset(dom_vmce(d)->mci_ctl, ~0,
-           sizeof(dom_vmce(d)->mci_ctl));
+           nr_mce_banks * sizeof(*dom_vmce(d)->mci_ctl));
 
     dom_vmce(d)->mcg_status = 0x0;
     dom_vmce(d)->mcg_cap = g_mcg_cap;
@@ -438,7 +438,7 @@ int vmce_domain_inject(
 int vmce_init(struct cpuinfo_x86 *c)
 {
     u64 value;
-    int i;
+    unsigned int i;
 
     if ( !h_mci_ctrl )
     {
@@ -449,17 +449,17 @@ int vmce_init(struct cpuinfo_x86 *c)
             return -ENOMEM;
         }
         /* Don't care banks before firstbank */
-        memset(h_mci_ctrl, 0xff, sizeof(h_mci_ctrl));
+        memset(h_mci_ctrl, ~0,
+               min(firstbank, nr_mce_banks) * sizeof(*h_mci_ctrl));
         for (i = firstbank; i < nr_mce_banks; i++)
             rdmsrl(MSR_IA32_MCx_CTL(i), h_mci_ctrl[i]);
     }
 
-    if (g_mcg_cap & MCG_CTL_P)
-        rdmsrl(MSR_IA32_MCG_CTL, h_mcg_ctl);
-
     rdmsrl(MSR_IA32_MCG_CAP, value);
     /* For Guest vMCE usage */
     g_mcg_cap = value & ~MCG_CMCI_P;
+    if (value & MCG_CTL_P)
+        rdmsrl(MSR_IA32_MCG_CTL, h_mcg_ctl);
 
     return 0;
 }

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2012-02-13 19:52 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-13 11:24 [PATCH] x86/vMCE: MC{G,i}_CTL handling adjustments Jan Beulich
2012-02-13 19:52 ` 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).