[PATCH] net: pktgen: don't register a device file under a name that

[Christian Brauner]

  ip link set lo name pgctrl
  ip link set pgctrl up
  echo "add_device pgctrl" > /proc/net/pktgen/kpktgend_0

  proc_dir_entry 'pktgen/pgctrl' already registered
  WARNING: fs/proc/generic.c:404 at proc_register+0x555/0x8b0, CPU#0: bash/241
  Call Trace:
   proc_create_data+0x9f/0xf0
   pktgen_thread_write+0xd9b/0x1780
   proc_reg_write+0x1c2/0x3c0
   vfs_write+0x1cd/0xfa0

The same happens from the netdev notifier when a device that pktgen has
a file for is renamed to pgctrl, and when a device that two threads
have under different "@" suffixes is renamed, both files move to the
bare device name then. The files of a network namespace belong to the
root of its user namespace and none of this needs a capability. So
once pktgen is loaded an unprivileged user can taint the kernel, or
crash it with panic_on_warn, from namespaces of its own.

Remember the name a device file is registered under and refuse a name
that pgctrl, a thread file or another device file has, both for
"add_device" and before the file of a renamed device moves, which then
keeps its old name. The thread files go by the prefix of their names,
the threads of a namespace are created one after the other while its
first thread file can be written already.

Reported-by: Chris Mason <mason@xxxxxxxxxx>
Suggested-by: Alexey Dobriyan <adobriyan@xxxxxxxxx>
Link: https://lore.kernel.org/r/9cb48eaf-72f2-4e9f-b979-1ead58deb22f@p183
Fixes: 6c2f91e077f1 ("proc: use WARN() rather than printk+backtrace")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Christian Brauner (Amutable) <brauner@xxxxxxxxxx>
---
 net/core/pktgen.c | 51 ++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 48 insertions(+), 3 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 02af70d360bc..4dd5920d5769 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -240,6 +240,7 @@ static char *pkt_flag_names[] = {
 #define PKTGEN_MAGIC 0xbe9be955
 #define PG_PROC_DIR "pktgen"
 #define PGCTRL	    "pgctrl"
+#define PG_THREAD_PREFIX "kpktgend_"
 
 #define MAX_CFLOWS  65536
 
@@ -416,6 +417,7 @@ struct pktgen_dev {
 				  */
 	netdevice_tracker dev_tracker;
 	char odevname[32];
+	char procname[32];	/* the name of ->entry in /proc/net/pktgen */
 	struct flow_state *flows;
 	unsigned int cflows;	/* Concurrent flows (config) */
 	unsigned int lflow;		/* Flow length  (config) */
@@ -2044,6 +2046,34 @@ static bool __pktgen_NN_threads(const struct pktgen_net *pn,
 	return false;
 }
 
+/*
+ * A device can be named like pgctrl or a thread file. Threads are still
+ * being created while the first thread file is writable, so match thread
+ * names by prefix. Called with pktgen_thread_lock held.
+ */
+static bool pktgen_proc_name_taken(const struct pktgen_net *pn,
+				   const char *name)
+{
+	const struct pktgen_thread *t;
+	const struct pktgen_dev *pkt_dev;
+
+	lockdep_assert_held(&pktgen_thread_lock);
+
+	if (!strcmp(name, PGCTRL) || str_has_prefix(name, PG_THREAD_PREFIX))
+		return true;
+	rcu_read_lock();
+	list_for_each_entry(t, &pn->pktgen_threads, th_list) {
+		list_for_each_entry_rcu(pkt_dev, &t->if_list, list) {
+			if (pkt_dev->entry && !strcmp(name, pkt_dev->procname)) {
+				rcu_read_unlock();
+				return true;
+			}
+		}
+	}
+	rcu_read_unlock();
+	return false;
+}
+
 /*
  * mark a device for removal
  */
@@ -2091,13 +2121,22 @@ static void pktgen_change_name(const struct pktgen_net *pn, struct net_device *d
 			if (pkt_dev->odev != dev)
 				continue;
 
+			/* renamed back to the name the file kept */
+			if (pkt_dev->entry && !strcmp(pkt_dev->procname, dev->name))
+				break;
+			if (pktgen_proc_name_taken(pn, dev->name)) {
+				pr_err("can't move proc entry for '%s', the name is taken\n",
+				       dev->name);
+				break;
+			}
 			proc_remove(pkt_dev->entry);
-
 			pkt_dev->entry = proc_create_data(dev->name, 0600,
 							  pn->proc_dir,
 							  &pktgen_if_proc_ops,
 							  pkt_dev);
-			if (!pkt_dev->entry)
+			if (pkt_dev->entry)
+				strscpy(pkt_dev->procname, dev->name);
+			else
 				pr_err("can't move proc entry for '%s'\n",
 				       dev->name);
 			break;
@@ -3841,6 +3880,10 @@ static int pktgen_add_device(struct pktgen_thread *t, const char *ifname)
 		pr_err("ERROR: interface already used\n");
 		return -EBUSY;
 	}
+	if (pktgen_proc_name_taken(t->net, ifname)) {
+		pr_err("ERROR: %s/%s is taken\n", PG_PROC_DIR, ifname);
+		return -EEXIST;
+	}
 
 	pkt_dev = kzalloc_node(sizeof(struct pktgen_dev), GFP_KERNEL, node);
 	if (!pkt_dev)
@@ -3888,6 +3931,7 @@ static int pktgen_add_device(struct pktgen_thread *t, const char *ifname)
 		err = -EINVAL;
 		goto out2;
 	}
+	strscpy(pkt_dev->procname, ifname);
 #ifdef CONFIG_XFRM
 	pkt_dev->ipsmode = XFRM_MODE_TRANSPORT;
 	pkt_dev->ipsproto = IPPROTO_ESP;
@@ -3937,7 +3981,8 @@ static int __net_init pktgen_create_thread(int cpu, struct pktgen_net *pn)
 	list_add_tail(&t->th_list, &pn->pktgen_threads);
 	init_completion(&t->start_done);
 
-	p = kthread_create_on_cpu(pktgen_thread_worker, t, cpu, "kpktgend_%d");
+	p = kthread_create_on_cpu(pktgen_thread_worker, t, cpu,
+				  PG_THREAD_PREFIX "%d");
 	if (IS_ERR(p)) {
 		pr_err("kthread_create_on_node() failed for cpu %d\n", t->cpu);
 		list_del(&t->th_list);
-- 
2.53.0


--