x86, mce: fix a race condition in mce_read()

Impact: bugfix

Considering the situation as follow:

before: mcelog.next == 1, mcelog.entry[0].finished = 1

+--------------------------------------------------------------------------
R                   W1                  W2                  W3

read mcelog.next (1)
                    mcelog.next++ (2)
                    (working on entry 1,
                    finished == 0)

mcelog.next = 0
                                        mcelog.next++ (1)
                                        (working on entry 0)
                                                           mcelog.next++ (2)
                                                           (working on entry 1)
                        <----------------- race ---------------->
                    (done on entry 1,
                    finished = 1)
                                                           (done on entry 1,
                                                           finished = 1)

To fix the race condition, a cmpxchg loop is added to mce_read() to
ensure no new MCE record can be added between mcelog.next reading and
mcelog.next = 0.

Signed-off-by: Huang Ying <ying.huang@intel.com>
Signed-off-by: Andi Kleen <ak@linux.intel.com>
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: H. Peter Anvin <hpa@zytor.com>

diff --git a/arch/x86/kernel/cpu/mcheck/mce_64.c b/arch/x86/kernel/cpu/mcheck/mce_64.c
index 1db94c0d5aafb8115acca96bd99051d203de55f7..870d08deccf78a5454142755f02175d7acc66b3c 100644 (file)
--- a/arch/x86/kernel/cpu/mcheck/mce_64.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_64.c
@@ -595,7 +595,7 @@ static ssize_t mce_read(struct file *filp, char __user *ubuf, size_t usize,
 {
        unsigned long *cpu_tsc;
        static DEFINE_MUTEX(mce_read_mutex);
-       unsigned next;
+       unsigned prev, next;
        char __user *buf = ubuf;
        int i, err;
 
@@ -614,25 +614,32 @@ static ssize_t mce_read(struct file *filp, char __user *ubuf, size_t usize,
        }
 
        err = 0;
-       for (i = 0; i < next; i++) {
-               unsigned long start = jiffies;
-
-               while (!mcelog.entry[i].finished) {
-                       if (time_after_eq(jiffies, start + 2)) {
-                               memset(mcelog.entry + i,0, sizeof(struct mce));
-                               goto timeout;
+       prev = 0;
+       do {
+               for (i = prev; i < next; i++) {
+                       unsigned long start = jiffies;
+
+                       while (!mcelog.entry[i].finished) {
+                               if (time_after_eq(jiffies, start + 2)) {
+                                       memset(mcelog.entry + i, 0,
+                                              sizeof(struct mce));
+                                       goto timeout;
+                               }
+                               cpu_relax();
                        }
-                       cpu_relax();
+                       smp_rmb();
+                       err |= copy_to_user(buf, mcelog.entry + i,
+                                           sizeof(struct mce));
+                       buf += sizeof(struct mce);
+timeout:
+                       ;
                }
-               smp_rmb();
-               err |= copy_to_user(buf, mcelog.entry + i, sizeof(struct mce));
-               buf += sizeof(struct mce);
- timeout:
-               ;
-       }
 
-       memset(mcelog.entry, 0, next * sizeof(struct mce));
-       mcelog.next = 0;
+               memset(mcelog.entry + prev, 0,
+                      (next - prev) * sizeof(struct mce));
+               prev = next;
+               next = cmpxchg(&mcelog.next, prev, 0);
+       } while (next != prev);
 
        synchronize_sched();