[PATCH] Missing brelse() in ext2/ext3 extended attribute code

From: Andreas Gruenbacher <agruen@suse.de>

Missing brelse() in ext2/ext3 extended attribute code

The ext2 and ext3 EA implementations fail to release a buffer_head if
the inode that is being accessed is sharing EAs with another inode, and
an attribute is set to the same value that it has already, like so:

        $ touch f g
	$ setfattr -n user.test -v test f g
	# (Now, both f and g refer to the same EA block.)
	$ setfattr -n user.test -v test f

With the bug, an "invalidate: busy buffer" or "invalidate: dirty
buffer" message will be logged when the file system is unmounted. This
patch fixes the problem.

At the implementation level:

The code was assuming that ext3_xattr_cache_find cannot return the same
block the inode already is associated with, so testing for (old_bh !=
new_bh) would determine whether the old block is resued or an additional
bh is held. This is wrong if the EA block is used by multiple inodes (in
which case it stays in the cache), and the block isn't actually
modified.  Instead of testing for (old_bh != new_bh), the code now does
a get_bh() in the branch that keeps the old block, which assures that
new_bh now is either NULL or a handle that must be released at the end
of ext3_xattr_set_handle2().

fs/ext2/xattr.c

@@ -732,7 +732,8 @@ ext2_xattr_set2(struct inode *inode, struct buffer_head *old_bh,
 			 * The old block will be released after updating
 			 * the inode.
 			 */
-			ea_bdebug(new_bh, "reusing block %ld",
+			ea_bdebug(new_bh, "%s block %ld",
+				(old_bh == new_bh) ? "keeping" : "reusing",
 				new_bh->b_blocknr);
 			
 			error = -EDQUOT;
@@ -746,6 +747,7 @@ ext2_xattr_set2(struct inode *inode, struct buffer_head *old_bh,
 		} else if (old_bh && header == HDR(old_bh)) {
 			/* Keep this block. */
 			new_bh = old_bh;
+			get_bh(new_bh);
 			ext2_xattr_cache_insert(new_bh);
 		} else {
 			/* We need to allocate a new block */
@@ -816,8 +818,7 @@ ext2_xattr_set2(struct inode *inode, struct buffer_head *old_bh,
 	}
 
 cleanup:
-	if (old_bh != new_bh)
-		brelse(new_bh);
+	brelse(new_bh);
 
 	return error;
 }

fs/ext3/xattr.c

@@ -733,7 +733,8 @@ ext3_xattr_set_handle2(handle_t *handle, struct inode *inode,
 			 * The old block will be released after updating
 			 * the inode.
 			 */
-			ea_bdebug(new_bh, "reusing block %ld",
+			ea_bdebug(new_bh, "%s block %ld",
+				(old_bh == new_bh) ? "keeping" : "reusing",
 				new_bh->b_blocknr);
 			
 			error = -EDQUOT;
@@ -750,6 +751,7 @@ ext3_xattr_set_handle2(handle_t *handle, struct inode *inode,
 		} else if (old_bh && header == HDR(old_bh)) {
 			/* Keep this block. */
 			new_bh = old_bh;
+			get_bh(new_bh);
 			ext3_xattr_cache_insert(new_bh);
 		} else {
 			/* We need to allocate a new block */
@@ -827,8 +829,7 @@ ext3_xattr_set_handle2(handle_t *handle, struct inode *inode,
 	}
 
 cleanup:
-	if (old_bh != new_bh)
-		brelse(new_bh);
+	brelse(new_bh);
 
 	return error;
 }