Skip to contentSkip to conversation

OOps in exec_usermodehelper

DISCUSSION Petr Vandrovec 2 participants 2 messages Nov 28, 2000 – Nov 29, 2000

lore
Share & tools
lore

Petr Vandrovec

author
#0 lore raw
Quoted 4 lines · Andrew Morton

hmm.. Quite a few things fixed here.

Could you please test this patch? It's against 2.4.0-test12-pre2, should be OK against test11.

I upgraded to 12-pre2 already ;-) It looks like that it works.

Quoted 5 lines

- keventd is now capable of reaping dead children. I will be all ears if someone can tell me how to get a kernel thread to accept signals without having to install a handler with

    sa.sa_handler = (__sighandler_t)100;

Is there any reason why not set sa.sa_handler to SIG_IGN? According to arch/i386/kernel/signal.c:do_signal(), signal manpage and my memory, kernel does automatic child reaping in such configuration. So you could even remove waitpid() loop from keventd. I did not tried it yet, but it looks like obvious solution...

BTW, are you sure that kernel does not try to deliver SIGSEGV to keventd() when signal arrives. It looks like that it should, but it probably fails somewhere during way due to non-existent userspace for this process.

Review

Quoted 2 lines
(quoted diff) +2 -0
+   sa.sa.sa_handler = (__sighandler_t)100;     /* Surely there's a better way? */
+   sa.sa.sa_flags = 0;

SA_RESTART? SA_NOCLDSTOP ?

Quoted 11 lines
(quoted diff) +2 -1
+   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 +59,19 @@
            schedule();

Review line 69

Quoted 10 lines
(quoted diff) +7 -1
        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);
+       }

Best regards,
Petr Vandrovec [email protected]

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

#1 lore raw
Quoted 9 lines · Petr Vandrovec
Quoted 4 lines · Andrew Morton

hmm.. Quite a few things fixed here.

Could you please test this patch? It's against 2.4.0-test12-pre2, should be OK against test11.

I upgraded to 12-pre2 already ;-) It looks like that it works.

Yup. It works.

Quoted 7 lines
Quoted 5 lines

- keventd is now capable of reaping dead children. I will be all ears if someone can tell me how to get a kernel thread to accept signals without having to install a handler with

    sa.sa_handler = (__sighandler_t)100;

Is there any reason why not set sa.sa_handler to SIG_IGN?

Nope. SIG_IGN works well for SIGCHLD, which is special-cased for the reasons which you identify. Thanks.

Quoted 5 lines

According
to arch/i386/kernel/signal.c:do_signal(), signal manpage and my memory, kernel does automatic child reaping in such configuration. So you could even remove waitpid() loop from keventd. I did not tried it yet, but it looks like obvious solution...

Unfortunately this doesn't work for kernel threads.

do_signal()

{
	...
        /*
         * We want the common case to go fast, which
         * is why we may in certain cases get here from
         * kernel mode. Just return without doing anything
         * if so.
         */
        if ((regs->xcs & 3) != 3)
		return 1;
	...
	<child reaping code goes here>

I suspect all the fancy signal frame assembly would blow up if it tried to build a frame on a kernel stack. Not sure...

do_signal() could be altered to do the reaping even for kernel threads, but that means touching every arch and it's not The Right Way.

Similarly, the kernel thread could be reparented to init via REMOVE_LINKS/SET_LINKS but again, I didn't want to commit a layering violation just because proper signal and task management for kernel threads is tricky.

Quoted 4 lines

BTW, are you sure that kernel does not try to deliver SIGSEGV to keventd() when signal arrives. It looks like that it should, but it probably fails somewhere during way due to non-existent userspace for this process.

Well, the fault handler exception table walker will see we're running a kernel thread and will force an oops. Plus SIGSEGV is blocked...

Quoted 4 lines

Review

Quoted 2 lines
(quoted diff) +2 -0
+   sa.sa.sa_handler = (__sighandler_t)100;     /* Surely there's a better way? */
+   sa.sa.sa_flags = 0;

SA_RESTART?

That's handled during signal delivery. Kernel threads don't get that far.

Quoted 1 line

SA_NOCLDSTOP ?

Kernel threads can block SIGSTOP :)

BTW, quite a lot of the modprobe-specific stuff can probably now be thrown away - it can use this version of call_usermodehelper().
But that's not a bug. Next time..
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/