[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#212764: marked as done (2 race conditions in init implementation)



Your message dated Fri, 03 Oct 2003 12:44:05 -0400
with message-id <E1A5T2D-0008G5-00@auric.debian.org>
and subject line Bug#212764: fixed in busybox-cvs 20030926-1
has caused the attached Bug report to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what I am
talking about this indicates a serious mail system misconfiguration
somewhere.  Please contact me immediately.)

Debian bug tracking system administrator
(administrator, Debian Bugs database)

--------------------------------------
Received: (at submit) by bugs.debian.org; 25 Sep 2003 21:42:20 +0000
>From guillaume@morinfr.org Thu Sep 25 16:41:28 2003
Return-path: <guillaume@morinfr.org>
Received: from 66-65-114-133.nyc.rr.com (siri.morinfr.org) [66.65.114.133] 
	by master.debian.org with esmtp (Exim 3.35 1 (Debian))
	id 1A2drb-0005cD-00; Thu, 25 Sep 2003 16:41:27 -0500
Received: from guillaum by siri.morinfr.org with local (Exim 4.22)
	id 1A2ds6-0000tq-5v; Thu, 25 Sep 2003 17:41:58 -0400
Date: Thu, 25 Sep 2003 17:41:58 -0400
From: Guillaume Morin <gemorin@debian.org>
To: Debian Bug Tracking System <submit@bugs.debian.org>
Subject: 2 race conditions in init implementation
Message-ID: <20030925214149.GA3327@siri.morinfr.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
X-Reportbug-Version: 2.29
User-Agent: Mutt/1.5.4i
Sender: Guillaume Morin <guillaume@morinfr.org>
Delivered-To: submit@bugs.debian.org
X-Spam-Status: No, hits=-11.9 required=4.0
	tests=HAS_PACKAGE,PATCH_UNIFIED_DIFF,SIGNATURE_SHORT_SPARSE
	autolearn=ham version=2.53-bugs.debian.org_2003_9_21
X-Spam-Level: 
X-Spam-Checker-Version: SpamAssassin 2.53-bugs.debian.org_2003_9_21 (1.174.2.15-2003-03-30-exp)

Package: busybox-cvs
Version: 0.60.99.cvs20030819-3 (not installed)
Severity: important
Tags: patch

Hi,

I have found two races conditions in the run() function of busybox's
init implementation (static pid_t run(const struct init_action *a)).

The runtime behavior is that the call to wait in waitfor() blocks
forever. I can easily reproduce it on SMP system. Actually, it should
only easily reproducable on 2 processors systems.

- 1st race condition

around line 450
        sigemptyset(&nmask);
        sigaddset(&nmask, SIGCHLD);
        sigprocmask(SIG_BLOCK, &nmask, &omask);

        if ((pid = fork()) == 0) {
		/* exec the command here  and exit*/
	}

	sigprocmask(SIG_SETMASK, &omask, NULL);
	return pid;

	/* the parent will call wait() after returning */

There is a race here. If the child exits before the SIGCHLD is
unblocked, wait() will block forever.

I have fixed the bug by replacing the wait() call in waitfor() with a
waitpid() call and handle the error.

- 2nd race condition

on line 486
                        /* Now fork off another process to just hang around */
                        if ((pid = fork()) < 0) {
                                message(LOG | CONSOLE, "Can't fork!");
                                _exit(1);
                        }

                        if (pid > 0) {

                                /* We are the parent -- wait till the child is done */
                                signal(SIGINT, SIG_IGN);
                                signal(SIGTSTP, SIG_IGN);
                                signal(SIGQUIT, SIG_IGN);
                                signal(SIGCHLD, SIG_DFL);

                                /* Wait for child to exit */
                                while ((tmp_pid = waitpid(pid, &junk, 0)) != pid);

If the child exits before the call to signal(SIGCHLD, SIG_DFL), another
sigchld handler might be executed or the signal may be ignored. In both
cases, waitpid() won't work, so this condition should be tested. I have
modified that in my patch.

There is another possible fix. You can move signal(SIGCHLD, SIG_DFL)
before the fork on line 457 ... Your call, this may be cleaner. The
signal calls are still very racy but it should not really matter.

--- /home/guillaum/tmp/burp/busybox-cvs-0.60.99.cvs20030819/init/init.c 2003-08-19 11:59:13.000000000 +0000
+++ busybox-cvs-0.60.99.cvs20030819/init/init.c 2003-09-25 21:24:41.000000000 +0000
@@ -496,7 +496,11 @@
                                signal(SIGCHLD, SIG_DFL);

                                /* Wait for child to exit */
-                               while ((tmp_pid = waitpid(pid, &junk, 0)) != pid);
+                               while ((tmp_pid = waitpid(pid, &junk, 0)) != pid) {
+                                       if (tmp_pid == -1 && errno == ECHILD)
+                                               break;
+                                       // FIXME handle other errors
+                               }

                                /* See if stealing the controlling tty back is necessary */
                                pgrp = tcgetpgrp(0);
@@ -620,12 +624,14 @@

        pid = run(a);
        while (1) {
-               wpid = wait(&status);
-               if (wpid > 0 && wpid != pid) {
-                       continue;
-               }
+               wpid = waitpid(pid,&status,0);
                if (wpid == pid)
                        break;
+               if (wpid == -1 && errno == ECHILD)
+                       /* we missed its termination */
+                       break;
+               /* FIXME other errors should maybe trigger an error, but allow
+                * the program to continue */
        }
        return wpid;
 }


HTH.

Guillaume.

-- System Information:
Debian Release: testing/unstable
Architecture: powerpc
Kernel: Linux siri 2.4.22-ben2 #19 ven sep 12 09:08:08 EDT 2003 ppc
Locale: LANG=C, LC_CTYPE=C (ignored: LC_ALL set to fr_FR@euro)


-- 
Guillaume Morin <guillaume@morinfr.org>

  I love you, I love you like a chocolate cake, like the trains, like the sea.
                     See me loving you, please. (Dionysos)

---------------------------------------
Received: (at 212764-close) by bugs.debian.org; 3 Oct 2003 16:53:43 +0000
>From troup@auric.debian.org Fri Oct 03 11:53:43 2003
Return-path: <troup@auric.debian.org>
Received: from auric.debian.org [206.246.226.45] 
	by master.debian.org with esmtp (Exim 3.35 1 (Debian))
	id 1A5TBW-0001f5-00; Fri, 03 Oct 2003 11:53:42 -0500
Received: from troup by auric.debian.org with local (Exim 3.35 1 (Debian))
	id 1A5T2D-0008G5-00; Fri, 03 Oct 2003 12:44:05 -0400
From: Bastian Blank <waldi@debian.org>
To: 212764-close@bugs.debian.org
X-Katie: lisa $Revision: 1.25 $
Subject: Bug#212764: fixed in busybox-cvs 20030926-1
Message-Id: <E1A5T2D-0008G5-00@auric.debian.org>
Sender: James Troup <troup@auric.debian.org>
Date: Fri, 03 Oct 2003 12:44:05 -0400
Delivered-To: 212764-close@bugs.debian.org

Source: busybox-cvs
Source-Version: 20030926-1

We believe that the bug you reported is fixed in the latest version of
busybox-cvs, which is due to be installed in the Debian FTP archive:

busybox-cvs-floppy-udeb_20030926-1_i386.udeb
  to pool/main/b/busybox-cvs/busybox-cvs-floppy-udeb_20030926-1_i386.udeb
busybox-cvs-static_20030926-1_i386.deb
  to pool/main/b/busybox-cvs/busybox-cvs-static_20030926-1_i386.deb
busybox-cvs-udeb_20030926-1_i386.udeb
  to pool/main/b/busybox-cvs/busybox-cvs-udeb_20030926-1_i386.udeb
busybox-cvs_20030926-1.diff.gz
  to pool/main/b/busybox-cvs/busybox-cvs_20030926-1.diff.gz
busybox-cvs_20030926-1.dsc
  to pool/main/b/busybox-cvs/busybox-cvs_20030926-1.dsc
busybox-cvs_20030926-1_i386.deb
  to pool/main/b/busybox-cvs/busybox-cvs_20030926-1_i386.deb
busybox-cvs_20030926.orig.tar.gz
  to pool/main/b/busybox-cvs/busybox-cvs_20030926.orig.tar.gz



A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to 212764@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Bastian Blank <waldi@debian.org> (supplier of updated busybox-cvs package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing ftpmaster@debian.org)


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Format: 1.7
Date: Fri, 26 Sep 2003 15:10:14 +0200
Source: busybox-cvs
Binary: busybox-cvs-static busybox-cvs busybox-cvs-udeb busybox-cvs-floppy-udeb
Architecture: source i386
Version: 20030926-1
Distribution: unstable
Urgency: low
Maintainer: Debian Install System Team <debian-boot@lists.debian.org>
Changed-By: Bastian Blank <waldi@debian.org>
Description: 
 busybox-cvs - Tiny utilities for small and embedded systems
 busybox-cvs-floppy-udeb - Tiny utilities for the debian-installer floppy images (udeb)
 busybox-cvs-static - Standalone rescue shell with tons of builtin utilities
 busybox-cvs-udeb - Tiny utilities for the debian-installer (udeb)
Closes: 211457 211675 212485 212764
Changes: 
 busybox-cvs (20030926-1) unstable; urgency=low
 .
   * new cvs version
   * Makefile
     - fix libpwdgrp link (upstream) (closes: #211675)
   * Rules.mak.in
     - fix optimization (closes: #212485)
   * debian/config*
     - update
   * debian/config*udeb*
     - move linux-i386 to linux (enable modutils on any linux arch)
     - rename net to floppy
     - enable wget status bar (closes: #211457)
   * init/init.c
     - workaround race conditions (closes: #212764)
Files: 
 60a493fd4db044e93d4c863bb6db84ef 835 - optional busybox-cvs_20030926-1.dsc
 bb886187fc0edbbcac3366b1a2e402c6 1273560 - optional busybox-cvs_20030926.orig.tar.gz
 375af534f0e870a765a393dac512f6eb 153756 - optional busybox-cvs_20030926-1.diff.gz
 510764227807c67f234b76c385134e44 101040 utils optional busybox-cvs_20030926-1_i386.deb
 6bc9d5dcd7cc9efd6ace0b1b0cc845b2 636756 shells optional busybox-cvs-static_20030926-1_i386.deb
 605069664a3b9ebaef28e4e5528fb3be 134516 debian-installer extra busybox-cvs-udeb_20030926-1_i386.udeb
 f45b0767d769e57c5126af2a7603941f 115850 debian-installer extra busybox-cvs-floppy-udeb_20030926-1_i386.udeb

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.3 (GNU/Linux)

iEYEARECAAYFAj90PMwACgkQLkAIIn9ODhE+UACg8Amz+7/oreLJQFjT/dzK/SAm
GOgAnAzbzaw1hKtrJ1SdzwzAYZvxoREl
=Tupv
-----END PGP SIGNATURE-----



Reply to: