[PATCH] Fix sysctl unregistration oops (CVE-2005-2709)

You could open the /proc/sys/net/ipv4/conf/<if>/<whatever> file, then wait for interface to go away, try to grab as much memory as possible in hope to hit the (kfreed) ctl_table. Then fill it with pointers to your function. Then do read from file you've opened and if you are lucky, you'll get it called as ->proc_handler() in kernel mode. So this is at least an Oops and possibly more. It does depend on an interface going away though, so less of a security risk than it would otherwise be. Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> Signed-off-by: Linus Torvalds <torvalds@osdl.org>

[PATCH] Fix sysctl unregistration oops (CVE-2005-2709)
You could open the /proc/sys/net/ipv4/conf/<if>/<whatever> file, then wait for interface to go away, try to grab as much memory as possible in hope to hit the (kfreed) ctl_table. Then fill it with pointers to your function. Then do read from file you've opened and if you are lucky, you'll get it called as ->proc_handler() in kernel mode. So this is at least an Oops and possibly more. It does depend on an interface going away though, so less of a security risk than it would otherwise be. Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> Signed-off-by: Linus Torvalds <torvalds@osdl.org>
Al Viro · Linus Torvalds
1 parent 8546df6f35
Showing 4 changed files with 116 additions and 31 deletions Side-by-side Diff
arch/s390/appldata/appldata_base.c
include/linux/proc_fs.h
include/linux/sysctl.h
kernel/sysctl.c
@@ -592,12 +592,15 @@
  */
 void appldata_unregister_ops(struct appldata_ops *ops)
 {
+	void *table;
 	spin_lock(&appldata_ops_lock);
-	unregister_sysctl_table(ops->sysctl_header);
 	list_del(&ops->list);
-	kfree(ops->ctl_table);
+	/* at that point any incoming access will fail */
+	table = ops->ctl_table;
 	ops->ctl_table = NULL;
 	spin_unlock(&appldata_ops_lock);
+	unregister_sysctl_table(ops->sysctl_header);
+	kfree(table);
 	P_INFO("%s-ops unregistered!\n", ops->name);
 }
 /********************** module-ops management <END> **************************/
@@ -66,6 +66,7 @@
 	write_proc_t *write_proc;
 	atomic_t count;		/* use count */
 	int deleted;		/* delete flag */
+	void *set;
 };
  
 struct kcore_list {
@@ -24,6 +24,7 @@
 #include <linux/compiler.h>
  
 struct file;
+struct completion;
  
 #define CTL_MAXNAME 10		/* how many path components do we allow in a
 				   call to sysctl?   In other words, what is
@@ -925,6 +926,8 @@
 {
 	ctl_table *ctl_table;
 	struct list_head ctl_entry;
+	int used;
+	struct completion *unregistering;
 };
  
 struct ctl_table_header * register_sysctl_table(ctl_table * table, 
@@ -169,7 +169,7 @@
  
 extern struct proc_dir_entry *proc_sys_root;
  
-static void register_proc_table(ctl_table *, struct proc_dir_entry *);
+static void register_proc_table(ctl_table *, struct proc_dir_entry *, void *);
 static void unregister_proc_table(ctl_table *, struct proc_dir_entry *);
 #endif
  
  
@@ -992,10 +992,51 @@
  
 extern void init_irq_proc (void);
  
+static DEFINE_SPINLOCK(sysctl_lock);
+
+/* called under sysctl_lock */
+static int use_table(struct ctl_table_header *p)
+{
+	if (unlikely(p->unregistering))
+		return 0;
+	p->used++;
+	return 1;
+}
+
+/* called under sysctl_lock */
+static void unuse_table(struct ctl_table_header *p)
+{
+	if (!--p->used)
+		if (unlikely(p->unregistering))
+			complete(p->unregistering);
+}
+
+/* called under sysctl_lock, will reacquire if has to wait */
+static void start_unregistering(struct ctl_table_header *p)
+{
+	/*
+	 * if p->used is 0, nobody will ever touch that entry again;
+	 * we'll eliminate all paths to it before dropping sysctl_lock
+	 */
+	if (unlikely(p->used)) {
+		struct completion wait;
+		init_completion(&wait);
+		p->unregistering = &wait;
+		spin_unlock(&sysctl_lock);
+		wait_for_completion(&wait);
+		spin_lock(&sysctl_lock);
+	}
+	/*
+	 * do not remove from the list until nobody holds it; walking the
+	 * list in do_sysctl() relies on that.
+	 */
+	list_del_init(&p->ctl_entry);
+}
+
 void __init sysctl_init(void)
 {
 #ifdef CONFIG_PROC_FS
-	register_proc_table(root_table, proc_sys_root);
+	register_proc_table(root_table, proc_sys_root, &root_table_header);
 	init_irq_proc();
 #endif
 }
@@ -1004,6 +1045,7 @@
 	       void __user *newval, size_t newlen)
 {
 	struct list_head *tmp;
+	int error = -ENOTDIR;
  
 	if (nlen <= 0 || nlen >= CTL_MAXNAME)
 		return -ENOTDIR;
  
  
  
@@ -1012,20 +1054,30 @@
 		if (!oldlenp || get_user(old_len, oldlenp))
 			return -EFAULT;
 	}
+	spin_lock(&sysctl_lock);
 	tmp = &root_table_header.ctl_entry;
 	do {
 		struct ctl_table_header *head =
 			list_entry(tmp, struct ctl_table_header, ctl_entry);
 		void *context = NULL;
-		int error = parse_table(name, nlen, oldval, oldlenp, 
+
+		if (!use_table(head))
+			continue;
+
+		spin_unlock(&sysctl_lock);
+
+		error = parse_table(name, nlen, oldval, oldlenp, 
 					newval, newlen, head->ctl_table,
 					&context);
 		kfree(context);
+
+		spin_lock(&sysctl_lock);
+		unuse_table(head);
 		if (error != -ENOTDIR)
-			return error;
-		tmp = tmp->next;
-	} while (tmp != &root_table_header.ctl_entry);
-	return -ENOTDIR;
+			break;
+	} while ((tmp = tmp->next) != &root_table_header.ctl_entry);
+	spin_unlock(&sysctl_lock);
+	return error;
 }
  
 asmlinkage long sys_sysctl(struct __sysctl_args __user *args)
  
  
@@ -1236,12 +1288,16 @@
 		return NULL;
 	tmp->ctl_table = table;
 	INIT_LIST_HEAD(&tmp->ctl_entry);
+	tmp->used = 0;
+	tmp->unregistering = NULL;
+	spin_lock(&sysctl_lock);
 	if (insert_at_head)
 		list_add(&tmp->ctl_entry, &root_table_header.ctl_entry);
 	else
 		list_add_tail(&tmp->ctl_entry, &root_table_header.ctl_entry);
+	spin_unlock(&sysctl_lock);
 #ifdef CONFIG_PROC_FS
-	register_proc_table(table, proc_sys_root);
+	register_proc_table(table, proc_sys_root, tmp);
 #endif
 	return tmp;
 }
  
@@ -1255,10 +1311,13 @@
  */
 void unregister_sysctl_table(struct ctl_table_header * header)
 {
-	list_del(&header->ctl_entry);
+	might_sleep();
+	spin_lock(&sysctl_lock);
+	start_unregistering(header);
 #ifdef CONFIG_PROC_FS
 	unregister_proc_table(header->ctl_table, proc_sys_root);
 #endif
+	spin_unlock(&sysctl_lock);
 	kfree(header);
 }
  
@@ -1269,7 +1328,7 @@
 #ifdef CONFIG_PROC_FS
  
 /* Scan the sysctl entries in table and add them all into /proc */
-static void register_proc_table(ctl_table * table, struct proc_dir_entry *root)
+static void register_proc_table(ctl_table * table, struct proc_dir_entry *root, void *set)
 {
 	struct proc_dir_entry *de;
 	int len;
  
@@ -1305,13 +1364,14 @@
 			de = create_proc_entry(table->procname, mode, root);
 			if (!de)
 				continue;
+			de->set = set;
 			de->data = (void *) table;
 			if (table->proc_handler)
 				de->proc_fops = &proc_sys_file_operations;
 		}
 		table->de = de;
 		if (de->mode & S_IFDIR)
-			register_proc_table(table->child, de);
+			register_proc_table(table->child, de, set);
 	}
 }
  
@@ -1336,6 +1396,13 @@
 				continue;
 		}
  
+		/*
+		 * In any case, mark the entry as goner; we'll keep it
+		 * around if it's busy, but we'll know to do nothing with
+		 * its fields.  We are under sysctl_lock here.
+		 */
+		de->data = NULL;
+
 		/* Don't unregister proc entries that are still being used.. */
 		if (atomic_read(&de->count))
 			continue;
  
  
@@ -1349,27 +1416,38 @@
 			  size_t count, loff_t *ppos)
 {
 	int op;
-	struct proc_dir_entry *de;
+	struct proc_dir_entry *de = PDE(file->f_dentry->d_inode);
 	struct ctl_table *table;
 	size_t res;
-	ssize_t error;
+	ssize_t error = -ENOTDIR;
  
-	de = PDE(file->f_dentry->d_inode);
-	if (!de || !de->data)
-		return -ENOTDIR;
-	table = (struct ctl_table *) de->data;
-	if (!table || !table->proc_handler)
-		return -ENOTDIR;
-	op = (write ? 002 : 004);
-	if (ctl_perm(table, op))
-		return -EPERM;
-	
-	res = count;
-
-	error = (*table->proc_handler) (table, write, file, buf, &res, ppos);
-	if (error)
-		return error;
-	return res;
+	spin_lock(&sysctl_lock);
+	if (de && de->data && use_table(de->set)) {
+		/*
+		 * at that point we know that sysctl was not unregistered
+		 * and won't be until we finish
+		 */
+		spin_unlock(&sysctl_lock);
+		table = (struct ctl_table *) de->data;
+		if (!table || !table->proc_handler)
+			goto out;
+		error = -EPERM;
+		op = (write ? 002 : 004);
+		if (ctl_perm(table, op))
+			goto out;
+		
+		/* careful: calling conventions are nasty here */
+		res = count;
+		error = (*table->proc_handler)(table, write, file,
+						buf, &res, ppos);
+		if (!error)
+			error = res;
+	out:
+		spin_lock(&sysctl_lock);
+		unuse_table(de->set);
+	}
+	spin_unlock(&sysctl_lock);
+	return error;
 }
  
 static int proc_opensys(struct inode *inode, struct file *file)
...	...	@@ -592,12 +592,15 @@
592	592	*/
593	593	void appldata_unregister_ops(struct appldata_ops *ops)
594	594	{
	595	+ void *table;
595	596	spin_lock(&appldata_ops_lock);
596		- unregister_sysctl_table(ops->sysctl_header);
597	597	list_del(&ops->list);
598		- kfree(ops->ctl_table);
	598	+ /* at that point any incoming access will fail */
	599	+ table = ops->ctl_table;
599	600	ops->ctl_table = NULL;
600	601	spin_unlock(&appldata_ops_lock);
	602	+ unregister_sysctl_table(ops->sysctl_header);
	603	+ kfree(table);
601	604	P_INFO("%s-ops unregistered!\n", ops->name);
602	605	}
603	606	/******************** module-ops management <END> ************************/
...	...	@@ -66,6 +66,7 @@
66	66	write_proc_t *write_proc;
67	67	atomic_t count; /* use count */
68	68	int deleted; /* delete flag */
	69	+ void *set;
69	70	};
70	71
71	72	struct kcore_list {
...	...	@@ -24,6 +24,7 @@
24	24	#include <linux/compiler.h>
25	25
26	26	struct file;
	27	+struct completion;
27	28
28	29	#define CTL_MAXNAME 10 /* how many path components do we allow in a
29	30	call to sysctl? In other words, what is
...	...	@@ -925,6 +926,8 @@
925	926	{
926	927	ctl_table *ctl_table;
927	928	struct list_head ctl_entry;
	929	+ int used;
	930	+ struct completion *unregistering;
928	931	};
929	932
930	933	struct ctl_table_header * register_sysctl_table(ctl_table * table,
...	...	@@ -169,7 +169,7 @@
169	169
170	170	extern struct proc_dir_entry *proc_sys_root;
171	171
172		-static void register_proc_table(ctl_table , struct proc_dir_entry );
	172	+static void register_proc_table(ctl_table , struct proc_dir_entry , void *);
173	173	static void unregister_proc_table(ctl_table , struct proc_dir_entry );
174	174	#endif
175	175
176	176
...	...	@@ -992,10 +992,51 @@
992	992
993	993	extern void init_irq_proc (void);
994	994
	995	+static DEFINE_SPINLOCK(sysctl_lock);
	996	+
	997	+/* called under sysctl_lock */
	998	+static int use_table(struct ctl_table_header *p)
	999	+{
	1000	+ if (unlikely(p->unregistering))
	1001	+ return 0;
	1002	+ p->used++;
	1003	+ return 1;
	1004	+}
	1005	+
	1006	+/* called under sysctl_lock */
	1007	+static void unuse_table(struct ctl_table_header *p)
	1008	+{
	1009	+ if (!--p->used)
	1010	+ if (unlikely(p->unregistering))
	1011	+ complete(p->unregistering);
	1012	+}
	1013	+
	1014	+/* called under sysctl_lock, will reacquire if has to wait */
	1015	+static void start_unregistering(struct ctl_table_header *p)
	1016	+{
	1017	+ /*
	1018	+ * if p->used is 0, nobody will ever touch that entry again;
	1019	+ * we'll eliminate all paths to it before dropping sysctl_lock
	1020	+ */
	1021	+ if (unlikely(p->used)) {
	1022	+ struct completion wait;
	1023	+ init_completion(&wait);
	1024	+ p->unregistering = &wait;
	1025	+ spin_unlock(&sysctl_lock);
	1026	+ wait_for_completion(&wait);
	1027	+ spin_lock(&sysctl_lock);
	1028	+ }
	1029	+ /*
	1030	+ * do not remove from the list until nobody holds it; walking the
	1031	+ * list in do_sysctl() relies on that.
	1032	+ */
	1033	+ list_del_init(&p->ctl_entry);
	1034	+}
	1035	+
995	1036	void __init sysctl_init(void)
996	1037	{
997	1038	#ifdef CONFIG_PROC_FS
998		- register_proc_table(root_table, proc_sys_root);
	1039	+ register_proc_table(root_table, proc_sys_root, &root_table_header);
999	1040	init_irq_proc();
1000	1041	#endif
1001	1042	}
...	...	@@ -1004,6 +1045,7 @@
1004	1045	void __user *newval, size_t newlen)
1005	1046	{
1006	1047	struct list_head *tmp;
	1048	+ int error = -ENOTDIR;
1007	1049
1008	1050	if (nlen <= 0 \|\| nlen >= CTL_MAXNAME)
1009	1051	return -ENOTDIR;
1010	1052
1011	1053
1012	1054
...	...	@@ -1012,20 +1054,30 @@
1012	1054	if (!oldlenp \|\| get_user(old_len, oldlenp))
1013	1055	return -EFAULT;
1014	1056	}
	1057	+ spin_lock(&sysctl_lock);
1015	1058	tmp = &root_table_header.ctl_entry;
1016	1059	do {
1017	1060	struct ctl_table_header *head =
1018	1061	list_entry(tmp, struct ctl_table_header, ctl_entry);
1019	1062	void *context = NULL;
1020		- int error = parse_table(name, nlen, oldval, oldlenp,
	1063	+
	1064	+ if (!use_table(head))
	1065	+ continue;
	1066	+
	1067	+ spin_unlock(&sysctl_lock);
	1068	+
	1069	+ error = parse_table(name, nlen, oldval, oldlenp,
1021	1070	newval, newlen, head->ctl_table,
1022	1071	&context);
1023	1072	kfree(context);
	1073	+
	1074	+ spin_lock(&sysctl_lock);
	1075	+ unuse_table(head);
1024	1076	if (error != -ENOTDIR)
1025		- return error;
1026		- tmp = tmp->next;
1027		- } while (tmp != &root_table_header.ctl_entry);
1028		- return -ENOTDIR;
	1077	+ break;
	1078	+ } while ((tmp = tmp->next) != &root_table_header.ctl_entry);
	1079	+ spin_unlock(&sysctl_lock);
	1080	+ return error;
1029	1081	}
1030	1082
1031	1083	asmlinkage long sys_sysctl(struct __sysctl_args __user *args)
1032	1084
1033	1085
...	...	@@ -1236,12 +1288,16 @@
1236	1288	return NULL;
1237	1289	tmp->ctl_table = table;
1238	1290	INIT_LIST_HEAD(&tmp->ctl_entry);
	1291	+ tmp->used = 0;
	1292	+ tmp->unregistering = NULL;
	1293	+ spin_lock(&sysctl_lock);
1239	1294	if (insert_at_head)
1240	1295	list_add(&tmp->ctl_entry, &root_table_header.ctl_entry);
1241	1296	else
1242	1297	list_add_tail(&tmp->ctl_entry, &root_table_header.ctl_entry);
	1298	+ spin_unlock(&sysctl_lock);
1243	1299	#ifdef CONFIG_PROC_FS
1244		- register_proc_table(table, proc_sys_root);
	1300	+ register_proc_table(table, proc_sys_root, tmp);
1245	1301	#endif
1246	1302	return tmp;
1247	1303	}
1248	1304
...	...	@@ -1255,10 +1311,13 @@
1255	1311	*/
1256	1312	void unregister_sysctl_table(struct ctl_table_header * header)
1257	1313	{
1258		- list_del(&header->ctl_entry);
	1314	+ might_sleep();
	1315	+ spin_lock(&sysctl_lock);
	1316	+ start_unregistering(header);
1259	1317	#ifdef CONFIG_PROC_FS
1260	1318	unregister_proc_table(header->ctl_table, proc_sys_root);
1261	1319	#endif
	1320	+ spin_unlock(&sysctl_lock);
1262	1321	kfree(header);
1263	1322	}
1264	1323
...	...	@@ -1269,7 +1328,7 @@
1269	1328	#ifdef CONFIG_PROC_FS
1270	1329
1271	1330	/* Scan the sysctl entries in table and add them all into /proc */
1272		-static void register_proc_table(ctl_table * table, struct proc_dir_entry *root)
	1331	+static void register_proc_table(ctl_table * table, struct proc_dir_entry root, void set)
1273	1332	{
1274	1333	struct proc_dir_entry *de;
1275	1334	int len;
1276	1335
...	...	@@ -1305,13 +1364,14 @@
1305	1364	de = create_proc_entry(table->procname, mode, root);
1306	1365	if (!de)
1307	1366	continue;
	1367	+ de->set = set;
1308	1368	de->data = (void *) table;
1309	1369	if (table->proc_handler)
1310	1370	de->proc_fops = &proc_sys_file_operations;
1311	1371	}
1312	1372	table->de = de;
1313	1373	if (de->mode & S_IFDIR)
1314		- register_proc_table(table->child, de);
	1374	+ register_proc_table(table->child, de, set);
1315	1375	}
1316	1376	}
1317	1377
...	...	@@ -1336,6 +1396,13 @@
1336	1396	continue;
1337	1397	}
1338	1398
	1399	+ /*
	1400	+ * In any case, mark the entry as goner; we'll keep it
	1401	+ * around if it's busy, but we'll know to do nothing with
	1402	+ * its fields. We are under sysctl_lock here.
	1403	+ */
	1404	+ de->data = NULL;
	1405	+
1339	1406	/* Don't unregister proc entries that are still being used.. */
1340	1407	if (atomic_read(&de->count))
1341	1408	continue;
1342	1409
1343	1410
...	...	@@ -1349,27 +1416,38 @@
1349	1416	size_t count, loff_t *ppos)
1350	1417	{
1351	1418	int op;
1352		- struct proc_dir_entry *de;
	1419	+ struct proc_dir_entry *de = PDE(file->f_dentry->d_inode);
1353	1420	struct ctl_table *table;
1354	1421	size_t res;
1355		- ssize_t error;
	1422	+ ssize_t error = -ENOTDIR;
1356	1423
1357		- de = PDE(file->f_dentry->d_inode);
1358		- if (!de \|\| !de->data)
1359		- return -ENOTDIR;
1360		- table = (struct ctl_table *) de->data;
1361		- if (!table \|\| !table->proc_handler)
1362		- return -ENOTDIR;
1363		- op = (write ? 002 : 004);
1364		- if (ctl_perm(table, op))
1365		- return -EPERM;
1366		-
1367		- res = count;
1368		-
1369		- error = (*table->proc_handler) (table, write, file, buf, &res, ppos);
1370		- if (error)
1371		- return error;
1372		- return res;
	1424	+ spin_lock(&sysctl_lock);
	1425	+ if (de && de->data && use_table(de->set)) {
	1426	+ /*
	1427	+ * at that point we know that sysctl was not unregistered
	1428	+ * and won't be until we finish
	1429	+ */
	1430	+ spin_unlock(&sysctl_lock);
	1431	+ table = (struct ctl_table *) de->data;
	1432	+ if (!table \|\| !table->proc_handler)
	1433	+ goto out;
	1434	+ error = -EPERM;
	1435	+ op = (write ? 002 : 004);
	1436	+ if (ctl_perm(table, op))
	1437	+ goto out;
	1438	+
	1439	+ /* careful: calling conventions are nasty here */
	1440	+ res = count;
	1441	+ error = (*table->proc_handler)(table, write, file,
	1442	+ buf, &res, ppos);
	1443	+ if (!error)
	1444	+ error = res;
	1445	+ out:
	1446	+ spin_lock(&sysctl_lock);
	1447	+ unuse_table(de->set);
	1448	+ }
	1449	+ spin_unlock(&sysctl_lock);
	1450	+ return error;
1373	1451	}
1374	1452
1375	1453	static int proc_opensys(struct inode inode, struct file file)