[patch] unblock signals in exec_usermodehelper

From: Andrew Morton (andrewm@uow.edu.au)
Date: Mon Dec 18 2000 - 10:28:44 EST


Linus,

Shuu Yamaguchi has found an interesting problem. When 8139too
is managed by /sbin/hotplug, 8139too's kernel thread becomes
unkillable. Attempts to close the interface cause ifconfig
to get stuck in D state, etc...

This is because keventd blocks all signals except SIGCHLD.

keventd parents /sbin/hotplug, which parents ifconfig,
which parents the 8139too kernel thread. The net
result is that the 8139too kernel thread blocks
SIGTERM via its current->blocked. SIGTERM is supposed
to stop it.

Obviously, 8139too should be more defensive about
its environment and I will address that in a separate
patch.

However I suggest that the kernel should be running usermode
processes in a more predictable environment, so this
patch simply unblocks all signals and calls recalc_sigpending()
just prior to doing the execve(). The rest of this patch
is just the standard optimisation of caching `current'
in a local variable.

This affects modprobe as well. Currently, modprobe
is executed with basically a random current->blocked.
It gets whatever its parent gives it, and its parent
can be anyone who autoloads a module.

Generally, the recent proliferation of kernel threads
and signal handlers in the kernel is causing us some
trouble. I suggest that we soon move to a scheme
where kernel threads are _always_ parented by keventd.
This is very simple to do.

For example, I have a bad feeling that if user Fred
has CAP_NET_ADMIN and he opens the 8139too device,
the 8139too thread will run with uid=Fred. If user
Bill also has CAP_NET_ADMIN, Bill won't be able to close
the device if he can't send SIGTERM to Fred's process.

For another example, I suspect that if a process
opens the 8139too device and then, for some other
reason does a waitpid(), and then someone closes
the device, the original opener will receive
notification of the exit of a child which it didn't
know it had.

--- linux-2.4.0-test13-pre3/kernel/kmod.c Tue Dec 12 19:24:23 2000
+++ linux-akpm/kernel/kmod.c Tue Dec 19 01:43:07 2000
@@ -11,6 +11,9 @@
         Limit the concurrent number of kmod modprobes to catch loops from
         "modprobe needs a service that is in a module".
         Keith Owens <kaos@ocs.com.au> December 1999
+
+ Unblock all signals when we exec a usermode process.
+ Shuu Yamaguchi <shuu@wondernetworkresources.com> December 2000
 */
 
 #define __KERNEL_SYSCALLS__
@@ -83,9 +86,10 @@
 int exec_usermodehelper(char *program_path, char *argv[], char *envp[])
 {
         int i;
+ struct task_struct *curtask = current;
 
- current->session = 1;
- current->pgrp = 1;
+ curtask->session = 1;
+ curtask->pgrp = 1;
 
         use_init_fs_context();
 
@@ -95,19 +99,21 @@
            as the super user right after the execve fails if you time
            the signal just right.
         */
- spin_lock_irq(&current->sigmask_lock);
- flush_signals(current);
- flush_signal_handlers(current);
- spin_unlock_irq(&current->sigmask_lock);
+ spin_lock_irq(&curtask->sigmask_lock);
+ sigemptyset(&curtask->blocked);
+ flush_signals(curtask);
+ flush_signal_handlers(curtask);
+ recalc_sigpending(curtask);
+ spin_unlock_irq(&curtask->sigmask_lock);
 
- for (i = 0; i < current->files->max_fds; i++ ) {
- if (current->files->fd[i]) close(i);
+ for (i = 0; i < curtask->files->max_fds; i++ ) {
+ if (curtask->files->fd[i]) close(i);
         }
 
         /* Drop the "current user" thing */
         {
- struct user_struct *user = current->user;
- current->user = INIT_USER;
+ struct user_struct *user = curtask->user;
+ curtask->user = INIT_USER;
                 atomic_inc(&INIT_USER->__count);
                 atomic_inc(&INIT_USER->processes);
                 atomic_dec(&user->processes);
@@ -115,9 +121,9 @@
         }
 
         /* Give kmod all effective privileges.. */
- current->euid = current->fsuid = 0;
- current->egid = current->fsgid = 0;
- cap_set_full(current->cap_effective);
+ curtask->euid = curtask->fsuid = 0;
+ curtask->egid = curtask->fsgid = 0;
+ cap_set_full(curtask->cap_effective);
 
         /* Allow execve args to be in kernel space. */
         set_fs(KERNEL_DS);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Sat Dec 23 2000 - 21:00:21 EST