Commit 3895dbf8 authored by Eric W. Biederman's avatar Eric W. Biederman

mnt: Protect the mountpoint hashtable with mount_lock

Protecting the mountpoint hashtable with namespace_sem was sufficient
until a call to umount_mnt was added to mntput_no_expire.  At which
point it became possible for multiple calls of put_mountpoint on
the same hash chain to happen on the same time.

Kristen Johansen <kjlx@templeofstupid.com> reported:
> This can cause a panic when simultaneous callers of put_mountpoint
> attempt to free the same mountpoint.  This occurs because some callers
> hold the mount_hash_lock, while others hold the namespace lock.  Some
> even hold both.
>
> In this submitter's case, the panic manifested itself as a GP fault in
> put_mountpoint() when it called hlist_del() and attempted to dereference
> a m_hash.pprev that had been poisioned by another thread.

Al Viro observed that the simple fix is to switch from using the namespace_sem
to the mount_lock to protect the mountpoint hash table.

I have taken Al's suggested patch moved put_mountpoint in pivot_root
(instead of taking mount_lock an additional time), and have replaced
new_mountpoint with get_mountpoint a function that does the hash table
lookup and addition under the mount_lock.   The introduction of get_mounptoint
ensures that only the mount_lock is needed to manipulate the mountpoint
hashtable.

d_set_mounted is modified to only set DCACHE_MOUNTED if it is not
already set.  This allows get_mountpoint to use the setting of
DCACHE_MOUNTED to ensure adding a struct mountpoint for a dentry
happens exactly once.

Cc: stable@vger.kernel.org
Fixes: ce07d891 ("mnt: Honor MNT_LOCKED when detaching mounts")
Reported-by: default avatarKrister Johansen <kjlx@templeofstupid.com>
Suggested-by: default avatarAl Viro <viro@ZenIV.linux.org.uk>
Acked-by: default avatarAl Viro <viro@ZenIV.linux.org.uk>
Signed-off-by: default avatar"Eric W. Biederman" <ebiederm@xmission.com>
parent 0c744ea4
...@@ -1336,9 +1336,12 @@ int d_set_mounted(struct dentry *dentry) ...@@ -1336,9 +1336,12 @@ int d_set_mounted(struct dentry *dentry)
} }
spin_lock(&dentry->d_lock); spin_lock(&dentry->d_lock);
if (!d_unlinked(dentry)) { if (!d_unlinked(dentry)) {
ret = -EBUSY;
if (!d_mountpoint(dentry)) {
dentry->d_flags |= DCACHE_MOUNTED; dentry->d_flags |= DCACHE_MOUNTED;
ret = 0; ret = 0;
} }
}
spin_unlock(&dentry->d_lock); spin_unlock(&dentry->d_lock);
out: out:
write_sequnlock(&rename_lock); write_sequnlock(&rename_lock);
......
...@@ -742,26 +742,50 @@ static struct mountpoint *lookup_mountpoint(struct dentry *dentry) ...@@ -742,26 +742,50 @@ static struct mountpoint *lookup_mountpoint(struct dentry *dentry)
return NULL; return NULL;
} }
static struct mountpoint *new_mountpoint(struct dentry *dentry) static struct mountpoint *get_mountpoint(struct dentry *dentry)
{ {
struct hlist_head *chain = mp_hash(dentry); struct mountpoint *mp, *new = NULL;
struct mountpoint *mp;
int ret; int ret;
mp = kmalloc(sizeof(struct mountpoint), GFP_KERNEL); if (d_mountpoint(dentry)) {
if (!mp) mountpoint:
read_seqlock_excl(&mount_lock);
mp = lookup_mountpoint(dentry);
read_sequnlock_excl(&mount_lock);
if (mp)
goto done;
}
if (!new)
new = kmalloc(sizeof(struct mountpoint), GFP_KERNEL);
if (!new)
return ERR_PTR(-ENOMEM); return ERR_PTR(-ENOMEM);
/* Exactly one processes may set d_mounted */
ret = d_set_mounted(dentry); ret = d_set_mounted(dentry);
if (ret) {
kfree(mp);
return ERR_PTR(ret);
}
mp->m_dentry = dentry; /* Someone else set d_mounted? */
mp->m_count = 1; if (ret == -EBUSY)
hlist_add_head(&mp->m_hash, chain); goto mountpoint;
INIT_HLIST_HEAD(&mp->m_list);
/* The dentry is not available as a mountpoint? */
mp = ERR_PTR(ret);
if (ret)
goto done;
/* Add the new mountpoint to the hash table */
read_seqlock_excl(&mount_lock);
new->m_dentry = dentry;
new->m_count = 1;
hlist_add_head(&new->m_hash, mp_hash(dentry));
INIT_HLIST_HEAD(&new->m_list);
read_sequnlock_excl(&mount_lock);
mp = new;
new = NULL;
done:
kfree(new);
return mp; return mp;
} }
...@@ -1595,11 +1619,11 @@ void __detach_mounts(struct dentry *dentry) ...@@ -1595,11 +1619,11 @@ void __detach_mounts(struct dentry *dentry)
struct mount *mnt; struct mount *mnt;
namespace_lock(); namespace_lock();
lock_mount_hash();
mp = lookup_mountpoint(dentry); mp = lookup_mountpoint(dentry);
if (IS_ERR_OR_NULL(mp)) if (IS_ERR_OR_NULL(mp))
goto out_unlock; goto out_unlock;
lock_mount_hash();
event++; event++;
while (!hlist_empty(&mp->m_list)) { while (!hlist_empty(&mp->m_list)) {
mnt = hlist_entry(mp->m_list.first, struct mount, mnt_mp_list); mnt = hlist_entry(mp->m_list.first, struct mount, mnt_mp_list);
...@@ -1609,9 +1633,9 @@ void __detach_mounts(struct dentry *dentry) ...@@ -1609,9 +1633,9 @@ void __detach_mounts(struct dentry *dentry)
} }
else umount_tree(mnt, UMOUNT_CONNECTED); else umount_tree(mnt, UMOUNT_CONNECTED);
} }
unlock_mount_hash();
put_mountpoint(mp); put_mountpoint(mp);
out_unlock: out_unlock:
unlock_mount_hash();
namespace_unlock(); namespace_unlock();
} }
...@@ -2038,9 +2062,7 @@ static struct mountpoint *lock_mount(struct path *path) ...@@ -2038,9 +2062,7 @@ static struct mountpoint *lock_mount(struct path *path)
namespace_lock(); namespace_lock();
mnt = lookup_mnt(path); mnt = lookup_mnt(path);
if (likely(!mnt)) { if (likely(!mnt)) {
struct mountpoint *mp = lookup_mountpoint(dentry); struct mountpoint *mp = get_mountpoint(dentry);
if (!mp)
mp = new_mountpoint(dentry);
if (IS_ERR(mp)) { if (IS_ERR(mp)) {
namespace_unlock(); namespace_unlock();
inode_unlock(dentry->d_inode); inode_unlock(dentry->d_inode);
...@@ -2059,7 +2081,11 @@ static struct mountpoint *lock_mount(struct path *path) ...@@ -2059,7 +2081,11 @@ static struct mountpoint *lock_mount(struct path *path)
static void unlock_mount(struct mountpoint *where) static void unlock_mount(struct mountpoint *where)
{ {
struct dentry *dentry = where->m_dentry; struct dentry *dentry = where->m_dentry;
read_seqlock_excl(&mount_lock);
put_mountpoint(where); put_mountpoint(where);
read_sequnlock_excl(&mount_lock);
namespace_unlock(); namespace_unlock();
inode_unlock(dentry->d_inode); inode_unlock(dentry->d_inode);
} }
...@@ -3135,9 +3161,9 @@ SYSCALL_DEFINE2(pivot_root, const char __user *, new_root, ...@@ -3135,9 +3161,9 @@ SYSCALL_DEFINE2(pivot_root, const char __user *, new_root,
touch_mnt_namespace(current->nsproxy->mnt_ns); touch_mnt_namespace(current->nsproxy->mnt_ns);
/* A moved mount should not expire automatically */ /* A moved mount should not expire automatically */
list_del_init(&new_mnt->mnt_expire); list_del_init(&new_mnt->mnt_expire);
put_mountpoint(root_mp);
unlock_mount_hash(); unlock_mount_hash();
chroot_fs_refs(&root, &new); chroot_fs_refs(&root, &new);
put_mountpoint(root_mp);
error = 0; error = 0;
out4: out4:
unlock_mount(old_mp); unlock_mount(old_mp);
......
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