diff options
author | Bram Moolenaar <Bram@vim.org> | 2017-11-26 16:50:41 +0100 |
---|---|---|
committer | Bram Moolenaar <Bram@vim.org> | 2017-11-26 16:50:41 +0100 |
commit | c41838aa01ef99540e2737c42e9b1283e3da5e26 (patch) | |
tree | e2c5cc5a1b629caf0f074b7fdaf01c4b05c220de /src/ex_cmds.c | |
parent | 2877d334ad1321d1fcd5f903c0493bd0cdd787f8 (diff) | |
download | vim-git-c41838aa01ef99540e2737c42e9b1283e3da5e26.tar.gz |
patch 8.0.1345: race condition between stat() and open() for viminfov8.0.1345
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.
Diffstat (limited to 'src/ex_cmds.c')
-rw-r--r-- | src/ex_cmds.c | 213 |
1 files changed, 111 insertions, 102 deletions
diff --git a/src/ex_cmds.c b/src/ex_cmds.c index f86e141fc..e95580c30 100644 --- a/src/ex_cmds.c +++ b/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) - /* - * For Unix we create the .viminfo non-accessible for others, - * because it may contain text from non-accessible documents. - */ - umask_save = umask(077); -#endif - fp_out = mch_fopen((char *)fname, WRITEBIN); -#if defined(UNIX) || defined(VMS) - (void)umask(umask_save); -#endif + + /* Create the new .viminfo non-accessible for others, because it may + * contain text from non-accessible documents. It is up to the user to + * widen access (e.g. to a group). This may also fail if there is a + * race condition, then just give up. */ + fd = mch_open((char *)fname, + O_CREAT|O_EXTRA|O_EXCL|O_WRONLY|O_NOFOLLOW, 0600); + if (fd < 0) + goto end; + fp_out = fdopen(fd, WRITEBIN); } 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, @@ -1924,127 +1931,129 @@ write_viminfo(char_u *file, int forceit) break; /* - * Check if tempfile already exists. Never overwrite an - * existing file! + * 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. */ - if (mch_stat((char *)tempname, &st_new) == 0) + wp = tempname + STRLEN(tempname) - 5; + if (wp < gettail(tempname)) /* empty file name? */ + wp = gettail(tempname); + for (;;) { -#ifdef UNIX - /* - * Check if tempfile is same as original file. May happen - * when modname() gave the same file back. E.g. silly - * link, or file name-length reached. Try again with - * shortname set. - */ - if (!shortname && st_new.st_dev == st_old.st_dev - && st_new.st_ino == st_old.st_ino) - { - vim_free(tempname); - tempname = NULL; - shortname = TRUE; - continue; - } -#endif /* - * Try another name. 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. + * Check if tempfile already exists. Never overwrite an + * existing file! */ - 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) + if (mch_stat((char *)tempname, &st_new) == 0) { +#ifdef UNIX /* - * They all exist? Must be something wrong! Don't - * write the viminfo file then. + * Check if tempfile is same as original file. May happen + * when modname() gave the same file back. E.g. silly + * link, or file name-length reached. Try again with + * shortname set. */ - if (*wp == 'a') + if (!shortname && st_new.st_dev == st_old.st_dev + && st_new.st_ino == st_old.st_ino) { - EMSG2(_("E929: Too many viminfo temp files, like %s!"), - tempname); vim_free(tempname); tempname = NULL; + shortname = TRUE; break; } +#endif } - } - break; - } - - if (tempname != NULL) - { + else + { + /* 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); - fp_out = mch_fopen((char *)tempname, WRITEBIN); - (void)umask(umask_save); + /* fdopen() fails for some reason */ + umask_save = umask(077); + fp_out = mch_fopen((char *)tempname, WRITEBIN); + (void)umask(umask_save); #else - int fd; + int fd; - /* 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 - * avoid it getting in the way. - * Others: r&w for user only. */ + /* 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 avoid it getting in the way. + * Others: r&w for user only. */ # ifdef UNIX - umask_save = umask(0); - fd = mch_open((char *)tempname, - O_CREAT|O_EXTRA|O_EXCL|O_WRONLY|O_NOFOLLOW, - (int)((st_old.st_mode & 0777) | 0600)); - (void)umask(umask_save); + umask_save = umask(0); + fd = mch_open((char *)tempname, + O_CREAT|O_EXTRA|O_EXCL|O_WRONLY|O_NOFOLLOW, + (int)((st_old.st_mode & 0777) | 0600)); + (void)umask(umask_save); # else - fd = mch_open((char *)tempname, - O_CREAT|O_EXTRA|O_EXCL|O_WRONLY|O_NOFOLLOW, 0600); + fd = mch_open((char *)tempname, + O_CREAT|O_EXTRA|O_EXCL|O_WRONLY|O_NOFOLLOW, 0600); # endif - if (fd < 0) - fp_out = NULL; - else - fp_out = fdopen(fd, WRITEBIN); + 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; + } - /* - * If we can't create in the same directory, try creating a - * "normal" temp file. This is just an attempt, renaming the temp - * file might fail as well. - */ - if (fp_out == NULL) - { - vim_free(tempname); - if ((tempname = vim_tempname('o', TRUE)) != NULL) - fp_out = mch_fopen((char *)tempname, WRITEBIN); + /* Assume file exists, try again with another name. */ + if (next_char == 'a' - 1) + { + /* They all exist? Must be something wrong! Don't write + * the viminfo file then. */ + 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) + if (mch_stat((char *)tempname, &tmp_st) >= 0) { - stat_T tmp_st; - - if (mch_stat((char *)tempname, &tmp_st) >= 0) - { - if (st_old.st_uid != tmp_st.st_uid) - /* Changing the owner might fail, in which case the - * file will now owned by the current user, oh well. */ - ignored = fchown(fileno(fp_out), st_old.st_uid, -1); - if (st_old.st_gid != tmp_st.st_gid - && fchown(fileno(fp_out), -1, st_old.st_gid) == -1) - /* can't set the group to what it should be, remove - * group permissions */ - (void)mch_setperm(tempname, 0600); - } - else - /* can't stat the file, set conservative permissions */ + if (st_old.st_uid != tmp_st.st_uid) + /* Changing the owner might fail, in which case the + * file will now owned by the current user, oh well. */ + ignored = fchown(fileno(fp_out), st_old.st_uid, -1); + if (st_old.st_gid != tmp_st.st_gid + && fchown(fileno(fp_out), -1, st_old.st_gid) == -1) + /* can't set the group to what it should be, remove + * group permissions */ (void)mch_setperm(tempname, 0600); } -#endif + else + /* can't stat the file, set conservative permissions */ + (void)mch_setperm(tempname, 0600); } } +#endif /* * Check if the new viminfo file can be written to. |