All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rusty Russell <rusty@rustcorp.com.au>
To: torvalds@transmeta.com
Cc: linux-kernel@vger.kernel.org, akpm@zip.com.au,
	Thomas Sailer <sailer@ife.ee.ethz.ch>,
	Marcel Holtmann <marcel@holtmann.org>,
	Jose Orlando Pereira <jop@di.uminho.pt>,
	J.E.J.Bottomley@HansenPartnership.com
Subject: [PATCH] Deprecate exec_usermodehelper, fix request_module.
Date: Thu, 02 Jan 2003 20:36:05 +1100	[thread overview]
Message-ID: <20030102093637.8C6892C2FB@lists.samba.org> (raw)

I got a report from Urban Widmark that modprobe dropped its privs on
executing an install command: turns out request_module
(ie. exec_usermodehelper) doesn't set the real uid or gid (so bash
drops privs).

These efforts to "clean" the current process are *always* going to be
buggy: we should use the event thread all the time, rather than
forking a random thread and trying to clean it.  This fixes
request_module to do that (kevent threads can't block, so we
double-fork).

There are still three
(obscure) users of exec_usermodehelper in the tree:

	drivers/net/hamradio/baycom_epp.c
	drivers/bluetooth/bt3c_cs.c
	arch/i386/mach-voyager/voyager_thread.c

Any comments? (Patch size due to some required reshuffling).
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Name: Make request_module use call_usermodehelper
Author: Rusty Russell
Status: Tested on 2.5.54

D: Urban Widmark points out that modprobe calls system() in many
D: configurations, which drops privs since request_module() doesn't
D: doesn't set ruid and rguid.
D:
D: Use a known-clean environment (as call_usermodehelper does).
D: Deprecate exec_usermodehelper, since it's probably buggy, maybe
D: exploitable.

diff -urNp --exclude TAGS -X /home/rusty/current-dontdiff --minimal linux-2.5.54/include/linux/kmod.h working-2.5.54-kmod/include/linux/kmod.h
--- linux-2.5.54/include/linux/kmod.h	Thu Jan  2 12:35:15 2003
+++ working-2.5.54-kmod/include/linux/kmod.h	Thu Jan  2 16:44:17 2003
@@ -21,6 +21,7 @@
 
 #include <linux/config.h>
 #include <linux/errno.h>
+#include <linux/compiler.h>
 
 #ifdef CONFIG_KMOD
 extern int request_module(const char * name);
@@ -29,7 +30,7 @@ static inline int request_module(const c
 #endif
 
 #define try_then_request_module(x, mod) ((x) ?: request_module(mod), (x))
-extern int exec_usermodehelper(char *program_path, char *argv[], char *envp[]);
+extern int exec_usermodehelper(char *program_path, char *argv[], char *envp[])  __deprecated;
 extern int call_usermodehelper(char *path, char *argv[], char *envp[]);
 
 #ifdef CONFIG_HOTPLUG
diff -urNp --exclude TAGS -X /home/rusty/current-dontdiff --minimal linux-2.5.54/kernel/kmod.c working-2.5.54-kmod/kernel/kmod.c
--- linux-2.5.54/kernel/kmod.c	Thu Jan  2 12:37:03 2003
+++ working-2.5.54-kmod/kernel/kmod.c	Thu Jan  2 20:15:52 2003
@@ -14,8 +14,10 @@
 
 	Unblock all signals when we exec a usermode process.
 	Shuu Yamaguchi <shuu@wondernetworkresources.com> December 2000
-*/
 
+	Fixed request_module to use call_usermodehelper path.  Jan 2003.
+	Rusty Russell <rusty@rustcorp.com.au>
+*/
 #define __KERNEL_SYSCALLS__
 
 #include <linux/config.h>
@@ -31,6 +33,7 @@
 #include <linux/workqueue.h>
 #include <linux/security.h>
 #include <linux/mount.h>
+#include <linux/kernel.h>
 #include <asm/uaccess.h>
 
 extern int max_threads, system_running;
@@ -141,41 +144,87 @@ int exec_usermodehelper(char *program_pa
 	set_fs(KERNEL_DS);
 
 	/* Go, go, go... */
+	printk("Execing %s\n", program_path);
 	if (execve(program_path, argv, envp) < 0)
 		return -errno;
 	return 0;
 }
 
-#ifdef CONFIG_KMOD
+struct subprocess_info {
+	struct completion *complete;
+	char *path;
+	char **argv;
+	char **envp;
+	int wait;
+	pid_t retval;
+};
 
 /*
-	modprobe_path is set via /proc/sys.
-*/
-char modprobe_path[256] = "/sbin/modprobe";
+ * This is the task which runs the usermode application
+ */
+static int ____call_usermodehelper(void *data)
+{
+	struct subprocess_info *sub_info = data;
+	int retval;
 
-static int exec_modprobe(void * module_name)
+	retval = -EPERM;
+	if (current->fs->root)
+		retval = exec_usermodehelper(sub_info->path, sub_info->argv, sub_info->envp);
+
+	/* Exec failed? */
+	sub_info->retval = (pid_t)retval;
+	do_exit(0);
+}
+
+/* Keventd can't block, but this (a child) can. */
+static int wait_for_helper(void *data)
 {
-	static char * envp[] = { "HOME=/", "TERM=linux", "PATH=/sbin:/usr/sbin:/bin:/usr/bin", NULL };
-	char *argv[] = { modprobe_path, "--", (char*)module_name, NULL };
-	int ret;
+	struct subprocess_info *sub_info = data;
+	pid_t pid;
 
-	if (!system_running)
-		return -EBUSY;
+	pid = kernel_thread(____call_usermodehelper, sub_info,
+			    CLONE_VFORK | SIGCHLD);
+	if (pid < 0)
+		sub_info->retval = pid;
+	else
+		sys_wait4(pid, NULL, 0, NULL);
 
-	ret = exec_usermodehelper(modprobe_path, argv, envp);
-	if (ret) {
-		static unsigned long last;
-		unsigned long now = jiffies;
-		if (now - last > HZ) {
-			last = now;
-			printk(KERN_DEBUG
-			       "kmod: failed to exec %s -s -k %s, errno = %d\n",
-			       modprobe_path, (char*) module_name, errno);
-		}
-	}
-	return ret;
+	complete(sub_info->complete);
+	return 0;
+}
+
+/*
+ * This is run by keventd.
+ */
+static void __call_usermodehelper(void *data)
+{
+	struct subprocess_info *sub_info = data;
+	pid_t pid;
+
+	/* CLONE_VFORK: wait until the usermode helper has execve'd
+	 * successfully We need the data structures to stay around
+	 * until that is done.  */
+	if (sub_info->wait)
+		pid = kernel_thread(wait_for_helper, sub_info,
+				    CLONE_KERNEL | SIGCHLD);
+	else
+		pid = kernel_thread(____call_usermodehelper, sub_info,
+				    CLONE_VFORK | SIGCHLD);
+
+	if (pid < 0) {
+		sub_info->retval = pid;
+		complete(sub_info->complete);
+	} else if (!sub_info->wait)
+		complete(sub_info->complete);
 }
 
+#ifdef CONFIG_KMOD
+
+/*
+	modprobe_path is set via /proc/sys.
+*/
+char modprobe_path[256] = "/sbin/modprobe";
+
 /**
  * request_module - try to load a kernel module
  * @module_name: Name of module
@@ -189,24 +238,33 @@ static int exec_modprobe(void * module_n
  * If module auto-loading support is disabled then this function
  * becomes a no-operation.
  */
-int request_module(const char * module_name)
+int request_module(const char *module_name)
 {
-	pid_t pid;
-	int waitpid_result;
-	sigset_t tmpsig;
-	int i, ret;
+	unsigned int max_modprobes;
+	DECLARE_COMPLETION(done);
+	char *argv[] = { modprobe_path, "--", (char*)module_name, NULL };
+	static char *envp[] = { "HOME=/",
+				"TERM=linux",
+				"PATH=/sbin:/usr/sbin:/bin:/usr/bin",
+				NULL };
+	struct subprocess_info sub_info = {
+		.complete	= &done,
+		.path		= modprobe_path,
+		.argv		= argv,
+		.envp		= envp,
+		.wait		= 1, /* We need to wait. */
+		.retval		= 0,
+	};
+	DECLARE_WORK(work, __call_usermodehelper, &sub_info);
 	static atomic_t kmod_concurrent = ATOMIC_INIT(0);
 #define MAX_KMOD_CONCURRENT 50	/* Completely arbitrary value - KAO */
 	static int kmod_loop_msg;
-	unsigned long saved_policy = current->policy;
 
-	current->policy = SCHED_NORMAL;
-	/* Don't allow request_module() when the system isn't set up */
-	if ( ! system_running ) {
-		printk(KERN_ERR "request_module[%s]: not ready\n", module_name);
-		ret = -EPERM;
-		goto out;
-	}
+	if (!system_running)
+		return -EBUSY;
+
+	if (modprobe_path[0] == '\0')
+		return 0;
 
 	/* If modprobe needs a service that is in a module, we get a recursive
 	 * loop.  Limit the number of running kmod threads to max_threads/2 or
@@ -216,52 +274,39 @@ int request_module(const char * module_n
 	 * process tables to get the command line, proc_pid_cmdline is static
 	 * and it is not worth changing the proc code just to handle this case. 
 	 * KAO.
+	 *
+
+	 * "trace the ppid" is simple, but will fail if someone's
+	 * parent exits.  I think this is as good as it gets. --RR
 	 */
-	i = max_threads/2;
-	if (i > MAX_KMOD_CONCURRENT)
-		i = MAX_KMOD_CONCURRENT;
+	max_modprobes = min(max_threads/2, MAX_KMOD_CONCURRENT);
 	atomic_inc(&kmod_concurrent);
-	if (atomic_read(&kmod_concurrent) > i) {
+	if (atomic_read(&kmod_concurrent) > max_modprobes) {
+		/* We may be blaming an innocent here, but unlikely */
 		if (kmod_loop_msg++ < 5)
 			printk(KERN_ERR
-			       "kmod: runaway modprobe loop assumed and stopped\n");
-		atomic_dec(&kmod_concurrent);
-		ret = -ENOMEM;
-		goto out;
-	}
-
-	pid = kernel_thread(exec_modprobe, (void*) module_name, 0);
-	if (pid < 0) {
-		printk(KERN_ERR "request_module[%s]: fork failed, errno %d\n", module_name, -pid);
+			       "request_module: runaway loop modprobe %s\n",
+			       module_name);
 		atomic_dec(&kmod_concurrent);
-		ret = pid;
-		goto out;
+		return -ENOMEM;
 	}
 
-	/* Block everything but SIGKILL/SIGSTOP */
-	spin_lock_irq(&current->sig->siglock);
-	tmpsig = current->blocked;
-	siginitsetinv(&current->blocked, sigmask(SIGKILL) | sigmask(SIGSTOP));
-	recalc_sigpending();
-	spin_unlock_irq(&current->sig->siglock);
-
-	waitpid_result = waitpid(pid, NULL, __WCLONE);
-	atomic_dec(&kmod_concurrent);
-
-	/* Allow signals again.. */
-	spin_lock_irq(&current->sig->siglock);
-	current->blocked = tmpsig;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sig->siglock);
+	schedule_work(&work);
+	wait_for_completion(&done);
 
-	if (waitpid_result != pid) {
-		printk(KERN_ERR "request_module[%s]: waitpid(%d,...) failed, errno %d\n",
-		       module_name, pid, -waitpid_result);
+	/* This is exec failing, not modprobe failing. */
+	if (sub_info.retval < 0) {
+		static unsigned long last;
+		unsigned long now = jiffies;
+		if (now - last > HZ) {
+			last = now;
+			printk(KERN_DEBUG
+			       "request_module: failed %s -- %s. error = %d\n",
+			       modprobe_path, module_name, sub_info.retval);
+		}
 	}
-	ret = 0;
-out:
-	current->policy = saved_policy;
-	return ret;
+	atomic_dec(&kmod_concurrent);
+	return sub_info.retval;
 }
 #endif /* CONFIG_KMOD */
 
@@ -289,49 +334,6 @@ EXPORT_SYMBOL(hotplug_path);
 
 #endif /* CONFIG_HOTPLUG */
 
-struct subprocess_info {
-	struct completion *complete;
-	char *path;
-	char **argv;
-	char **envp;
-	pid_t retval;
-};
-
-/*
- * This is the task which runs the usermode application
- */
-static int ____call_usermodehelper(void *data)
-{
-	struct subprocess_info *sub_info = data;
-	int retval;
-
-	retval = -EPERM;
-	if (current->fs->root)
-		retval = exec_usermodehelper(sub_info->path, sub_info->argv, sub_info->envp);
-
-	/* Exec failed? */
-	sub_info->retval = (pid_t)retval;
-	do_exit(0);
-}
-
-/*
- * This is run by keventd.
- */
-static void __call_usermodehelper(void *data)
-{
-	struct subprocess_info *sub_info = data;
-	pid_t pid;
-
-	/*
-	 * CLONE_VFORK: wait until the usermode helper has execve'd successfully
-	 * We need the data structures to stay around until that is done.
-	 */
-	pid = kernel_thread(____call_usermodehelper, sub_info, CLONE_VFORK | SIGCHLD);
-	if (pid < 0)
-		sub_info->retval = pid;
-	complete(sub_info->complete);
-}
-
 /**
  * call_usermodehelper - start a usermode application
  * @path: pathname for the application
@@ -353,6 +355,7 @@ int call_usermodehelper(char *path, char
 		.path		= path,
 		.argv		= argv,
 		.envp		= envp,
+		.wait		= 0,
 		.retval		= 0,
 	};
 	DECLARE_WORK(work, __call_usermodehelper, &sub_info);

             reply	other threads:[~2003-01-02  9:28 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-01-02  9:36 Rusty Russell [this message]
2003-01-02 17:14 ` [PATCH] Deprecate exec_usermodehelper, fix request_module Marcel Holtmann
2003-01-02 19:08   ` Thomas Sailer
2003-01-03  5:08     ` Rusty Russell
2003-01-03  6:21       ` Brad Hards
2003-01-03  7:25         ` Jeff Garzik
2003-01-03  7:36         ` Andrew Morton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20030102093637.8C6892C2FB@lists.samba.org \
    --to=rusty@rustcorp.com.au \
    --cc=J.E.J.Bottomley@HansenPartnership.com \
    --cc=akpm@zip.com.au \
    --cc=jop@di.uminho.pt \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marcel@holtmann.org \
    --cc=sailer@ife.ee.ethz.ch \
    --cc=torvalds@transmeta.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.