From 72c4f88a9d99432b1e3645d8ddccd17089a670c3 Mon Sep 17 00:00:00 2001 From: Andrew Morton Date: Tue, 8 Apr 2003 21:31:28 -0700 Subject: [PATCH] [PATCH] Missing brelse() in ext2/ext3 extended attribute code From: Andreas Gruenbacher 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 | 7 ++++--- fs/ext3/xattr.c | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/fs/ext2/xattr.c b/fs/ext2/xattr.c index 5ecb528167ab8..01e94a68c13e9 100644 --- a/fs/ext2/xattr.c +++ b/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; } diff --git a/fs/ext3/xattr.c b/fs/ext3/xattr.c index 8eee2a4b010d8..46863b4ff8df8 100644 --- a/fs/ext3/xattr.c +++ b/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 @@ getblk_failed: } cleanup: - if (old_bh != new_bh) - brelse(new_bh); + brelse(new_bh); return error; } -- 2.53.0