sysctl: deprecate sys_sysctl in a user space visible fashion.

After adding checking to register_sysctl_table and finding a whole new set of bugs. Missed by countless code reviews and testers I have finally lost patience with the binary sysctl interface. The binary sysctl interface has been sort of deprecated for years and finding a user space program that uses the syscall is more difficult then finding a needle in a haystack. Problems continue to crop up, with the in kernel implementation. So since supporting something that no one uses is silly, deprecate sys_sysctl with a sufficient grace period and notice that the handful of user space applications that care can be fixed or replaced. The /proc/sys sysctl interface that people use will continue to be supported indefinitely. This patch moves the tested warning about sysctls from the path where sys_sysctl to a separate path called from both implementations of sys_sysctl, and it adds a proper entry into Documentation/feature-removal-schedule. Allowing us to revisit this in a couple years time and actually kill sys_sysctl. [lethal@linux-sh.org: sysctl: Fix syscall disabled build] Signed-off-by: Eric W. Biederman <ebiederm@xmission.com> Signed-off-by: Paul Mundt <lethal@linux-sh.org> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

sysctl: deprecate sys_sysctl in a user space visible fashion.
After adding checking to register_sysctl_table and finding a whole new set of bugs. Missed by countless code reviews and testers I have finally lost patience with the binary sysctl interface. The binary sysctl interface has been sort of deprecated for years and finding a user space program that uses the syscall is more difficult then finding a needle in a haystack. Problems continue to crop up, with the in kernel implementation. So since supporting something that no one uses is silly, deprecate sys_sysctl with a sufficient grace period and notice that the handful of user space applications that care can be fixed or replaced. The /proc/sys sysctl interface that people use will continue to be supported indefinitely. This patch moves the tested warning about sysctls from the path where sys_sysctl to a separate path called from both implementations of sys_sysctl, and it adds a proper entry into Documentation/feature-removal-schedule. Allowing us to revisit this in a couple years time and actually kill sys_sysctl. [lethal@linux-sh.org: sysctl: Fix syscall disabled build] Signed-off-by: Eric W. Biederman <ebiederm@xmission.com> Signed-off-by: Paul Mundt <lethal@linux-sh.org> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Eric W. Biederman · Linus Torvalds
1 parent 8ada720d89
Showing 2 changed files with 76 additions and 23 deletions Side-by-side Diff
Documentation/feature-removal-schedule.txt
kernel/sysctl.c
@@ -82,6 +82,41 @@
  
 ---------------------------
  
+What:	sys_sysctl
+When:	September 2010
+Option: CONFIG_SYSCTL_SYSCALL
+Why:	The same information is available in a more convenient from
+	/proc/sys, and none of the sysctl variables appear to be
+	important performance wise.
+
+	Binary sysctls are a long standing source of subtle kernel
+	bugs and security issues.
+
+	When I looked several months ago all I could find after
+	searching several distributions were 5 user space programs and
+	glibc (which falls back to /proc/sys) using this syscall.
+
+	The man page for sysctl(2) documents it as unusable for user
+	space programs.
+
+	sysctl(2) is not generally ABI compatible to a 32bit user
+	space application on a 64bit and a 32bit kernel.
+
+	For the last several months the policy has been no new binary
+	sysctls and no one has put forward an argument to use them.
+
+	Binary sysctls issues seem to keep happening appearing so
+	properly deprecating them (with a warning to user space) and a
+	2 year grace warning period will mean eventually we can kill
+	them and end the pain.
+
+	In the mean time individual binary sysctls can be dealt with
+	in a piecewise fashion.
+
+Who:	Eric Biederman <ebiederm@xmission.com>
+
+---------------------------
+
 What:  a.out interpreter support for ELF executables
 When:  2.6.25
 Files: fs/binfmt_elf.c
@@ -55,6 +55,8 @@
 #include <asm/stacktrace.h>
 #endif
  
+static int deprecated_sysctl_warning(struct __sysctl_args *args);
+
 #if defined(CONFIG_SYSCTL)
  
 /* External variables not in a header file. */
  
@@ -1347,10 +1349,15 @@
 	if (copy_from_user(&tmp, args, sizeof(tmp)))
 		return -EFAULT;
  
+	error = deprecated_sysctl_warning(&tmp);
+	if (error)
+		goto out;
+
 	lock_kernel();
 	error = do_sysctl(tmp.name, tmp.nlen, tmp.oldval, tmp.oldlenp,
 			  tmp.newval, tmp.newlen);
 	unlock_kernel();
+out:
 	return error;
 }
 #endif /* CONFIG_SYSCTL_SYSCALL */
  
  
  
  
  
@@ -2540,35 +2547,19 @@
  
 asmlinkage long sys_sysctl(struct __sysctl_args __user *args)
 {
-	static int msg_count;
 	struct __sysctl_args tmp;
-	int name[CTL_MAXNAME];
-	int i;
+	int error;
  
-	/* Read in the sysctl name for better debug message logging */
 	if (copy_from_user(&tmp, args, sizeof(tmp)))
 		return -EFAULT;
-	if (tmp.nlen <= 0 || tmp.nlen >= CTL_MAXNAME)
-		return -ENOTDIR;
-	for (i = 0; i < tmp.nlen; i++)
-		if (get_user(name[i], tmp.name + i))
-			return -EFAULT;
  
-	/* Ignore accesses to kernel.version */
-	if ((tmp.nlen == 2) && (name[0] == CTL_KERN) && (name[1] == KERN_VERSION))
-		goto out;
+	error = deprecated_sysctl_warning(&tmp);
  
-	if (msg_count < 5) {
-		msg_count++;
-		printk(KERN_INFO
-			"warning: process `%s' used the removed sysctl "
-			"system call with ", current->comm);
-		for (i = 0; i < tmp.nlen; i++)
-			printk("%d.", name[i]);
-		printk("\n");
-	}
-out:
-	return -ENOSYS;
+	/* If no error reading the parameters then just -ENOSYS ... */
+	if (!error)
+		error = -ENOSYS;
+
+	return error;
 }
  
 int sysctl_data(struct ctl_table *table, int __user *name, int nlen,
@@ -2607,6 +2598,33 @@
 }
  
 #endif /* CONFIG_SYSCTL_SYSCALL */
+
+static int deprecated_sysctl_warning(struct __sysctl_args *args)
+{
+	static int msg_count;
+	int name[CTL_MAXNAME];
+	int i;
+
+	/* Read in the sysctl name for better debug message logging */
+	for (i = 0; i < args->nlen; i++)
+		if (get_user(name[i], args->name + i))
+			return -EFAULT;
+
+	/* Ignore accesses to kernel.version */
+	if ((args->nlen == 2) && (name[0] == CTL_KERN) && (name[1] == KERN_VERSION))
+		return 0;
+
+	if (msg_count < 5) {
+		msg_count++;
+		printk(KERN_INFO
+			"warning: process `%s' used the deprecated sysctl "
+			"system call with ", current->comm);
+		for (i = 0; i < args->nlen; i++)
+			printk("%d.", name[i]);
+		printk("\n");
+	}
+	return 0;
+}
  
 /*
  * No sense putting this after each symbol definition, twice,
...	...	@@ -82,6 +82,41 @@
82	82
83	83	---------------------------
84	84
	85	+What: sys_sysctl
	86	+When: September 2010
	87	+Option: CONFIG_SYSCTL_SYSCALL
	88	+Why: The same information is available in a more convenient from
	89	+ /proc/sys, and none of the sysctl variables appear to be
	90	+ important performance wise.
	91	+
	92	+ Binary sysctls are a long standing source of subtle kernel
	93	+ bugs and security issues.
	94	+
	95	+ When I looked several months ago all I could find after
	96	+ searching several distributions were 5 user space programs and
	97	+ glibc (which falls back to /proc/sys) using this syscall.
	98	+
	99	+ The man page for sysctl(2) documents it as unusable for user
	100	+ space programs.
	101	+
	102	+ sysctl(2) is not generally ABI compatible to a 32bit user
	103	+ space application on a 64bit and a 32bit kernel.
	104	+
	105	+ For the last several months the policy has been no new binary
	106	+ sysctls and no one has put forward an argument to use them.
	107	+
	108	+ Binary sysctls issues seem to keep happening appearing so
	109	+ properly deprecating them (with a warning to user space) and a
	110	+ 2 year grace warning period will mean eventually we can kill
	111	+ them and end the pain.
	112	+
	113	+ In the mean time individual binary sysctls can be dealt with
	114	+ in a piecewise fashion.
	115	+
	116	+Who: Eric Biederman <ebiederm@xmission.com>
	117	+
	118	+---------------------------
	119	+
85	120	What: a.out interpreter support for ELF executables
86	121	When: 2.6.25
87	122	Files: fs/binfmt_elf.c
...	...	@@ -55,6 +55,8 @@
55	55	#include <asm/stacktrace.h>
56	56	#endif
57	57
	58	+static int deprecated_sysctl_warning(struct __sysctl_args *args);
	59	+
58	60	#if defined(CONFIG_SYSCTL)
59	61
60	62	/* External variables not in a header file. */
61	63
...	...	@@ -1347,10 +1349,15 @@
1347	1349	if (copy_from_user(&tmp, args, sizeof(tmp)))
1348	1350	return -EFAULT;
1349	1351
	1352	+ error = deprecated_sysctl_warning(&tmp);
	1353	+ if (error)
	1354	+ goto out;
	1355	+
1350	1356	lock_kernel();
1351	1357	error = do_sysctl(tmp.name, tmp.nlen, tmp.oldval, tmp.oldlenp,
1352	1358	tmp.newval, tmp.newlen);
1353	1359	unlock_kernel();
	1360	+out:
1354	1361	return error;
1355	1362	}
1356	1363	#endif /* CONFIG_SYSCTL_SYSCALL */
1357	1364
1358	1365
1359	1366
1360	1367
1361	1368
...	...	@@ -2540,35 +2547,19 @@
2540	2547
2541	2548	asmlinkage long sys_sysctl(struct __sysctl_args __user *args)
2542	2549	{
2543		- static int msg_count;
2544	2550	struct __sysctl_args tmp;
2545		- int name[CTL_MAXNAME];
2546		- int i;
	2551	+ int error;
2547	2552
2548		- /* Read in the sysctl name for better debug message logging */
2549	2553	if (copy_from_user(&tmp, args, sizeof(tmp)))
2550	2554	return -EFAULT;
2551		- if (tmp.nlen <= 0 \|\| tmp.nlen >= CTL_MAXNAME)
2552		- return -ENOTDIR;
2553		- for (i = 0; i < tmp.nlen; i++)
2554		- if (get_user(name[i], tmp.name + i))
2555		- return -EFAULT;
2556	2555
2557		- /* Ignore accesses to kernel.version */
2558		- if ((tmp.nlen == 2) && (name[0] == CTL_KERN) && (name[1] == KERN_VERSION))
2559		- goto out;
	2556	+ error = deprecated_sysctl_warning(&tmp);
2560	2557
2561		- if (msg_count < 5) {
2562		- msg_count++;
2563		- printk(KERN_INFO
2564		- "warning: process `%s' used the removed sysctl "
2565		- "system call with ", current->comm);
2566		- for (i = 0; i < tmp.nlen; i++)
2567		- printk("%d.", name[i]);
2568		- printk("\n");
2569		- }
2570		-out:
2571		- return -ENOSYS;
	2558	+ /* If no error reading the parameters then just -ENOSYS ... */
	2559	+ if (!error)
	2560	+ error = -ENOSYS;
	2561	+
	2562	+ return error;
2572	2563	}
2573	2564
2574	2565	int sysctl_data(struct ctl_table table, int __user name, int nlen,
...	...	@@ -2607,6 +2598,33 @@
2607	2598	}
2608	2599
2609	2600	#endif /* CONFIG_SYSCTL_SYSCALL */
	2601	+
	2602	+static int deprecated_sysctl_warning(struct __sysctl_args *args)
	2603	+{
	2604	+ static int msg_count;
	2605	+ int name[CTL_MAXNAME];
	2606	+ int i;
	2607	+
	2608	+ /* Read in the sysctl name for better debug message logging */
	2609	+ for (i = 0; i < args->nlen; i++)
	2610	+ if (get_user(name[i], args->name + i))
	2611	+ return -EFAULT;
	2612	+
	2613	+ /* Ignore accesses to kernel.version */
	2614	+ if ((args->nlen == 2) && (name[0] == CTL_KERN) && (name[1] == KERN_VERSION))
	2615	+ return 0;
	2616	+
	2617	+ if (msg_count < 5) {
	2618	+ msg_count++;
	2619	+ printk(KERN_INFO
	2620	+ "warning: process `%s' used the deprecated sysctl "
	2621	+ "system call with ", current->comm);
	2622	+ for (i = 0; i < args->nlen; i++)
	2623	+ printk("%d.", name[i]);
	2624	+ printk("\n");
	2625	+ }
	2626	+ return 0;
	2627	+}
2610	2628
2611	2629	/*
2612	2630	* No sense putting this after each symbol definition, twice,