lkml.org 
[lkml]   [2003]   [Apr]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH] Missing brelse() in ext2/ext3 extended attribute code (2.5.66)
Date
Hello Andrew,

here is a fix for a bug that became visible by the recent fix to
fs/mbcache.c:mb_cache_entry_insert(). An explanation is in the attachment.

Could you please include that in your next update to Linus?


Thanks,
Andreas.
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().

Andreas Gruenbacher <agruen@suse.de>


Index: linux-2.5.66/fs/ext2/xattr.c
===================================================================
--- linux-2.5.66.orig/fs/ext2/xattr.c 2003-03-24 23:00:50.000000000 +0100
+++ linux-2.5.66/fs/ext2/xattr.c 2003-04-06 04:16:59.000000000 +0200
@@ -732,7 +732,8 @@
* 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 @@
} 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 @@
}

cleanup:
- if (old_bh != new_bh)
- brelse(new_bh);
+ brelse(new_bh);

return error;
}
Index: linux-2.5.66/fs/ext3/xattr.c
===================================================================
--- linux-2.5.66.orig/fs/ext3/xattr.c 2003-03-24 23:00:46.000000000 +0100
+++ linux-2.5.66/fs/ext3/xattr.c 2003-04-06 04:17:31.000000000 +0200
@@ -733,7 +733,8 @@
* 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 @@
} 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 @@
}

cleanup:
- if (old_bh != new_bh)
- brelse(new_bh);
+ brelse(new_bh);

return error;
}
\
 
 \ /
  Last update: 2005-03-22 13:34    [W:0.141 / U:0.308 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site