diff mbox series

netns: dangling pointer on netns bind mount point.

Message ID 20200407023512.GA25005@ubuntu
State New
Headers show
Series netns: dangling pointer on netns bind mount point. | expand

Commit Message

Levi April 7, 2020, 2:35 a.m. UTC 
When we try to bind mount on network namespace (ex) /proc/{pid}/ns/net,
inode's private data can have dangling pointer to net_namespace that was
already freed in below case.

    1. Forking the process.
    2. [PARENT] Waiting the Child till the end.
    3. [CHILD] call unshare for creating new network namespace
    4. [CHILD] Bind mount with /proc/self/ns/net to some mount point.
    5. [CHILD] Exit child.
    6. [PARENT] Try to setns with binded mount point

In step 5, net_namespace made by child process'll be freed,
But in bind mount point, it still held the pointer to net_namespace made
by child process.
In this situation, when parent try to call "setns" systemcall with the
bind mount point, parent process try to access to freed memory, That
makes memory corruption.

This patch fix the above scenario by increaseing reference count.

Signed-off-by: Levi Yun <ppbuk5246@gmail.com>
---
 fs/namespace.c              | 31 +++++++++++++++++++++++++++++++
 include/net/net_namespace.h |  7 +++++++
 net/core/net_namespace.c    |  5 -----
 3 files changed, 38 insertions(+), 5 deletions(-)

--
2.26.0
diff mbox series

Patch

diff --git a/fs/namespace.c b/fs/namespace.c
index a28e4db075ed..ed0fbb6a1b52 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -31,6 +31,10 @@ 
 #include <linux/fs_context.h>
 #include <linux/shmem_fs.h>

+#ifdef CONFIG_NET_NS
+#include <net/net_namespace.h>
+#endif
+
 #include "pnode.h"
 #include "internal.h"

@@ -1013,12 +1017,25 @@  vfs_submount(const struct dentry *mountpoint, struct file_system_type *type,
 }
 EXPORT_SYMBOL_GPL(vfs_submount);

+#ifdef CONFIG_NET_NS
+static bool is_net_ns_file(struct dentry *dentry)
+{
+	/* Is this a proxy for a network namespace? */
+	return dentry->d_op == &ns_dentry_operations &&
+		dentry->d_fsdata == &netns_operations;
+}
+#endif
+
 static struct mount *clone_mnt(struct mount *old, struct dentry *root,
 					int flag)
 {
 	struct super_block *sb = old->mnt.mnt_sb;
 	struct mount *mnt;
 	int err;
+#ifdef CONFIG_NET_NS
+	struct ns_common *ns = NULL;
+	struct net *net = NULL;
+#endif

 	mnt = alloc_vfsmnt(old->mnt_devname);
 	if (!mnt)
@@ -1035,6 +1052,20 @@  static struct mount *clone_mnt(struct mount *old, struct dentry *root,
 			goto out_free;
 	}

+#ifdef CONFIG_NET_NS
+	if (!(flag & CL_COPY_MNT_NS_FILE) && is_net_ns_file(root)) {
+		ns = get_proc_ns(d_inode(root));
+		if (ns == NULL || ns->ops->type != CLONE_NEWNET) {
+			err = -EINVAL;
+
+			goto out_free;
+		}
+
+		net = to_net_ns(ns);
+		net = get_net(net);
+	}
+#endif
+
 	mnt->mnt.mnt_flags = old->mnt.mnt_flags;
 	mnt->mnt.mnt_flags &= ~(MNT_WRITE_HOLD|MNT_MARKED|MNT_INTERNAL);

diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index ab96fb59131c..275258d1dbee 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -474,4 +474,11 @@  static inline void fnhe_genid_bump(struct net *net)
 	atomic_inc(&net->fnhe_genid);
 }

+#ifdef CONFIG_NET_NS
+static inline struct net *to_net_ns(struct ns_common *ns)
+{
+	return container_of(ns, struct net, ns);
+}
+#endif
+
 #endif /* __NET_NET_NAMESPACE_H */
diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
index 190ca66a383b..3a6d9155806f 100644
--- a/net/core/net_namespace.c
+++ b/net/core/net_namespace.c
@@ -1343,11 +1343,6 @@  static struct ns_common *netns_get(struct task_struct *task)
 	return net ? &net->ns : NULL;
 }

-static inline struct net *to_net_ns(struct ns_common *ns)
-{
-	return container_of(ns, struct net, ns);
-}
-
 static void netns_put(struct ns_common *ns)
 {
 	put_net(to_net_ns(ns));