Skip to content

Commit

Permalink
Fix the regression created by "set S_DEAD on unlink()..." commit
Browse files Browse the repository at this point in the history
1) i_flags simply doesn't work for mount/unlink race prevention;
we may have many links to file and rm on one of those obviously
shouldn't prevent bind on top of another later on.  To fix it
right way we need to mark _dentry_ as unsuitable for mounting
upon; new flag (DCACHE_CANT_MOUNT) is protected by d_flags and
i_mutex on the inode in question.  Set it (with dont_mount(dentry))
in unlink/rmdir/etc., check (with cant_mount(dentry)) in places
in namespace.c that used to check for S_DEAD.  Setting S_DEAD
is still needed in places where we used to set it (for directories
getting killed), since we rely on it for readdir/rmdir race
prevention.

2) rename()/mount() protection has another bogosity - we unhash
the target before we'd checked that it's not a mountpoint.  Fixed.

3) ancient bogosity in pivot_root() - we locked i_mutex on the
right directory, but checked S_DEAD on the different (and wrong)
one.  Noticed and fixed.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
  • Loading branch information
Al Viro committed May 15, 2010
1 parent 6a251b0 commit d83c49f
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 11 deletions.
1 change: 1 addition & 0 deletions drivers/usb/core/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,7 @@ static int usbfs_rmdir(struct inode *dir, struct dentry *dentry)
mutex_lock(&inode->i_mutex);
dentry_unhash(dentry);
if (usbfs_empty(dentry)) {
dont_mount(dentry);
drop_nlink(dentry->d_inode);
drop_nlink(dentry->d_inode);
dput(dentry);
Expand Down
4 changes: 4 additions & 0 deletions fs/configfs/dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,7 @@ static void detach_groups(struct config_group *group)

configfs_detach_group(sd->s_element);
child->d_inode->i_flags |= S_DEAD;
dont_mount(child);

mutex_unlock(&child->d_inode->i_mutex);

Expand Down Expand Up @@ -840,6 +841,7 @@ static int configfs_attach_item(struct config_item *parent_item,
mutex_lock(&dentry->d_inode->i_mutex);
configfs_remove_dir(item);
dentry->d_inode->i_flags |= S_DEAD;
dont_mount(dentry);
mutex_unlock(&dentry->d_inode->i_mutex);
d_delete(dentry);
}
Expand Down Expand Up @@ -882,6 +884,7 @@ static int configfs_attach_group(struct config_item *parent_item,
if (ret) {
configfs_detach_item(item);
dentry->d_inode->i_flags |= S_DEAD;
dont_mount(dentry);
}
configfs_adjust_dir_dirent_depth_after_populate(sd);
mutex_unlock(&dentry->d_inode->i_mutex);
Expand Down Expand Up @@ -1725,6 +1728,7 @@ void configfs_unregister_subsystem(struct configfs_subsystem *subsys)
mutex_unlock(&configfs_symlink_mutex);
configfs_detach_group(&group->cg_item);
dentry->d_inode->i_flags |= S_DEAD;
dont_mount(dentry);
mutex_unlock(&dentry->d_inode->i_mutex);

d_delete(dentry);
Expand Down
21 changes: 13 additions & 8 deletions fs/namei.c
Original file line number Diff line number Diff line change
Expand Up @@ -2176,8 +2176,10 @@ int vfs_rmdir(struct inode *dir, struct dentry *dentry)
error = security_inode_rmdir(dir, dentry);
if (!error) {
error = dir->i_op->rmdir(dir, dentry);
if (!error)
if (!error) {
dentry->d_inode->i_flags |= S_DEAD;
dont_mount(dentry);
}
}
}
mutex_unlock(&dentry->d_inode->i_mutex);
Expand Down Expand Up @@ -2261,7 +2263,7 @@ int vfs_unlink(struct inode *dir, struct dentry *dentry)
if (!error) {
error = dir->i_op->unlink(dir, dentry);
if (!error)
dentry->d_inode->i_flags |= S_DEAD;
dont_mount(dentry);
}
}
mutex_unlock(&dentry->d_inode->i_mutex);
Expand Down Expand Up @@ -2572,17 +2574,20 @@ static int vfs_rename_dir(struct inode *old_dir, struct dentry *old_dentry,
return error;

target = new_dentry->d_inode;
if (target) {
if (target)
mutex_lock(&target->i_mutex);
dentry_unhash(new_dentry);
}
if (d_mountpoint(old_dentry)||d_mountpoint(new_dentry))
error = -EBUSY;
else
else {
if (target)
dentry_unhash(new_dentry);
error = old_dir->i_op->rename(old_dir, old_dentry, new_dir, new_dentry);
}
if (target) {
if (!error)
if (!error) {
target->i_flags |= S_DEAD;
dont_mount(new_dentry);
}
mutex_unlock(&target->i_mutex);
if (d_unhashed(new_dentry))
d_rehash(new_dentry);
Expand Down Expand Up @@ -2614,7 +2619,7 @@ static int vfs_rename_other(struct inode *old_dir, struct dentry *old_dentry,
error = old_dir->i_op->rename(old_dir, old_dentry, new_dir, new_dentry);
if (!error) {
if (target)
target->i_flags |= S_DEAD;
dont_mount(new_dentry);
if (!(old_dir->i_sb->s_type->fs_flags & FS_RENAME_DOES_D_MOVE))
d_move(old_dentry, new_dentry);
}
Expand Down
6 changes: 3 additions & 3 deletions fs/namespace.c
Original file line number Diff line number Diff line change
Expand Up @@ -1432,7 +1432,7 @@ static int graft_tree(struct vfsmount *mnt, struct path *path)

err = -ENOENT;
mutex_lock(&path->dentry->d_inode->i_mutex);
if (IS_DEADDIR(path->dentry->d_inode))
if (cant_mount(path->dentry))
goto out_unlock;

err = security_sb_check_sb(mnt, path);
Expand Down Expand Up @@ -1623,7 +1623,7 @@ static int do_move_mount(struct path *path, char *old_name)

err = -ENOENT;
mutex_lock(&path->dentry->d_inode->i_mutex);
if (IS_DEADDIR(path->dentry->d_inode))
if (cant_mount(path->dentry))
goto out1;

if (d_unlinked(path->dentry))
Expand Down Expand Up @@ -2234,7 +2234,7 @@ SYSCALL_DEFINE2(pivot_root, const char __user *, new_root,
if (!check_mnt(root.mnt))
goto out2;
error = -ENOENT;
if (IS_DEADDIR(new.dentry->d_inode))
if (cant_mount(old.dentry))
goto out2;
if (d_unlinked(new.dentry))
goto out2;
Expand Down
14 changes: 14 additions & 0 deletions include/linux/dcache.h
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,8 @@ d_iput: no no no yes

#define DCACHE_FSNOTIFY_PARENT_WATCHED 0x0080 /* Parent inode is watched by some fsnotify listener */

#define DCACHE_CANT_MOUNT 0x0100

extern spinlock_t dcache_lock;
extern seqlock_t rename_lock;

Expand Down Expand Up @@ -358,6 +360,18 @@ static inline int d_unlinked(struct dentry *dentry)
return d_unhashed(dentry) && !IS_ROOT(dentry);
}

static inline int cant_mount(struct dentry *dentry)
{
return (dentry->d_flags & DCACHE_CANT_MOUNT);
}

static inline void dont_mount(struct dentry *dentry)
{
spin_lock(&dentry->d_lock);
dentry->d_flags |= DCACHE_CANT_MOUNT;
spin_unlock(&dentry->d_lock);
}

static inline struct dentry *dget_parent(struct dentry *dentry)
{
struct dentry *ret;
Expand Down

0 comments on commit d83c49f

Please sign in to comment.