From: Eric Sandeen <sandeen@redhat.com>
Subject: [RHEL5.1 PATCH] - buffer: memorder fix
Date: Wed, 21 Feb 2007 16:09:08 -0600
Bugzilla: 225172
Message-Id: <45DCC304.8050901@redhat.com>
Changelog: [ext3] buffer: memorder fix


This is for BZ 225172: LTC31365-ext3 error: bit already cleared for block, aborted journal, remount read-only

Lack of a memory barrier in unlock_buffer() was leading to a race where 2 processes would
both try to free a shared extended attribute block upon file deletion.

I'd almost have argued for this one-liner to be in an errata kernel, but no-one 
else has found it in testing, so... it's probably still debatable.

Please ack,

Thanks,

-Eric

--------- patch follows ------------

X-Git-Tag: v2.6.21-rc1^0~274^2~265
X-Git-Url: http://git.kernel.org/git/?p=linux%2Fkernel%2Fgit%2Ftorvalds%2Flinux-2.6.git;a=commitdiff_plain;h=72ed3d035855841ad611ee48b20909e9619d4a79

[PATCH] buffer: memorder fix

unlock_buffer(), like unlock_page(), must not clear the lock without
ensuring that the critical section is closed.

Mingming later sent the same patch, saying:

  We are running SDET benchmark and saw double free issue for ext3 extended
  attributes block, which complains the same xattr block already being freed (in
  ext3_xattr_release_block()).  The problem could also been triggered by
  multiple threads loop untar/rm a kernel tree.

  The race is caused by missing a memory barrier at unlock_buffer() before the
  lock bit being cleared, resulting in possible concurrent h_refcounter update.
  That causes a reference counter leak, then later leads to the double free that
  we have seen.

  Inside unlock_buffer(), there is a memory barrier is placed *after* the lock
  bit is being cleared, however, there is no memory barrier *before* the bit is
  cleared.  On some arch the h_refcount update instruction and the clear bit
  instruction could be reordered, thus leave the critical section re-entered.

  The race is like this: For example, if the h_refcount is initialized as 1,

  cpu 0:                                   cpu1
  --------------------------------------   -----------------------------------
  lock_buffer() /* test_and_set_bit */
  clear_buffer_locked(bh);
                                          lock_buffer() /* test_and_set_bit */
  h_refcount = h_refcount+1; /* = 2*/     h_refcount = h_refcount + 1; /*= 2 */
                                          clear_buffer_locked(bh);
  ....                                    ......

  We lost a h_refcount here. We need a memory barrier before the buffer head lock
  bit being cleared to force the order of the two writes.  Please apply.

Signed-off-by: Nick Piggin <npiggin@suse.de>
Signed-off-by: Mingming Cao <cmm@us.ibm.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---

Index: linux-2.6.18-6.el5/fs/buffer.c
===================================================================
--- linux-2.6.18-6.el5.orig/fs/buffer.c
+++ linux-2.6.18-6.el5/fs/buffer.c
@@ -77,6 +77,7 @@ EXPORT_SYMBOL(__lock_buffer);
 
 void fastcall unlock_buffer(struct buffer_head *bh)
 {
+	smp_mb__before_clear_bit();
 	clear_buffer_locked(bh);
 	smp_mb__after_clear_bit();
 	wake_up_bit(&bh->b_state, BH_Lock);