[PATCH] 8139too reparent_to_init() race

[PATCH] 8139too reparent_to_init() race

[Robert Kuebel]

hello all,

lately i noticed this message during boot-up (when the network
interfaces were being configured) ...

"task `ifconfig' exit_signal 17 in reparent_to_init"

this happens only about 1/2 of the time.

after some digging this is what i found...
sometimes ifconfig's parent exits before ifconfig reaches
rtl8139_thread().  when this happens, ifconfig's exit_signal is set to
SIGCHLD (in forget_original_parent), because its new parent is init.
then rlt8139_thread() is reached it calls reparent_to_init(), which
complains that exit_signal is already non-zero.

basically this patch stops rtl8139_thread() from calling
reparent_to_init() when its parent is already init.

is this the right way to fix the problem?
should reparent_to_init() check that the parent is not already init?

as a budding kernel hacker i would appreciate any comment on this
change.

please, cc me on replies.  i can only handle the digest form of this
list.

thanks.
rob.

--- linux-2.4.13/drivers/net/8139too.orig.c	Sun Oct 28 18:16:48 2001
+++ linux-2.4.13/drivers/net/8139too.c	Sun Oct 28 18:18:47 2001
@@ -1654,7 +1654,8 @@
 	unsigned long timeout;
 
 	daemonize ();
-	reparent_to_init();
+	if (current->p_opptr != child_reaper)
+		reparent_to_init();
 	spin_lock_irq(&current->sigmask_lock);
 	sigemptyset(&current->blocked);
 	recalc_sigpending(current);

Re: [PATCH] 8139too reparent_to_init() race

[Andrew Morton]

Robert Kuebel wrote:
> 
> hello all,
> 
> lately i noticed this message during boot-up (when the network
> interfaces were being configured) ...
> 
> "task `ifconfig' exit_signal 17 in reparent_to_init"
> 
> this happens only about 1/2 of the time.
> 
> after some digging this is what i found...
> sometimes ifconfig's parent exits before ifconfig reaches
> rtl8139_thread().  when this happens, ifconfig's exit_signal is set to
> SIGCHLD (in forget_original_parent), because its new parent is init.
> then rlt8139_thread() is reached it calls reparent_to_init(), which
> complains that exit_signal is already non-zero.
> 
> basically this patch stops rtl8139_thread() from calling
> reparent_to_init() when its parent is already init.
> 

Thanks - that's a useful analysis.

The check in reparent_to_init() was to warn about the situation
where someone had deliberately set the exit signal to some
non-zero value and then the child calls reparent_to_init() - it's
telling you that we're about to stomp on your chosen exit signal.
I hadn't thought about the forget_original_parent() case.

So the fix should be to change the debug code in reparent_to_init()
so it doesn't complain if the exit signal is already SIGCHLD.  Or
just kill it off altogether.


--- linux-2.4.14-pre3/kernel/sched.c	Tue Oct 23 23:09:48 2001
+++ linux-akpm/kernel/sched.c	Sun Oct 28 17:23:26 2001
@@ -1250,11 +1250,6 @@ void reparent_to_init(void)
 	SET_LINKS(this_task);
 
 	/* Set the exit signal to SIGCHLD so we signal init on exit */
-	if (this_task->exit_signal != 0) {
-		printk(KERN_ERR "task `%s' exit_signal %d in "
-				__FUNCTION__ "\n",
-			this_task->comm, this_task->exit_signal);
-	}
 	this_task->exit_signal = SIGCHLD;
 
 	/* We also take the runqueue_lock while altering task fields

-