[Xen-changelog] [xen master] mce: fix race condition in mctelem_xchg_head

[patchbot@xxxxxxx]

commit e9af61b969906976188609379183cb304935f448
Author:     Frediano Ziglio <frediano.ziglio@xxxxxxxxxx>
AuthorDate: Fri Jan 17 15:58:27 2014 +0100
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Fri Jan 17 15:58:27 2014 +0100

    mce: fix race condition in mctelem_xchg_head
    
    The function (mctelem_xchg_head()) used to exchange mce telemetry
    list heads is racy.  It may write to the head twice, with the second
    write linking to an element in the wrong state.
    
    If there are two threads, T1 inserting on committed list; and T2
    trying to consume it.
    
    1. T1 starts inserting an element (A), sets prev pointer (mcte_prev).
    2. T1 is interrupted after the cmpxchg succeeded.
    3. T2 gets the list and changes element A and updates the commit list
       head.
    4. T1 resumes, reads pointer to prev again and compare with result
       from the cmpxchg which succeeded but in the meantime prev changed
       in memory.
    5. T1 thinks the cmpxchg failed and goes around the loop again,
       linking head to A again.
    
    To solve the race use temporary variable for prev pointer.
    
    *linkp (which point to a field in the element) must be updated before
    the cmpxchg() as after a successful cmpxchg the element might be
    immediately removed and reinitialized.
    
    The wmb() prior to the cmpchgptr() call is not necessary since it is
    already a full memory barrier.  This wmb() is thus removed.
    
    Signed-off-by: Frediano Ziglio <frediano.ziglio@xxxxxxxxxx>
    Reviewed-by: Liu Jinsong <jinsong.liu@xxxxxxxxx>
---
 xen/arch/x86/cpu/mcheck/mctelem.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/xen/arch/x86/cpu/mcheck/mctelem.c 
b/xen/arch/x86/cpu/mcheck/mctelem.c
index 37d830f..895ce1a 100644
--- a/xen/arch/x86/cpu/mcheck/mctelem.c
+++ b/xen/arch/x86/cpu/mcheck/mctelem.c
@@ -127,13 +127,14 @@ static DEFINE_PER_CPU(struct mc_telem_cpu_ctl, mctctl);
 static DEFINE_SPINLOCK(processing_lock);
 
 static void mctelem_xchg_head(struct mctelem_ent **headp,
-                               struct mctelem_ent **old,
+                               struct mctelem_ent **linkp,
                                struct mctelem_ent *new)
 {
        for (;;) {
-               *old = *headp;
-               wmb();
-               if (cmpxchgptr(headp, *old, new) == *old)
+               struct mctelem_ent *old;
+
+               *linkp = old = *headp;
+               if (cmpxchgptr(headp, old, new) == old)
                        break;
        }
 }
--
generated by git-patchbot for /home/xen/git/xen.git#master

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