2000-12-18 15:54:37

by Andrew Morton

[permalink] [raw]
Subject: [patch] unblock signals in exec_usermodehelper

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 <[email protected]> December 1999
+
+ Unblock all signals when we exec a usermode process.
+ Shuu Yamaguchi <[email protected]> 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);