lk/linux-kernel-test
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

reiserfs: fix xattr root locking/refcount bug

Browse Source 
The listxattr() and getxattr() operations are only protected by a read
lock.  As a result, if either of these operations run in parallel, a race
condition exists where the xattr_root will end up being cached twice, which
results in the leaking of a reference and a BUG() on umount.

This patch refactors get_xa_root(), __get_xa_root(), and create_xa_root(),
into one get_xa_root() function that takes the appropriate locking around
the entire critical section.

Reported, diagnosed and tested by Andrea Righi <a.righi@cineca.it>

Signed-off-by: Jeff Mahoney <jeffm@suse.com>
Cc: Andrea Righi <a.righi@cineca.it>
Cc: "Vladimir V. Saveliev" <vs@namesys.com>
Cc: Edward Shishkin <edward@namesys.com>
Cc: Alex Zarochentsev <zam@namesys.com>
Cc: <stable@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This commit is contained in:
Jeff Mahoney
2007-04-23 14:41:17 -07:00
committed by Linus Torvalds
parent 1a641fceb6
commit 9b7f375505
1 changed files with 24 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files

92
fs/reiserfs/xattr.c
View File

@@ -54,82 +54,48 @@
static struct reiserfs_xattr_handler *find_xattr_handler_prefix(const char static struct reiserfs_xattr_handler *find_xattr_handler_prefix(const char
*prefix); *prefix);
static struct dentry *create_xa_root(struct super_block *sb) /* Returns the dentry referring to the root of the extended attribute
* directory tree. If it has already been retrieved, it is used. If it
* hasn't been created and the flags indicate creation is allowed, we
* attempt to create it. On error, we return a pointer-encoded error.
*/
static struct dentry *get_xa_root(struct super_block *sb, int flags)
{ {
struct dentry *privroot = dget(REISERFS_SB(sb)->priv_root); struct dentry *privroot = dget(REISERFS_SB(sb)->priv_root);
struct dentry *xaroot; struct dentry *xaroot;
/* This needs to be created at mount-time */ /* This needs to be created at mount-time */
if (!privroot) if (!privroot)
return ERR_PTR(-EOPNOTSUPP); return ERR_PTR(-ENODATA);
mutex_lock(&privroot->d_inode->i_mutex);
if (REISERFS_SB(sb)->xattr_root) {
xaroot = dget(REISERFS_SB(sb)->xattr_root);
goto out;
}
xaroot = lookup_one_len(XAROOT_NAME, privroot, strlen(XAROOT_NAME)); xaroot = lookup_one_len(XAROOT_NAME, privroot, strlen(XAROOT_NAME));
if (IS_ERR(xaroot)) { if (IS_ERR(xaroot)) {
goto out; goto out;
} else if (!xaroot->d_inode) { } else if (!xaroot->d_inode) {
int err; int err = -ENODATA;
mutex_lock(&privroot->d_inode->i_mutex); if (flags == 0 || flags & XATTR_CREATE)
err = err = privroot->d_inode->i_op->mkdir(privroot->d_inode,
privroot->d_inode->i_op->mkdir(privroot->d_inode, xaroot, xaroot, 0700);
0700);
mutex_unlock(&privroot->d_inode->i_mutex);
if (err) { if (err) {
dput(xaroot); dput(xaroot);
dput(privroot); xaroot = ERR_PTR(err);
return ERR_PTR(err); goto out;
} }
REISERFS_SB(sb)->xattr_root = dget(xaroot);
} }
REISERFS_SB(sb)->xattr_root = dget(xaroot);
out: out:
mutex_unlock(&privroot->d_inode->i_mutex);
dput(privroot); dput(privroot);
return xaroot; return xaroot;
} }
/* This will return a dentry, or error, refering to the xa root directory.
* If the xa root doesn't exist yet, the dentry will be returned without
* an associated inode. This dentry can be used with ->mkdir to create
* the xa directory. */
static struct dentry *__get_xa_root(struct super_block *s)
{
struct dentry *privroot = dget(REISERFS_SB(s)->priv_root);
struct dentry *xaroot = NULL;
if (IS_ERR(privroot) || !privroot)
return privroot;
xaroot = lookup_one_len(XAROOT_NAME, privroot, strlen(XAROOT_NAME));
if (IS_ERR(xaroot)) {
goto out;
} else if (!xaroot->d_inode) {
dput(xaroot);
xaroot = NULL;
goto out;
}
REISERFS_SB(s)->xattr_root = dget(xaroot);
out:
dput(privroot);
return xaroot;
}
/* Returns the dentry (or NULL) referring to the root of the extended
* attribute directory tree. If it has already been retrieved, it is used.
* Otherwise, we attempt to retrieve it from disk. It may also return
* a pointer-encoded error.
*/
static inline struct dentry *get_xa_root(struct super_block *s)
{
struct dentry *dentry = dget(REISERFS_SB(s)->xattr_root);
if (!dentry)
dentry = __get_xa_root(s);
return dentry;
}
/* Opens the directory corresponding to the inode's extended attribute store. /* Opens the directory corresponding to the inode's extended attribute store.
* If flags allow, the tree to the directory may be created. If creation is * If flags allow, the tree to the directory may be created. If creation is
* prohibited, -ENODATA is returned. */ * prohibited, -ENODATA is returned. */
@@ -138,21 +104,11 @@ static struct dentry *open_xa_dir(const struct inode *inode, int flags)
struct dentry *xaroot, *xadir; struct dentry *xaroot, *xadir;
char namebuf[17]; char namebuf[17];
xaroot = get_xa_root(inode->i_sb); xaroot = get_xa_root(inode->i_sb, flags);
if (IS_ERR(xaroot)) { if (IS_ERR(xaroot))
return xaroot; return xaroot;
} else if (!xaroot) {
if (flags == 0 || flags & XATTR_CREATE) {
xaroot = create_xa_root(inode->i_sb);
if (IS_ERR(xaroot))
return xaroot;
}
if (!xaroot)
return ERR_PTR(-ENODATA);
}
/* ok, we have xaroot open */ /* ok, we have xaroot open */
snprintf(namebuf, sizeof(namebuf), "%X.%X", snprintf(namebuf, sizeof(namebuf), "%X.%X",
le32_to_cpu(INODE_PKEY(inode)->k_objectid), le32_to_cpu(INODE_PKEY(inode)->k_objectid),
inode->i_generation); inode->i_generation);
@@ -821,7 +777,7 @@ int reiserfs_delete_xattrs(struct inode *inode)
/* Leftovers besides . and .. -- that's not good. */ /* Leftovers besides . and .. -- that's not good. */
if (dir->d_inode->i_nlink <= 2) { if (dir->d_inode->i_nlink <= 2) {
root = get_xa_root(inode->i_sb); root = get_xa_root(inode->i_sb, XATTR_REPLACE);
reiserfs_write_lock_xattrs(inode->i_sb); reiserfs_write_lock_xattrs(inode->i_sb);
err = vfs_rmdir(root->d_inode, dir); err = vfs_rmdir(root->d_inode, dir);
reiserfs_write_unlock_xattrs(inode->i_sb); reiserfs_write_unlock_xattrs(inode->i_sb);