Re: [leafnode-list] lockfile_exists disaster, analysis of 1.9.19 and

[krasel@xxxxxxxxxxxxxxxxxxxxxxxxxxxx (Cornelius Krasel)]

Matthias Andree wrote:

> ----------------------------------------------------------------------
> ANALYSIS OF 1.9.19 RACE
> ----------------------------------------------------------------------
> 
> 1.9.19 has a TOC-TOU race (time of check - time of use). If a context
> switch occurs between the read-open and the write-open of the lock file,
> two processes can have the lock, since both of them have seen "no lock
> file there" (or possibly "removed stale lock file"), and fopen(...,"w")
> does not set O_EXCL on open(2). Non-atomicity issue here.

Here is what I believe to be a fix. Patch your version of 1.9.19 with it:

--- miscutil.c.orig	Tue Jun  5 17:36:13 2001
+++ miscutil.c	Wed Jun 13 17:18:17 2001
@@ -115,44 +115,65 @@
  * also return 1, otherwise return 0
  */
 int lockfile_exists( void ) {
-    FILE * lf ;
-    int  pid ;
+    int  fd;
+    FILE *lf;
+    int  pid;
+    char *lockfilename;
 
-    if ( ( lf = fopen( lockfile, "r+" )) != NULL ) {
-	if ( fscanf( lf, "%d", &pid ) ) {
+    /* obtain a unique lockfile name, consisting of lockfile + pid */
+    lockfilename = critmalloc(strlen(lockfile)+10,
+			      "Allocating space for lockfile name");
+    snprintf(lockfilename, strlen(lockfile)+9, "%s-%d", lockfile, getpid());
+    if ((fd = open(lockfilename, O_WRONLY | O_CREAT | O_EXCL, 0600)) < 0) {
+	syslog(LOG_ERR, "Creating unique lockfile failed: %m");
+	return 1;	/* behave like a lockfile would exist */
+    }
+    close(fd);
+
+    /* try to obtain exclusive control over the lockfile */
+    if (link(lockfilename, lockfile) < 0) {
+	/* lockfile already exists. Does the process still exist as well? */
+	lf = fopen(lockfile, "r+");
+	if (lf == NULL) {
+	    syslog(LOG_ERR, "Cannot open old lockfile: %m");
+	    unlink(lockfilename);
+	    return 1;	/* lockfile exists */
+	}
+	if (fscanf(lf, "%d", &pid)) {
 	    fclose( lf );
-	    if ( kill( pid, 0 ) == -1 && errno == ESRCH ) {
+	    if ( kill(pid, 0) == -1 && errno == ESRCH ) {
 		if ( verbose )
-		    printf( "Removing stale lockfile.\n" );
-		syslog( LOG_INFO, "Removing stale lockfile: pid %d", pid );
-		unlink( lockfile );
+		    printf( "Lockfile is stale, ignoring.\n" );
+		syslog( LOG_INFO, "Ignoring stale lockfile: pid %d", pid );
 	    } else {
 		printf( "lockfile %s exists, abort ...\n", lockfile);
 		syslog( LOG_ERR, "lockfile %s exists, abort ...",
 			lockfile);
+		unlink(lockfilename);
 		return 1;
 	    }
 	} else {
-	    fclose( lf );
-	    syslog( LOG_ERR, "unable to read lockfile %s, abort ...", lockfile );
+	    fclose(lf);
+	    syslog(LOG_ERR, "unable to read lockfile %s, abort ...", lockfile);
+	    unlink(lockfilename);
 	    return 1;
 	}
-    } else if ( errno != ENOENT ) {
-	syslog( LOG_ERR, "unable to open lockfile %s: %d %m", lockfile, errno );
     }
-    if ( ( lf = fopen( lockfile, "w" )) != NULL ) {
+
+    /* We have obtained the exclusive lock. Write pid into it. */
+    if ( (lf = fopen( lockfile, "w" )) != NULL ) {
 	fprintf( lf, "%d", (int)getpid() );
-	fclose( lf );
+	fclose(lf);
+	unlink(lockfilename);
 	return 0;
     } else {
-	printf( "Could not open lockfile %s for writing, abort program ...\n",
-		lockfile );
-	syslog( LOG_ERR, "Could not open lockfile %s for writing: %m",
-		lockfile );
+	printf("Could not open lockfile %s for writing, abort program ...\n",
+		lockfile);
+	syslog(LOG_ERR, "Could not open lockfile %s for writing: %m", lockfile);
+	unlink(lockfilename);
 	return 1;
     }
 }
-
 
 /*
  * check whether "groupname" is represented in interesting.groups without

I don't have tcpserver, but maybe somebody else can try out whether this
version of lockfile_exists() is safe. I put this version into my local
2.0b tree right now and will soon find out whether there are any problems
with it :-/

--Cornelius.

-- 
/* Cornelius Krasel, U Wuerzburg, Dept. of Pharmacology, Versbacher Str. 9 */
/* D-97078 Wuerzburg, Germany   email: krasel@xxxxxxxxxxxxxxxxxxxxxxxxxxxx */
/* "Science is the game we play with God to find out what His rules are."  */

-- 
leafnode-list@xxxxxxxxxxxxxxxxxxxxxxxxxxxx -- mailing list for leafnode
To unsubscribe, send mail with "unsubscribe" in the subject to the list