[Users] 2.6.18-194.8.1.el5.028stab070.2 (was: [Announce] New kernel released: RHEL5 testing 2.6.18-194.8.1.el5 028stab070.2)

[Solar Designer]

Kir,

Since you set Reply-To on these announcements to point to the users
list, I thought I'd reply in public. ;-)

On Sat, Jul 10, 2010 at 07:37:11PM +0400, Kir Kolyshkin wrote:
> OpenVZ project has released an updated RHEL5 based
> testing kernel. Read below for more information.

Thank you for starting to make these kernels available - this is helpful
for our project.

> Since 028stab069.6:
> * Rebased to 194.8.1.el5 kernel (security fixes, see links below)
> * Fixed some memory leaks in timer and ipv4 code
> * Added dummy /proc/sys/kernel/hotplug file to CTs to make udev happy
> * Backported support for /proc/*/stack to query task's stack trace
> * Fixed a set of bugs in bonding driver
> * A compilation fix (#1425)

I've interdiff'ed 069.6 vs. 070.2 and looked through the changes.  Here
are a few curious ones I spotted, which were not clear from the list
above and from the Red Hat advisory and CVEs:

--- linux-2.6.18.ovz/fs/ioprio.c	2010-05-29 12:46:59.000000000 +0400
+++ linux-2.6.18.ovz/fs/ioprio.c	2010-07-08 20:01:52.000000000 +0400
@@ -92,7 +92,7 @@
 			if (!who)
 				p = current;
 			else
-				p = find_task_by_pid_all(who);
+				p = find_task_by_pid_ve(who);
 			if (p)
 				ret = set_task_ioprio(p, ioprio);
 			break;

This could have been nasty, but this is sys_ioprio_set(), which starts with:

	if (!ve_is_super(get_exec_env()))
		return -EPERM;

So I guess we were OK either way?

diff -u linux-2.6.18.ovz/fs/proc/base.c linux-2.6.18.ovz/fs/proc/base.c
--- linux-2.6.18.ovz/fs/proc/base.c	2010-05-29 12:46:59.000000000 +0400
+++ linux-2.6.18.ovz/fs/proc/base.c	2010-07-08 20:01:52.000000000 +0400
[...]
@@ -260,6 +262,9 @@
 #ifdef CONFIG_TASK_IO_ACCOUNTING
 	E(PROC_TGID_IO,             "io",  S_IFREG|S_IRUGO),
 #endif
+#ifdef CONFIG_STACKTRACE_PROC
+	E(PROC_TGID_STACK,     "stack",   S_IFREG|S_IRUGO),
+#endif
 
 	{0,0,NULL,0}
 };
@@ -307,6 +312,9 @@
 #ifdef CONFIG_TASK_IO_ACCOUNTING
 	E(PROC_TID_IO,         "io",      S_IFREG|S_IRUGO),
 #endif
+#ifdef CONFIG_STACKTRACE_PROC
+	E(PROC_TID_STACK,      "stack",   S_IFREG|S_IRUGO),
+#endif
 
 	{0,0,NULL,0}
 };

S_IRUGO looks risky.  For the stack, it could mean ASLR bypass and
infoleaks.  For I/O, it could mean subtle infoleaks (via precise timing
of I/O stats changes).  Luckily, for the stack this is dealt with:

@@ -587,6 +595,43 @@
 }
 #endif /* CONFIG_KALLSYMS */
 
+static int proc_pid_stack(struct task_struct *task, char *buffer)
+{
+	struct stack_trace trace;
+	unsigned long *entries;
+	int i, ret = 0;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;

Yet it'd feel cleaner to restrict the perms.

For I/O, the issue appears to be there (I did not check for it on a
running kernel, though).

I recommend that you harden the perms on both kinds of /proc entries.
Of course, this is not OpenVZ-specific.

diff -u linux-2.6.18.ovz/kernel/sysctl.c linux-2.6.18.ovz/kernel/sysctl.c
--- linux-2.6.18.ovz/kernel/sysctl.c	2010-05-29 12:47:01.000000000 +0400
+++ linux-2.6.18.ovz/kernel/sysctl.c	2010-07-08 20:01:52.000000000 +0400
@@ -635,8 +635,10 @@
 		.data		= &uevent_helper,
 		.maxlen		= UEVENT_HELPER_PATH_LEN,
 		.mode		= 0644,
-		.proc_handler	= &proc_dostring,
-		.strategy	= &sysctl_string,
+		.proc_handler	= &proc_dostring_ve_immutable,
+		.strategy	= &sysctl_string_ve_immutable,
+		.extra1		= "",
+		.virt_handler	= 1,
 	},
 #endif
 #ifdef CONFIG_CHR_DEV_SG

What was the impact of not doing it before?

Thanks again,

Alexander