Hello!
It seems I have found some kind of race, if hotplug is enabled,
and one have changing number of network interfaces (observed on ppp).
I use 2.4.0-test11. Problem is 100% repeatable.
My test case is this: I start ppp over ssh session, then do ifconfig
and then kill pppd (by kill -9 pid).
Immediatelly I getting: 1. waitpid(<somepid>) failed, -512 2. oops.
Inspecting oops, I have found that
current->files == NULL in kmod.c::exec_usermodehelper.
After some investigations, it appears, that when pppd exits,
ppp interface gets destroyed, and /sbin/hotplug net is called,
using now dead pppd as parent.
Simple (and probably not that good) solution is attached as patch, below.
It get gid of oops, but waitpid failed thing is still there.
Also I post oops here, too. (sorry, no Code decoding)
--- kmod.c.orig Mon Dec 4 15:31:38 2000
+++ kmod.c Mon Dec 4 15:32:25 2000
@@ -99,8 +99,10 @@
flush_signal_handlers(current);
spin_unlock_irq(¤t->sigmask_lock);
- for (i = 0; i < current->files->max_fds; i++ ) {
- if (current->files->fd[i]) close(i);
+ if ( current->files ) {
+ for (i = 0; i < current->files->max_fds; i++ ) {
+ if (current->files->fd[i]) close(i);
+ }
}
/* Drop the "current user" thing */
Unable to handle kernel NULL pointer dereference at virtual address 00000008
c01122a4
*pde = 00000000
Oops: 0000
CPU: 0
EIP: 0010:[<c01122a4>]
Using defaults from ksymoops -t elf32-i386 -a i386
EFLAGS: 00010246
eax: c7acea64 ebx: c7a20000 ecx: c1292000 edx: 00000000
esi: 00000006 edi: 00000000 ebp: c1228320 esp: c7a21fb8
ds: 0018 es: 0018 ss: 0018
Process pppd (pid: 181, stackpage=c7a21000)
Stack: 00000100 c7c43d84 c7c43db4 c7a76b20 c7b8c0c0 c1228320 c7f87a60 c7f87a60
c7f87a60 c7f87a60 c011259c c01d7ba0 c7c43e44 c7c43e30 c01073f8 c7c43db4
00000078 c7c43e44
Call Trace: [<c011259c>] [<c01073f8>]
Code: 8b 4f 08 39 ca 7d 22 90 8b 47 14 83 3c 90 00 74 13 89 f0 89
>>EIP; c01122a4 <exec_usermodehelper+298/3d8> <=====
Trace; c011259c <request_module+1b8/1bc>
Trace; c01073f8 <kernel_thread+28/1b8>
Bye,
Oleg
Oleg Drokin wrote:
>
> Hello!
>
> It seems I have found some kind of race, if hotplug is enabled,
> and one have changing number of network interfaces (observed on ppp).
> I use 2.4.0-test11. Problem is 100% repeatable.
>
> My test case is this: I start ppp over ssh session, then do ifconfig
> and then kill pppd (by kill -9 pid).
> Immediatelly I getting: 1. waitpid(<somepid>) failed, -512 2. oops.
> Inspecting oops, I have found that
> current->files == NULL in kmod.c::exec_usermodehelper.
> After some investigations, it appears, that when pppd exits,
> ppp interface gets destroyed, and /sbin/hotplug net is called,
> using now dead pppd as parent.
> Simple (and probably not that good) solution is attached as patch, below.
> It get gid of oops, but waitpid failed thing is still there.
Yup. This is due to the kernel trying to fork/exec from within
a half-dead process.
Could you please test this patch? It was sent to Linus for
test12-pre4, but....
--- linux-2.4.0-test12-pre4/include/linux/sched.h Mon Dec 4 21:07:13 2000
+++ linux-akpm/include/linux/sched.h Tue Dec 5 00:36:08 2000
@@ -150,6 +150,7 @@
asmlinkage void schedule(void);
extern void schedule_task(struct tq_struct *task);
+extern int start_context_thread(void);
/*
* The default fd array needs to be at least BITS_PER_LONG,
--- linux-2.4.0-test12-pre4/kernel/kmod.c Tue Nov 21 20:11:21 2000
+++ linux-akpm/kernel/kmod.c Tue Dec 5 00:36:08 2000
@@ -165,7 +165,7 @@
int request_module(const char * module_name)
{
- int pid;
+ pid_t pid;
int waitpid_result;
sigset_t tmpsig;
int i;
@@ -259,40 +259,121 @@
static int exec_helper (void *arg)
{
+ long ret;
void **params = (void **) arg;
char *path = (char *) params [0];
char **argv = (char **) params [1];
char **envp = (char **) params [2];
- return exec_usermodehelper (path, argv, envp);
+
+ ret = exec_usermodehelper (path, argv, envp);
+ if (ret < 0)
+ ret = -ret;
+ do_exit(ret);
}
+struct subprocess_info {
+ struct semaphore *sem;
+ char *path;
+ char **argv;
+ char **envp;
+ int retval;
+};
-int call_usermodehelper (char *path, char **argv, char **envp)
+/*
+ * This is a standalone child of keventd. It forks off another thread which
+ * is the desired usermode helper and then waits for the child to exit.
+ * We return the usermode process's exit code, or some -ve error code.
+ */
+static int ____call_usermodehelper(void *data)
{
- void *params [3] = { path, argv, envp };
- int pid, pid2, retval;
+ struct subprocess_info *sub_info = data;
+ struct task_struct *curtask = current;
+ void *params [3] = { sub_info->path, sub_info->argv, sub_info->envp };
+ pid_t pid, pid2;
mm_segment_t fs;
+ int retval = 0;
- if ( ! current->fs->root ) {
- printk(KERN_ERR "call_usermodehelper[%s]: no root fs\n",
- path);
- return -EPERM;
- }
- if ((pid = kernel_thread (exec_helper, (void *) params, 0)) < 0) {
- printk(KERN_ERR "failed fork %s, errno = %d", argv [0], -pid);
- return -1;
- }
+ if (!curtask->fs->root) {
+ printk(KERN_ERR "call_usermodehelper[%s]: no root fs\n", sub_info->path);
+ retval = -EPERM;
+ goto up_and_out;
+ }
+ if ((pid = kernel_thread(exec_helper, (void *) params, 0)) < 0) {
+ printk(KERN_ERR "failed fork2 %s, errno = %d", sub_info->argv[0], -pid);
+ retval = pid;
+ goto up_and_out;
+ }
+
+ if (retval >= 0) {
+ /* Block everything but SIGKILL/SIGSTOP */
+ spin_lock_irq(&curtask->sigmask_lock);
+ siginitsetinv(&curtask->blocked, sigmask(SIGKILL) | sigmask(SIGSTOP));
+ recalc_sigpending(curtask);
+ spin_unlock_irq(&curtask->sigmask_lock);
+
+ /* Allow the system call to access kernel memory */
+ fs = get_fs();
+ set_fs(KERNEL_DS);
+ pid2 = waitpid(pid, &retval, __WCLONE);
+ if (pid2 == -1 && errno < 0)
+ pid2 = errno;
+ set_fs(fs);
+
+ if (pid2 != pid) {
+ printk(KERN_ERR "waitpid(%d) failed, %d\n", pid, pid2);
+ retval = (pid2 < 0) ? pid2 : -1;
+ }
+ }
+
+up_and_out:
+ sub_info->retval = retval;
+ curtask->exit_signal = SIGCHLD; /* Wake up parent */
+ up_and_exit(sub_info->sem, retval);
+}
- fs = get_fs ();
- set_fs (KERNEL_DS);
- pid2 = waitpid (pid, &retval, __WCLONE);
- set_fs (fs);
+/*
+ * This is a schedule_task function, so we must not sleep for very long at all.
+ * But the exec'ed process could do anything at all. So we launch another
+ * kernel thread.
+ */
+static void __call_usermodehelper(void *data)
+{
+ struct subprocess_info *sub_info = data;
+ pid_t pid;
- if (pid2 != pid) {
- printk(KERN_ERR "waitpid(%d) failed, %d\n", pid, pid2);
- return -1;
+ if ((pid = kernel_thread (____call_usermodehelper, (void *)sub_info, 0)) < 0) {
+ printk(KERN_ERR "failed fork1 %s, errno = %d", sub_info->argv[0], -pid);
+ sub_info->retval = pid;
+ up(sub_info->sem);
}
- return retval;
+}
+
+/*
+ * This function can be called via do_exit->__exit_files, which means that
+ * we're partway through exitting and things break if we fork children.
+ * So we use keventd to parent the usermode helper.
+ * We return the usermode application's exit code or some -ve error.
+ */
+int call_usermodehelper (char *path, char **argv, char **envp)
+{
+ DECLARE_MUTEX_LOCKED(sem);
+ struct subprocess_info sub_info = {
+ sem: &sem,
+ path: path,
+ argv: argv,
+ envp: envp,
+ retval: 0,
+ };
+ struct tq_struct tqs = {
+ next: 0,
+ sync: 0,
+ routine: __call_usermodehelper,
+ data: &sub_info,
+ };
+
+ schedule_task(&tqs);
+ down(&sem); /* Wait for an error or completion */
+ return sub_info.retval;
}
EXPORT_SYMBOL(exec_usermodehelper);
--- linux-2.4.0-test12-pre4/kernel/context.c Tue Nov 21 20:11:21 2000
+++ linux-akpm/kernel/context.c Tue Dec 5 00:36:08 2000
@@ -6,16 +6,23 @@
* [email protected]
*/
+#define __KERNEL_SYSCALLS__
+
#include <linux/module.h>
#include <linux/kernel.h>
#include <linux/sched.h>
#include <linux/init.h>
+#include <linux/unistd.h>
+#include <linux/signal.h>
static DECLARE_TASK_QUEUE(tq_context);
static DECLARE_WAIT_QUEUE_HEAD(context_task_wq);
+static int keventd_running;
void schedule_task(struct tq_struct *task)
{
+ if (keventd_running == 0)
+ printk(KERN_ERR "schedule_task(): keventd has not started\n");
queue_task(task, &tq_context);
wake_up(&context_task_wq);
}
@@ -24,18 +31,27 @@
static int context_thread(void *dummy)
{
- DECLARE_WAITQUEUE(wait, current);
+ struct task_struct *curtask = current;
+ DECLARE_WAITQUEUE(wait, curtask);
+ struct k_sigaction sa;
daemonize();
- strcpy(current->comm, "eventd");
+ strcpy(curtask->comm, "keventd");
+ keventd_running = 1;
- spin_lock_irq(¤t->sigmask_lock);
- sigfillset(¤t->blocked);
- recalc_sigpending(current);
- spin_unlock_irq(¤t->sigmask_lock);
+ spin_lock_irq(&curtask->sigmask_lock);
+ siginitsetinv(&curtask->blocked, sigmask(SIGCHLD));
+ recalc_sigpending(curtask);
+ spin_unlock_irq(&curtask->sigmask_lock);
+
+ /* Install a handler so SIGCLD is delivered */
+ sa.sa.sa_handler = SIG_IGN;
+ sa.sa.sa_flags = 0;
+ siginitset(&sa.sa.sa_mask, sigmask(SIGCHLD));
+ do_sigaction(SIGCHLD, &sa, (struct k_sigaction *)0);
for (;;) {
- current->state = TASK_INTERRUPTIBLE;
+ __set_task_state(curtask, TASK_INTERRUPTIBLE);
add_wait_queue(&context_task_wq, &wait);
/*
@@ -46,15 +62,19 @@
schedule();
remove_wait_queue(&context_task_wq, &wait);
- current->state = TASK_RUNNING;
+ __set_task_state(curtask, TASK_RUNNING);
run_task_queue(&tq_context);
+ if (signal_pending(curtask)) {
+ while (waitpid(-1, (unsigned int *)0, __WALL|WNOHANG) > 0)
+ ;
+ flush_signals(curtask);
+ recalc_sigpending(curtask);
+ }
}
}
-static int __init start_context_thread(void)
+int start_context_thread(void)
{
kernel_thread(context_thread, NULL, CLONE_FS | CLONE_FILES | CLONE_SIGHAND);
return 0;
}
-
-module_init(start_context_thread);
--- linux-2.4.0-test12-pre4/init/main.c Mon Dec 4 21:07:13 2000
+++ linux-akpm/init/main.c Tue Dec 5 00:36:08 2000
@@ -700,6 +700,7 @@
else mount_initrd =0;
#endif
+ start_context_thread();
do_initcalls();
/* .. filesystems .. */
@@ -712,16 +713,16 @@
init_pcmcia_ds(); /* Do this last */
#endif
-#ifdef CONFIG_HOTPLUG
+ /* Mount the root filesystem.. */
+ mount_root();
+
+#if defined(CONFIG_HOTPLUG) && defined(CONFIG_NET)
/* do this after other 'do this last' stuff, because we want
* to minimize spurious executions of /sbin/hotplug
* during boot-up
*/
net_notifier_init();
#endif
-
- /* Mount the root filesystem.. */
- mount_root();
mount_devfs_fs ();
Hello!
On Tue, Dec 05, 2000 at 12:42:05AM +1100, Andrew Morton wrote:
> Could you please test this patch? It was sent to Linus for
> test12-pre4, but....
Yes, it works that way.
But code path and readability became horrible, unfortunatelly. :(
Bye,
Oleg