Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id ; Thu, 2 Jan 2003 04:28:13 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id ; Thu, 2 Jan 2003 04:28:13 -0500 Received: from dp.samba.org ([66.70.73.150]:54209 "EHLO lists.samba.org") by vger.kernel.org with ESMTP id ; Thu, 2 Jan 2003 04:28:08 -0500 From: Rusty Russell To: torvalds@transmeta.com Cc: linux-kernel@vger.kernel.org, akpm@zip.com.au, Thomas Sailer , Marcel Holtmann , Jose Orlando Pereira , J.E.J.Bottomley@HansenPartnership.com Subject: [PATCH] Deprecate exec_usermodehelper, fix request_module. Date: Thu, 02 Jan 2003 20:36:05 +1100 Message-Id: <20030102093637.8C6892C2FB@lists.samba.org> Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 10969 Lines: 375 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 #include +#include #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 December 2000 -*/ + Fixed request_module to use call_usermodehelper path. Jan 2003. + Rusty Russell +*/ #define __KERNEL_SYSCALLS__ #include @@ -31,6 +33,7 @@ #include #include #include +#include #include 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(¤t->sig->siglock); - tmpsig = current->blocked; - siginitsetinv(¤t->blocked, sigmask(SIGKILL) | sigmask(SIGSTOP)); - recalc_sigpending(); - spin_unlock_irq(¤t->sig->siglock); - - waitpid_result = waitpid(pid, NULL, __WCLONE); - atomic_dec(&kmod_concurrent); - - /* Allow signals again.. */ - spin_lock_irq(¤t->sig->siglock); - current->blocked = tmpsig; - recalc_sigpending(); - spin_unlock_irq(¤t->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); - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/