Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 7 May 1998 03:18:03 +0200
From:      Eivind Eklund <eivind@yes.no>
To:        "Jordan K. Hubbard" <jkh@time.cdrom.com>, Snob Art Genre <benedict@echonyc.com>
Cc:        "Ron G. Minnich" <rminnich@Sarnoff.COM>, hackers@FreeBSD.ORG
Subject:   Re: an interesting problem with pkg_add
Message-ID:  <19980507031803.45094@follo.net>
In-Reply-To: <19995.894391274@time.cdrom.com>; from Jordan K. Hubbard on Tue, May 05, 1998 at 11:01:14AM -0700
References:  <Pine.GSO.3.96.980505125137.22120B-100000@echonyc.com> <19995.894391274@time.cdrom.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, May 05, 1998 at 11:01:14AM -0700, Jordan K. Hubbard wrote:
> I know these bugs were extremely pathological and if I hadn't already
> done my best to chase down and fix them, I'd feel a lot worse than I
> do right now about it.  Having fixed it (knock on wood), all I can say
> at this stage is "man, I'm sorry Ron!  I hope you had backups and I
> only wish you'd hit ^C *after* the package tools had been upgraded to
> 2.2-stable!"  :-)

When I read the 3.0-current sources, I see something that looks like a
race condition.  Unless I'm reading this incorrectly, if a sprintf to
LogDir is interrupted by a signal the vsystem() call can happen on a
partially filled LogDir buffer.  There are other parts of the code
that is so twisted that I won't hazard a guess as to whether this is
the last bug or not.

A patch like the following would fix the race, but overall the code
need a large cleanup if we're to be certain it work correct.  E.g, the
signal handler now malloc()s memory (bringing in another race), and
none of the communication with signal handler seem to be done
atomically.

Index: perform.c
===================================================================
RCS file: /home/ncvs/src/usr.sbin/pkg_install/add/perform.c,v
retrieving revision 1.49
diff -u -r1.49 perform.c
--- perform.c	1998/02/16 17:16:14	1.49
+++ perform.c	1998/05/07 01:10:50
@@ -33,6 +33,7 @@
 static int pkg_do(char *);
 static int sanity_check(char *);
 static char LogDir[FILENAME_MAX];
+static int zapLogDir;		/* Should we delete LogDir? */
 
 int
 pkg_perform(char **pkgs)
@@ -73,6 +74,7 @@
     int inPlace;
 
     code = 0;
+    zapLogDir = 0;
     LogDir[0] = '\0';
     strcpy(playpen, FirstPen);
     inPlace = 0;
@@ -364,6 +366,7 @@
 	    goto success;	/* well, partial anyway */
 	}
 	sprintf(LogDir, "%s/%s", (tmp = getenv(PKG_DBDIR)) ? tmp : DEF_LOG_DIR, PkgName);
+	zapLogDir = 1;
 	if (Verbose)
 	    printf("Attempting to record package into %s..\n", LogDir);
 	if (make_hierarchy(LogDir)) {
@@ -476,7 +479,7 @@
 	in_cleanup = 1;
     	if (signo)
 		printf("Signal %d received, cleaning up..\n", signo);
-    	if (!Fake && LogDir[0])
+    	if (!Fake && zapLogDir && LogDir[0])
 		vsystem("%s -rf %s", REMOVE_CMD, LogDir);
     	leave_playpen();
     }


Eivind.

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-hackers" in the body of the message



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?19980507031803.45094>