All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Fix a race in put_mountpoint.
@ 2016-12-31  4:10 Krister Johansen
  2016-12-31  6:17 ` Al Viro
  0 siblings, 1 reply; 63+ messages in thread
From: Krister Johansen @ 2016-12-31  4:10 UTC (permalink / raw)
  To: Alexander Viro; +Cc: linux-fsdevel, Eric W. Biederman

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.

Instead of trying to force all mountpoint hash users back under the
namespace lock, add locks that cover just the mountpoint hash.  This
uses hlist_bl to protect against simlultaneous additions and removals,
and RCU for lookups.

Signed-off-by: Krister Johansen <kjlx@templeofstupid.com>
---
 fs/mount.h     |  6 ++++--
 fs/namespace.c | 62 ++++++++++++++++++++++++++++++++++++++++------------------
 2 files changed, 47 insertions(+), 21 deletions(-)

diff --git a/fs/mount.h b/fs/mount.h
index 2c856fc..1a2f41a 100644
--- a/fs/mount.h
+++ b/fs/mount.h
@@ -3,6 +3,7 @@
 #include <linux/poll.h>
 #include <linux/ns_common.h>
 #include <linux/fs_pin.h>
+#include <linux/rculist_bl.h>
 
 struct mnt_namespace {
 	atomic_t		count;
@@ -24,10 +25,11 @@ struct mnt_pcp {
 };
 
 struct mountpoint {
-	struct hlist_node m_hash;
+	struct hlist_bl_node m_hash;
 	struct dentry *m_dentry;
 	struct hlist_head m_list;
-	int m_count;
+	atomic_t m_count;
+	struct rcu_head m_rcu;
 };
 
 struct mount {
diff --git a/fs/namespace.c b/fs/namespace.c
index b5b1259..7c29420 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -63,7 +63,7 @@ static int mnt_id_start = 0;
 static int mnt_group_start = 1;
 
 static struct hlist_head *mount_hashtable __read_mostly;
-static struct hlist_head *mountpoint_hashtable __read_mostly;
+static struct hlist_bl_head *mountpoint_hashtable __read_mostly;
 static struct kmem_cache *mnt_cache __read_mostly;
 static DECLARE_RWSEM(namespace_sem);
 
@@ -89,7 +89,7 @@ static inline struct hlist_head *m_hash(struct vfsmount *mnt, struct dentry *den
 	return &mount_hashtable[tmp & m_hash_mask];
 }
 
-static inline struct hlist_head *mp_hash(struct dentry *dentry)
+static inline struct hlist_bl_head *mp_hash(struct dentry *dentry)
 {
 	unsigned long tmp = ((unsigned long)dentry / L1_CACHE_BYTES);
 	tmp = tmp + (tmp >> mp_hash_shift);
@@ -727,27 +727,36 @@ bool __is_local_mountpoint(struct dentry *dentry)
 
 static struct mountpoint *lookup_mountpoint(struct dentry *dentry)
 {
-	struct hlist_head *chain = mp_hash(dentry);
+	struct hlist_bl_head *chain = mp_hash(dentry);
+	struct hlist_bl_node *node;
 	struct mountpoint *mp;
 
-	hlist_for_each_entry(mp, chain, m_hash) {
+	rcu_read_lock();
+	hlist_bl_for_each_entry_rcu(mp, node, chain, m_hash) {
 		if (mp->m_dentry == dentry) {
 			/* might be worth a WARN_ON() */
-			if (d_unlinked(dentry))
-				return ERR_PTR(-ENOENT);
-			mp->m_count++;
-			return mp;
+			if (d_unlinked(dentry)) {
+				mp = ERR_PTR(-ENOENT);
+				goto out;
+			}
+			if (atomic_inc_not_zero(&mp->m_count))
+				goto out;
 		}
 	}
-	return NULL;
+	mp = NULL;
+out:
+	rcu_read_unlock();
+	return mp;
 }
 
 static struct mountpoint *new_mountpoint(struct dentry *dentry)
 {
-	struct hlist_head *chain = mp_hash(dentry);
+	struct hlist_bl_head *chain = mp_hash(dentry);
 	struct mountpoint *mp;
 	int ret;
 
+	WARN_ON(!rwsem_is_locked(&namespace_sem));
+
 	mp = kmalloc(sizeof(struct mountpoint), GFP_KERNEL);
 	if (!mp)
 		return ERR_PTR(-ENOMEM);
@@ -759,22 +768,37 @@ static struct mountpoint *new_mountpoint(struct dentry *dentry)
 	}
 
 	mp->m_dentry = dentry;
-	mp->m_count = 1;
-	hlist_add_head(&mp->m_hash, chain);
+	init_rcu_head(&mp->m_rcu);
+	atomic_set(&mp->m_count, 1);
+	hlist_bl_lock(chain);
+	hlist_bl_add_head_rcu(&mp->m_hash, chain);
+	hlist_bl_unlock(chain);
 	INIT_HLIST_HEAD(&mp->m_list);
 	return mp;
 }
 
+static void free_mountpoint(struct rcu_head *head)
+{
+	struct mountpoint *mp;
+
+	mp = container_of(head, struct mountpoint, m_rcu);
+	kfree(mp);
+}
+
 static void put_mountpoint(struct mountpoint *mp)
 {
-	if (!--mp->m_count) {
+	if (atomic_dec_and_test(&mp->m_count)) {
 		struct dentry *dentry = mp->m_dentry;
+		struct hlist_bl_head *chain = mp_hash(dentry);
+
 		BUG_ON(!hlist_empty(&mp->m_list));
 		spin_lock(&dentry->d_lock);
 		dentry->d_flags &= ~DCACHE_MOUNTED;
 		spin_unlock(&dentry->d_lock);
-		hlist_del(&mp->m_hash);
-		kfree(mp);
+		hlist_bl_lock(chain);
+		hlist_bl_del_rcu(&mp->m_hash);
+		hlist_bl_unlock(chain);
+		call_rcu(&mp->m_rcu, free_mountpoint);
 	}
 }
 
@@ -846,7 +870,7 @@ void mnt_set_mountpoint(struct mount *mnt,
 			struct mountpoint *mp,
 			struct mount *child_mnt)
 {
-	mp->m_count++;
+	atomic_inc(&mp->m_count);
 	mnt_add_count(mnt, 1);	/* essentially, that's mntget */
 	child_mnt->mnt_mountpoint = dget(mp->m_dentry);
 	child_mnt->mnt_parent = mnt;
@@ -3120,7 +3144,7 @@ SYSCALL_DEFINE2(pivot_root, const char __user *, new_root,
 	/* make certain new is below the root */
 	if (!is_path_reachable(new_mnt, new.dentry, &root))
 		goto out4;
-	root_mp->m_count++; /* pin it so it won't go away */
+	atomic_inc(&root_mp->m_count); /* pin it so it won't go away */
 	lock_mount_hash();
 	detach_mnt(new_mnt, &parent_path);
 	detach_mnt(root_mnt, &root_parent);
@@ -3199,7 +3223,7 @@ void __init mnt_init(void)
 				0,
 				&m_hash_shift, &m_hash_mask, 0, 0);
 	mountpoint_hashtable = alloc_large_system_hash("Mountpoint-cache",
-				sizeof(struct hlist_head),
+				sizeof(struct hlist_bl_head),
 				mphash_entries, 19,
 				0,
 				&mp_hash_shift, &mp_hash_mask, 0, 0);
@@ -3210,7 +3234,7 @@ void __init mnt_init(void)
 	for (u = 0; u <= m_hash_mask; u++)
 		INIT_HLIST_HEAD(&mount_hashtable[u]);
 	for (u = 0; u <= mp_hash_mask; u++)
-		INIT_HLIST_HEAD(&mountpoint_hashtable[u]);
+		INIT_HLIST_BL_HEAD(&mountpoint_hashtable[u]);
 
 	kernfs_init();
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 63+ messages in thread

end of thread, other threads:[~2017-06-07 13:16 UTC | newest]

Thread overview: 63+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-31  4:10 [PATCH] Fix a race in put_mountpoint Krister Johansen
2016-12-31  6:17 ` Al Viro
2017-01-03  0:51   ` Eric W. Biederman
2017-01-03  1:48     ` Al Viro
2017-01-03  3:17       ` Eric W. Biederman
2017-01-03  4:00         ` Al Viro
2017-01-04  3:52           ` Eric W. Biederman
2017-01-04  3:53             ` [PATCH] mnt: Protect the mountpoint hashtable with mount_lock Eric W. Biederman
2017-01-04 21:04               ` [REVIEW][PATCH] mnt: Tuck mounts under others instead of creating shadow/side mounts Eric W. Biederman
2017-01-07  5:06                 ` Al Viro
2017-01-11  0:10                   ` Eric W. Biederman
2017-01-11  4:11                     ` Al Viro
2017-01-11 16:03                       ` Eric W. Biederman
2017-01-11 16:18                         ` [REVIEW][PATCH 1/2] mnt: Fix propagate_mount_busy to notice all cases of busy mounts Eric W. Biederman
2017-01-11 16:19                           ` [REVIEW][PATCH 2/2] mnt: Tuck mounts under others instead of creating shadow/side mounts Eric W. Biederman
2017-01-12  5:45                             ` Al Viro
2017-01-20  7:20                               ` Eric W. Biederman
2017-01-20  7:26                               ` [PATCH v5] " Eric W. Biederman
2017-01-21  3:58                                 ` Ram Pai
2017-01-21  4:15                                   ` Eric W. Biederman
2017-01-23 19:02                                     ` Ram Pai
2017-01-24  0:16                                       ` Eric W. Biederman
2017-02-03 10:54                                         ` Eric W. Biederman
2017-02-03 17:10                                           ` Ram Pai
2017-02-03 18:26                                             ` Eric W. Biederman
2017-02-03 20:28                                               ` Ram Pai
2017-02-03 20:58                                                 ` Eric W. Biederman
2017-02-06  3:25                                                   ` Andrei Vagin
2017-02-06 21:40                                                     ` Ram Pai
2017-02-07  6:35                                                       ` Andrei Vagin
2017-01-12  5:30                           ` [REVIEW][PATCH 1/2] mnt: Fix propagate_mount_busy to notice all cases of busy mounts Al Viro
2017-01-20  7:18                             ` Eric W. Biederman
2017-01-13 20:32                           ` Andrei Vagin
2017-01-18 19:20                             ` Andrei Vagin
2017-01-20 23:18                           ` Ram Pai
2017-01-23  8:15                             ` Eric W. Biederman
2017-01-23 17:04                               ` Ram Pai
2017-01-12  5:03                         ` [REVIEW][PATCH] mnt: Tuck mounts under others instead of creating shadow/side mounts Al Viro
2017-05-14  2:15                 ` Andrei Vagin
2017-05-14  4:05                   ` Eric W. Biederman
2017-05-14  9:26                     ` Eric W. Biederman
2017-05-15 18:27                       ` Andrei Vagin
2017-05-15 19:42                         ` Eric W. Biederman
2017-05-15 20:10                           ` [REVIEW][PATCH] mnt: In umount propagation reparent in a separate pass Eric W. Biederman
2017-05-15 23:12                             ` Andrei Vagin
2017-05-16  5:42                             ` [PATCH] test: check a case when a mount is propagated between exiting mounts Andrei Vagin
2017-05-17  5:54                             ` [REVIEW][PATCH 1/2] mnt: In propgate_umount handle visiting mounts in any order Eric W. Biederman
2017-05-17  5:55                               ` [REVIEW][PATCH 2/2] mnt: Make propagate_umount less slow for overlapping mount propagation trees Eric W. Biederman
2017-05-17 22:48                                 ` Andrei Vagin
2017-05-17 23:26                                   ` Eric W. Biederman
2017-05-18  0:51                                     ` Andrei Vagin
2017-05-24 20:42                               ` [REVIEW][PATCH 1/2] mnt: In propgate_umount handle visiting mounts in any order Ram Pai
2017-05-24 21:54                                 ` Eric W. Biederman
2017-05-24 22:35                                   ` Ram Pai
2017-05-30  6:07                               ` Ram Pai
2017-05-30 15:07                                 ` Eric W. Biederman
2017-06-07  9:54                                   ` Ram Pai
2017-06-07 13:09                                     ` Eric W. Biederman
2017-05-22  8:15                             ` [REVIEW][PATCH] mnt: In umount propagation reparent in a separate pass Ram Pai
2017-05-22 18:33                               ` Eric W. Biederman
2017-05-22 22:34                                 ` Ram Pai
2017-05-23 13:58                                   ` Eric W. Biederman
2017-01-06  7:00               ` [PATCH] mnt: Protect the mountpoint hashtable with mount_lock Krister Johansen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.