patch 8.0.1345: race condition between stat() and open() for viminfo

Problem:    Race condition between stat() and open() for the viminfo temp
            file. (Simon Ruderich)
Solution:   use open() with O_EXCL to atomically check if the file exists.
            Don't try using a temp file, renaming it will fail anyway.

src/ex_cmds.c

@@ -1825,7 +1825,6 @@ write_viminfo(char_u *file, int forceit)
     FILE	*fp_out = NULL;	/* output viminfo file */
     char_u	*tempname = NULL;	/* name of temp viminfo file */
     stat_T	st_new;		/* mch_stat() of potential new file */
-    char_u	*wp;
 #if defined(UNIX) || defined(VMS)
     mode_t	umask_save;
 #endif
@@ -1847,27 +1846,29 @@ write_viminfo(char_u *file, int forceit)
     fp_in = mch_fopen((char *)fname, READBIN);
     if (fp_in == NULL)
     {
+	int fd;
+
 	/* if it does exist, but we can't read it, don't try writing */
 	if (mch_stat((char *)fname, &st_new) == 0)
 	    goto end;
-#if defined(UNIX) || defined(VMS)
+
-	/*
+	/* Create the new .viminfo non-accessible for others, because it may
-	 * For Unix we create the .viminfo non-accessible for others,
+	 * contain text from non-accessible documents. It is up to the user to
-	 * because it may contain text from non-accessible documents.
+	 * widen access (e.g. to a group). This may also fail if there is a
-	 */
+	 * race condition, then just give up. */
-	umask_save = umask(077);
+	fd = mch_open((char *)fname,
-#endif
+			    O_CREAT|O_EXTRA|O_EXCL|O_WRONLY|O_NOFOLLOW, 0600);
-	fp_out = mch_fopen((char *)fname, WRITEBIN);
+	if (fd < 0)
-#if defined(UNIX) || defined(VMS)
+	    goto end;
-	(void)umask(umask_save);
+	fp_out = fdopen(fd, WRITEBIN);
-#endif
     }
     else
     {
 	/*
 	 * There is an existing viminfo file.  Create a temporary file to
 	 * write the new viminfo into, in the same directory as the
-	 * existing viminfo file, which will be renamed later.
+	 * existing viminfo file, which will be renamed once all writing is
+	 * successful.
 	 */
 #ifdef UNIX
 	/*
@@ -1901,12 +1902,18 @@ write_viminfo(char_u *file, int forceit)
 #endif
 
 	/*
-	 * Make tempname.
+	 * Make tempname, find one that does not exist yet.
+	 * Beware of a race condition: If someone logs out and all Vim
+	 * instances exit at the same time a temp file might be created between
+	 * stat() and open().  Use mch_open() with O_EXCL to avoid that.
 	 * May try twice: Once normal and once with shortname set, just in
 	 * case somebody puts his viminfo file in an 8.3 filesystem.
 	 */
 	for (;;)
 	{
+	    int		next_char = 'z';
+	    char_u	*wp;
+
 	    tempname = buf_modname(
 #ifdef UNIX
 				    shortname,
@@ -1923,6 +1930,17 @@ write_viminfo(char_u *file, int forceit)
 	    if (tempname == NULL)		/* out of memory */
 		break;
 
+	    /*
+	     * Try a series of names.  Change one character, just before
+	     * the extension.  This should also work for an 8.3
+	     * file name, when after adding the extension it still is
+	     * the same file as the original.
+	     */
+	    wp = tempname + STRLEN(tempname) - 5;
+	    if (wp < gettail(tempname))	    /* empty file name? */
+		wp = gettail(tempname);
+	    for (;;)
+	    {
 		/*
 		 * Check if tempfile already exists.  Never overwrite an
 		 * existing file!
@@ -1942,40 +1960,14 @@ write_viminfo(char_u *file, int forceit)
 			vim_free(tempname);
 			tempname = NULL;
 			shortname = TRUE;
-		    continue;
+			break;
 		    }
 #endif
-		/*
+		}
-		 * Try another name.  Change one character, just before
+		else
-		 * the extension.  This should also work for an 8.3
-		 * file name, when after adding the extension it still is
-		 * the same file as the original.
-		 */
-		wp = tempname + STRLEN(tempname) - 5;
-		if (wp < gettail(tempname))	    /* empty file name? */
-		    wp = gettail(tempname);
-		for (*wp = 'z'; mch_stat((char *)tempname, &st_new) == 0;
-								    --*wp)
-		{
-		    /*
-		     * They all exist?  Must be something wrong! Don't
-		     * write the viminfo file then.
-		     */
-		    if (*wp == 'a')
-		    {
-			EMSG2(_("E929: Too many viminfo temp files, like %s!"),
-								    tempname);
-			vim_free(tempname);
-			tempname = NULL;
-			break;
-		    }
-		}
-	    }
-	    break;
-	}
-
-	if (tempname != NULL)
 		{
+		    /* Try creating the file exclusively.  This may fail if
+		     * another Vim tries to do it at the same time. */
 #ifdef VMS
 		    /* fdopen() fails for some reason */
 		    umask_save = umask(077);
@@ -1986,8 +1978,8 @@ write_viminfo(char_u *file, int forceit)
 
 		    /* Use mch_open() to be able to use O_NOFOLLOW and set file
 		     * protection:
-	     * Unix: same as original file, but strip s-bit.  Reset umask to
+		     * Unix: same as original file, but strip s-bit.  Reset
-	     * avoid it getting in the way.
+		     * umask to avoid it getting in the way.
 		     * Others: r&w for user only. */
 # ifdef UNIX
 		    umask_save = umask(0);
@@ -2000,32 +1992,50 @@ write_viminfo(char_u *file, int forceit)
 			     O_CREAT|O_EXTRA|O_EXCL|O_WRONLY|O_NOFOLLOW, 0600);
 # endif
 		    if (fd < 0)
+		    {
 			fp_out = NULL;
+# ifdef EEXIST
+			/* Avoid trying lots of names while the problem is lack
+			 * of premission, only retry if the file already
+			 * exists. */
+			if (errno != EEXIST)
+			    break;
+# endif
+		    }
 		    else
 			fp_out = fdopen(fd, WRITEBIN);
 #endif /* VMS */
+		    if (fp_out != NULL)
+			break;
+		}
 
-	    /*
+		/* Assume file exists, try again with another name. */
-	     * If we can't create in the same directory, try creating a
+		if (next_char == 'a' - 1)
-	     * "normal" temp file.  This is just an attempt, renaming the temp
-	     * file might fail as well.
-	     */
-	    if (fp_out == NULL)
 		{
-		vim_free(tempname);
+		    /* They all exist?  Must be something wrong! Don't write
-		if ((tempname = vim_tempname('o', TRUE)) != NULL)
+		     * the viminfo file then. */
-		    fp_out = mch_fopen((char *)tempname, WRITEBIN);
+		    EMSG2(_("E929: Too many viminfo temp files, like %s!"),
+								     tempname);
+		    break;
+		}
+		*wp = next_char;
+		--next_char;
+	    }
+
+	    if (tempname != NULL)
+		break;
+	    /* continue if shortname was set */
 	}
 
 #if defined(UNIX) && defined(HAVE_FCHOWN)
+	if (tempname != NULL && fp_out != NULL)
+	{
+		stat_T	tmp_st;
+
 	    /*
 	     * Make sure the original owner can read/write the tempfile and
 	     * otherwise preserve permissions, making sure the group matches.
 	     */
-	    if (fp_out != NULL)
-	    {
-		stat_T	tmp_st;
-
 	    if (mch_stat((char *)tempname, &tmp_st) >= 0)
 	    {
 		if (st_old.st_uid != tmp_st.st_uid)
@@ -2042,9 +2052,8 @@ write_viminfo(char_u *file, int forceit)
 		/* can't stat the file, set conservative permissions */
 		(void)mch_setperm(tempname, 0600);
 	}
+    }
 #endif
-	}
-    }
 
     /*
      * Check if the new viminfo file can be written to.

src/version.c

@@ -771,6 +771,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1345,
 /**/
     1344,
 /**/