Subject: race in mkdir(1)
To: None <tech-userlevel@netbsd.org>
From: Chuck Silvers <chuq@chuq.com>
List: tech-userlevel
Date: 11/24/2002 08:21:03
--OgqxwSJOaUobr8KG
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

I do build.sh with "-j 8", and occasionally during the "make obj" pass
mkdir errors out with "File exists".  this is due to a race between
multiple "mkdir -p" operations.  in mkdir.c:mkpath() there's

		if (stat(path, &sb)) {
			if (errno != ENOENT ||
			    mkdir(path, done ? mode : dir_mode)) {
				warn("%s", path);
				return (-1);
			}


if another process creates the directory between the stat() and the mkdir(),
then a spurious error is returned.  does anyone see anything wrong with the
attached patch?

-Chuck

--OgqxwSJOaUobr8KG
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="diff.mkdir"

Index: bin/mkdir/mkdir.c
===================================================================
RCS file: /cvsroot/basesrc/bin/mkdir/mkdir.c,v
retrieving revision 1.26
diff -u -r1.26 mkdir.c
--- bin/mkdir/mkdir.c	2002/02/19 06:30:12	1.26
+++ bin/mkdir/mkdir.c	2002/11/24 16:11:41
@@ -148,43 +148,58 @@
 {
 	struct stat sb;
 	char *slash;
-	int done;
+	int done, rv;
 
 	done = 0;
 	slash = path;
 
-	while (!done) {
+	for (;;) {
 		slash += strspn(slash, "/");
 		slash += strcspn(slash, "/");
 
 		done = (*slash == '\0');
 		*slash = '\0';
 
-		if (stat(path, &sb)) {
-			if (errno != ENOENT ||
-			    mkdir(path, done ? mode : dir_mode)) {
-				warn("%s", path);
-				return (-1);
-			}
-			/*
-			 * The mkdir() and umask() calls both honor only the
-			 * file permission bits, so if you try to set a mode
-			 * including the sticky, setuid, setgid bits you lose
-			 * them. So chmod().
-			 */
-			if (done && (mode & ~(S_IRWXU|S_IRWXG|S_IRWXU)) != 0 &&
-			    chmod(path, mode) == -1) {
-				warn("%s", path);
-				return (-1);
-			}
-		} else if (!S_ISDIR(sb.st_mode)) {
-			warnx("%s: %s", path, strerror(ENOTDIR));
+		rv = mkdir(path, done ? mode : dir_mode);
+		if (rv < 0 && errno != EEXIST) {
+			warn("%s", path);
 			return (-1);
 		}
-
+		if (done) {
+			break;
+		}
 		*slash = '/';
 	}
 
+	/*
+	 * Check for the final component being something other than
+	 * a directory.
+	 */
+
+	if (rv < 0) {
+		if (stat(path, &sb) < 0) {
+			warn("stat %s failed", path);
+			return (-1);
+		}
+		if (!S_ISDIR(sb.st_mode)) {
+			errno = ENOTDIR;
+			warn("%s", path);
+			return (-1);
+		}
+	}
+
+	/*
+	 * The mkdir() and umask() calls both honor only the
+	 * file permission bits, so if you try to set a mode
+	 * including the sticky, setuid, setgid bits you lose
+	 * them. So chmod().
+	 */
+
+	if ((mode & ~(S_IRWXU|S_IRWXG|S_IRWXU)) != 0 &&
+	    chmod(path, mode) == -1) {
+		warn("%s", path);
+		return (-1);
+	}
 	return (0);
 }
 

--OgqxwSJOaUobr8KG--