Commit 18367501 authored by Al Viro's avatar Al Viro

fix loop checks in d_materialise_unique()

Both __d_unalias() and __d_materialise_dentry() need loop prevention.
Grab rename_lock in caller, check for loops there...

As a side benefit, we have dentry_lock_for_move() called only under
rename_lock, which seriously reduces deadlock potential of the
execrable "locking order" used for ->d_lock.
Signed-off-by: default avatarAl Viro <viro@zeniv.linux.org.uk>
parent 94c0d4ec
...@@ -2213,14 +2213,15 @@ static void dentry_unlock_parents_for_move(struct dentry *dentry, ...@@ -2213,14 +2213,15 @@ static void dentry_unlock_parents_for_move(struct dentry *dentry,
* The hash value has to match the hash queue that the dentry is on.. * The hash value has to match the hash queue that the dentry is on..
*/ */
/* /*
* d_move - move a dentry * __d_move - move a dentry
* @dentry: entry to move * @dentry: entry to move
* @target: new dentry * @target: new dentry
* *
* Update the dcache to reflect the move of a file name. Negative * Update the dcache to reflect the move of a file name. Negative
* dcache entries should not be moved in this way. * dcache entries should not be moved in this way. Caller hold
* rename_lock.
*/ */
void d_move(struct dentry * dentry, struct dentry * target) static void __d_move(struct dentry * dentry, struct dentry * target)
{ {
if (!dentry->d_inode) if (!dentry->d_inode)
printk(KERN_WARNING "VFS: moving negative dcache entry\n"); printk(KERN_WARNING "VFS: moving negative dcache entry\n");
...@@ -2228,8 +2229,6 @@ void d_move(struct dentry * dentry, struct dentry * target) ...@@ -2228,8 +2229,6 @@ void d_move(struct dentry * dentry, struct dentry * target)
BUG_ON(d_ancestor(dentry, target)); BUG_ON(d_ancestor(dentry, target));
BUG_ON(d_ancestor(target, dentry)); BUG_ON(d_ancestor(target, dentry));
write_seqlock(&rename_lock);
dentry_lock_for_move(dentry, target); dentry_lock_for_move(dentry, target);
write_seqcount_begin(&dentry->d_seq); write_seqcount_begin(&dentry->d_seq);
...@@ -2275,6 +2274,20 @@ void d_move(struct dentry * dentry, struct dentry * target) ...@@ -2275,6 +2274,20 @@ void d_move(struct dentry * dentry, struct dentry * target)
spin_unlock(&target->d_lock); spin_unlock(&target->d_lock);
fsnotify_d_move(dentry); fsnotify_d_move(dentry);
spin_unlock(&dentry->d_lock); spin_unlock(&dentry->d_lock);
}
/*
* d_move - move a dentry
* @dentry: entry to move
* @target: new dentry
*
* Update the dcache to reflect the move of a file name. Negative
* dcache entries should not be moved in this way.
*/
void d_move(struct dentry *dentry, struct dentry *target)
{
write_seqlock(&rename_lock);
__d_move(dentry, target);
write_sequnlock(&rename_lock); write_sequnlock(&rename_lock);
} }
EXPORT_SYMBOL(d_move); EXPORT_SYMBOL(d_move);
...@@ -2302,7 +2315,7 @@ struct dentry *d_ancestor(struct dentry *p1, struct dentry *p2) ...@@ -2302,7 +2315,7 @@ struct dentry *d_ancestor(struct dentry *p1, struct dentry *p2)
* This helper attempts to cope with remotely renamed directories * This helper attempts to cope with remotely renamed directories
* *
* It assumes that the caller is already holding * It assumes that the caller is already holding
* dentry->d_parent->d_inode->i_mutex and the inode->i_lock * dentry->d_parent->d_inode->i_mutex, inode->i_lock and rename_lock
* *
* Note: If ever the locking in lock_rename() changes, then please * Note: If ever the locking in lock_rename() changes, then please
* remember to update this too... * remember to update this too...
...@@ -2317,11 +2330,6 @@ static struct dentry *__d_unalias(struct inode *inode, ...@@ -2317,11 +2330,6 @@ static struct dentry *__d_unalias(struct inode *inode,
if (alias->d_parent == dentry->d_parent) if (alias->d_parent == dentry->d_parent)
goto out_unalias; goto out_unalias;
/* Check for loops */
ret = ERR_PTR(-ELOOP);
if (d_ancestor(alias, dentry))
goto out_err;
/* See lock_rename() */ /* See lock_rename() */
ret = ERR_PTR(-EBUSY); ret = ERR_PTR(-EBUSY);
if (!mutex_trylock(&dentry->d_sb->s_vfs_rename_mutex)) if (!mutex_trylock(&dentry->d_sb->s_vfs_rename_mutex))
...@@ -2331,7 +2339,7 @@ static struct dentry *__d_unalias(struct inode *inode, ...@@ -2331,7 +2339,7 @@ static struct dentry *__d_unalias(struct inode *inode,
goto out_err; goto out_err;
m2 = &alias->d_parent->d_inode->i_mutex; m2 = &alias->d_parent->d_inode->i_mutex;
out_unalias: out_unalias:
d_move(alias, dentry); __d_move(alias, dentry);
ret = alias; ret = alias;
out_err: out_err:
spin_unlock(&inode->i_lock); spin_unlock(&inode->i_lock);
...@@ -2416,15 +2424,24 @@ struct dentry *d_materialise_unique(struct dentry *dentry, struct inode *inode) ...@@ -2416,15 +2424,24 @@ struct dentry *d_materialise_unique(struct dentry *dentry, struct inode *inode)
alias = __d_find_alias(inode, 0); alias = __d_find_alias(inode, 0);
if (alias) { if (alias) {
actual = alias; actual = alias;
/* Is this an anonymous mountpoint that we could splice write_seqlock(&rename_lock);
* into our tree? */
if (IS_ROOT(alias)) { if (d_ancestor(alias, dentry)) {
/* Check for loops */
actual = ERR_PTR(-ELOOP);
} else if (IS_ROOT(alias)) {
/* Is this an anonymous mountpoint that we
* could splice into our tree? */
__d_materialise_dentry(dentry, alias); __d_materialise_dentry(dentry, alias);
write_sequnlock(&rename_lock);
__d_drop(alias); __d_drop(alias);
goto found; goto found;
} } else {
/* Nope, but we must(!) avoid directory aliasing */ /* Nope, but we must(!) avoid directory
* aliasing */
actual = __d_unalias(inode, dentry, alias); actual = __d_unalias(inode, dentry, alias);
}
write_sequnlock(&rename_lock);
if (IS_ERR(actual)) if (IS_ERR(actual))
dput(alias); dput(alias);
goto out_nolock; goto out_nolock;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment