From 7fe4062aebf56a7de1ae389907098cd461f8d485 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Wed, 9 Aug 2017 16:34:40 -0700 Subject: [PATCH 1/2] Fix make-temp-file race on local files For the motivation behind this patch, please see Bug#28023 and: http://emacshorrors.com/posts/make-temp-name.html The race still exists for magic file names; fixing this will require Tramp surgery. * admin/merge-gnulib (GNULIB_MODULES): Add tempname, now that Emacs is using it directly. * configure.ac (AUTO_DEPEND): Remove AC_SYS_LONG_FILE_NAMES; no longer needed. * lib/gnulib.mk.in, m4/gnulib-comp.m4: Regenerate. * lisp/files.el (files--make-magic-temp-file): Rename from make-temp-file. Add comment describing race bug. (make-temp-file): Use fileio--make-temp-file for non-magic files names. * src/fileio.c: Include tempname.h. (make_temp_name_tbl, make_temp_name_count) (make_temp_name_count_initialized_p, make_temp_name): Remove. (Ffileio__make_temp_file): New function. (Fmake_temp_name): Use it. * src/filelock.c (get_boot_time): Use Ffileio__make_temp_file instead of make_temp_name; this avoids a race. --- admin/CPP-DEFINES | 1 - admin/merge-gnulib | 4 +- configure.ac | 3 - lib/gnulib.mk.in | 5 +- lisp/files.el | 15 +++++ m4/gnulib-comp.m4 | 13 +--- src/buffer.c | 1 - src/fileio.c | 170 ++++++++++++++--------------------------------------- src/filelock.c | 13 ++-- src/lisp.h | 1 - 10 files changed, 69 insertions(+), 157 deletions(-) diff --git a/admin/CPP-DEFINES b/admin/CPP-DEFINES index cead305aee..10b558d1ad 100644 --- a/admin/CPP-DEFINES +++ b/admin/CPP-DEFINES @@ -205,7 +205,6 @@ HAVE_LIBXML2 HAVE_LIBXMU HAVE_LOCALTIME_R HAVE_LOCAL_SOCKETS -HAVE_LONG_FILE_NAMES HAVE_LONG_LONG_INT HAVE_LRAND48 HAVE_LSTAT diff --git a/admin/merge-gnulib b/admin/merge-gnulib index a16d7fa53e..7eca64305d 100755 --- a/admin/merge-gnulib +++ b/admin/merge-gnulib @@ -39,8 +39,8 @@ GNULIB_MODULES= manywarnings memrchr minmax mkostemp mktime nstrftime pipe2 pselect pthread_sigmask putenv qcopy-acl readlink readlinkat sig2str socklen stat-time std-gnu11 stdalign stddef stdio - stpcpy strtoimax symlink sys_stat - sys_time time time_r time_rz timegm timer-time timespec-add timespec-sub + stpcpy strtoimax symlink sys_stat sys_time + tempname time time_r time_rz timegm timer-time timespec-add timespec-sub update-copyright unlocked-io utimens vla warnings ' diff --git a/configure.ac b/configure.ac index 9f80620a80..86d5b3e94f 100644 --- a/configure.ac +++ b/configure.ac @@ -1779,9 +1779,6 @@ AC_DEFUN fi AC_SUBST(AUTO_DEPEND) -dnl checks for operating system services -AC_SYS_LONG_FILE_NAMES - #### Choose a window system. ## We leave window_system equal to none if diff --git a/lib/gnulib.mk.in b/lib/gnulib.mk.in index c5df3f42e4..30986b4ed7 100644 --- a/lib/gnulib.mk.in +++ b/lib/gnulib.mk.in @@ -21,7 +21,7 @@ # the same distribution terms as the rest of that program. # # Generated by gnulib-tool. -# Reproduce by: gnulib-tool --import --lib=libgnu --source-base=lib --m4-base=m4 --doc-base=doc --tests-base=tests --aux-dir=build-aux --avoid=close --avoid=dup --avoid=fchdir --avoid=fstat --avoid=malloc-posix --avoid=msvc-inval --avoid=msvc-nothrow --avoid=open --avoid=openat-die --avoid=opendir --avoid=raise --avoid=save-cwd --avoid=select --avoid=setenv --avoid=sigprocmask --avoid=stat --avoid=stdarg --avoid=stdbool --avoid=threadlib --avoid=tzset --avoid=unsetenv --avoid=utime --avoid=utime-h --gnu-make --makefile-name=gnulib.mk.in --conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca-opt binary-io byteswap c-ctype c-strcase careadlinkat close-stream count-leading-zeros count-one-bits count-trailing-zeros crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512 d-type diffseq dtoastr dtotimespec dup2 environ execinfo explicit_bzero faccessat fcntl fcntl-h fdatasync fdopendir filemode filevercmp flexmember fstatat fsync getloadavg getopt-gnu gettime gettimeofday gitlog-to-changelog ignore-value intprops largefile lstat manywarnings memrchr minmax mkostemp mktime nstrftime pipe2 pselect pthread_sigmask putenv qcopy-acl readlink readlinkat sig2str socklen stat-time std-gnu11 stdalign stddef stdio stpcpy strtoimax symlink sys_stat sys_time time time_r time_rz timegm timer-time timespec-add timespec-sub unlocked-io update-copyright utimens vla warnings +# Reproduce by: gnulib-tool --import --lib=libgnu --source-base=lib --m4-base=m4 --doc-base=doc --tests-base=tests --aux-dir=build-aux --avoid=close --avoid=dup --avoid=fchdir --avoid=fstat --avoid=malloc-posix --avoid=msvc-inval --avoid=msvc-nothrow --avoid=open --avoid=openat-die --avoid=opendir --avoid=raise --avoid=save-cwd --avoid=select --avoid=setenv --avoid=sigprocmask --avoid=stat --avoid=stdarg --avoid=stdbool --avoid=threadlib --avoid=tzset --avoid=unsetenv --avoid=utime --avoid=utime-h --gnu-make --makefile-name=gnulib.mk.in --conditional-dependencies --no-libtool --macro-prefix=gl --no-vc-files alloca-opt binary-io byteswap c-ctype c-strcase careadlinkat close-stream count-leading-zeros count-one-bits count-trailing-zeros crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512 d-type diffseq dtoastr dtotimespec dup2 environ execinfo explicit_bzero faccessat fcntl fcntl-h fdatasync fdopendir filemode filevercmp flexmember fstatat fsync getloadavg getopt-gnu gettime gettimeofday gitlog-to-changelog ignore-value intprops largefile lstat manywarnings memrchr minmax mkostemp mktime nstrftime pipe2 pselect pthread_sigmask putenv qcopy-acl readlink readlinkat sig2str socklen stat-time std-gnu11 stdalign stddef stdio stpcpy strtoimax symlink sys_stat sys_time tempname time time_r time_rz timegm timer-time timespec-add timespec-sub unlocked-io update-copyright utimens vla warnings MOSTLYCLEANFILES += core *.stackdump @@ -904,7 +904,6 @@ gl_GNULIB_ENABLED_euidaccess = @gl_GNULIB_ENABLED_euidaccess@ gl_GNULIB_ENABLED_getdtablesize = @gl_GNULIB_ENABLED_getdtablesize@ gl_GNULIB_ENABLED_getgroups = @gl_GNULIB_ENABLED_getgroups@ gl_GNULIB_ENABLED_strtoll = @gl_GNULIB_ENABLED_strtoll@ -gl_GNULIB_ENABLED_tempname = @gl_GNULIB_ENABLED_tempname@ gl_LIBOBJS = @gl_LIBOBJS@ gl_LTLIBOBJS = @gl_LTLIBOBJS@ gltests_LIBOBJS = @gltests_LIBOBJS@ @@ -2701,10 +2700,8 @@ endif ## begin gnulib module tempname ifeq (,$(OMIT_GNULIB_MODULE_tempname)) -ifneq (,$(gl_GNULIB_ENABLED_tempname)) libgnu_a_SOURCES += tempname.c -endif EXTRA_DIST += tempname.h endif diff --git a/lisp/files.el b/lisp/files.el index f2758ab18c..3ed36d24db 100644 --- a/lisp/files.el +++ b/lisp/files.el @@ -1417,6 +1417,19 @@ make-temp-file If DIR-FLAG is non-nil, create a new empty directory instead of a file. If SUFFIX is non-nil, add that at the end of the file name." + (let ((absolute-prefix (expand-file-name prefix temporary-file-directory))) + ;; FIXME: Establish make-temp-file file name handlers, + ;; to create temporary files and directories without races. + ;; For now, if the prefix is magic, fall back on the traditional, + ;; insecure way to create temporary files. + (if (find-file-name-handler absolute-prefix 'write-region) + (files--make-magic-temp-file prefix dir-flag suffix) + (fileio--make-temp-file absolute-prefix + (if dir-flag t) (or suffix ""))))) + +(defun files--make-magic-temp-file (prefix &optional dir-flag suffix) + "Implement (make-temp-file PREFIX DIR-FLAG SUFFIX), +even if the result is a magic file name." ;; Create temp files with strict access rights. It's easy to ;; loosen them later, whereas it's impossible to close the ;; time-window of loose permissions otherwise. @@ -1435,6 +1448,8 @@ make-temp-file (setq file (concat file suffix))) (if dir-flag (make-directory file) + ;; FIXME: The 'excl is only aspirational; it is + ;; actually a no-op if FILE is a magic file name. (write-region "" nil file nil 'silent nil 'excl)) nil) (file-already-exists t)) diff --git a/m4/gnulib-comp.m4 b/m4/gnulib-comp.m4 index 69d77229bf..d1089860e1 100644 --- a/m4/gnulib-comp.m4 +++ b/m4/gnulib-comp.m4 @@ -387,6 +387,7 @@ AC_DEFUN AC_PROG_MKDIR_P gl_SYS_TYPES_H AC_PROG_MKDIR_P + gl_FUNC_GEN_TEMPNAME gl_HEADER_TIME_H gl_TIME_R if test $HAVE_LOCALTIME_R = 0 || test $REPLACE_LOCALTIME_R = 1; then @@ -424,7 +425,6 @@ AC_DEFUN gl_gnulib_enabled_03e0aaad4cb89ca757653bd367a6ccb7=false gl_gnulib_enabled_6099e9737f757db36c47fa9d9f02e88c=false gl_gnulib_enabled_strtoll=false - gl_gnulib_enabled_tempname=false gl_gnulib_enabled_682e609604ccaac6be382e4ee3a4eaec=false func_gl_gnulib_m4code_260941c0e5dc67ec9e87d1fb321c300b () { @@ -560,13 +560,6 @@ AC_DEFUN gl_gnulib_enabled_strtoll=true fi } - func_gl_gnulib_m4code_tempname () - { - if ! $gl_gnulib_enabled_tempname; then - gl_FUNC_GEN_TEMPNAME - gl_gnulib_enabled_tempname=true - fi - } func_gl_gnulib_m4code_682e609604ccaac6be382e4ee3a4eaec () { if ! $gl_gnulib_enabled_682e609604ccaac6be382e4ee3a4eaec; then @@ -612,9 +605,6 @@ AC_DEFUN if test $REPLACE_LSTAT = 1; then func_gl_gnulib_m4code_dosname fi - if test $HAVE_MKOSTEMP = 0; then - func_gl_gnulib_m4code_tempname - fi if test $HAVE_READLINKAT = 0; then func_gl_gnulib_m4code_260941c0e5dc67ec9e87d1fb321c300b fi @@ -644,7 +634,6 @@ AC_DEFUN AM_CONDITIONAL([gl_GNULIB_ENABLED_03e0aaad4cb89ca757653bd367a6ccb7], [$gl_gnulib_enabled_03e0aaad4cb89ca757653bd367a6ccb7]) AM_CONDITIONAL([gl_GNULIB_ENABLED_6099e9737f757db36c47fa9d9f02e88c], [$gl_gnulib_enabled_6099e9737f757db36c47fa9d9f02e88c]) AM_CONDITIONAL([gl_GNULIB_ENABLED_strtoll], [$gl_gnulib_enabled_strtoll]) - AM_CONDITIONAL([gl_GNULIB_ENABLED_tempname], [$gl_gnulib_enabled_tempname]) AM_CONDITIONAL([gl_GNULIB_ENABLED_682e609604ccaac6be382e4ee3a4eaec], [$gl_gnulib_enabled_682e609604ccaac6be382e4ee3a4eaec]) # End of code from modules m4_ifval(gl_LIBSOURCES_LIST, [ diff --git a/src/buffer.c b/src/buffer.c index 0d0f43e937..2d508f35cf 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -1085,7 +1085,6 @@ is first appended to NAME, to speed up finding a non-existent buffer. */) genbase = name; else { - /* Note fileio.c:make_temp_name does random differently. */ char number[sizeof "-999999"]; int i = XFASTINT (Frandom (make_number (999999))); AUTO_STRING_WITH_LEN (lnumber, number, sprintf (number, "-%d", i)); diff --git a/src/fileio.c b/src/fileio.c index 15845e3914..1ef9bfa82b 100644 --- a/src/fileio.c +++ b/src/fileio.c @@ -97,6 +97,7 @@ along with GNU Emacs. If not, see . */ #include #include #include +#include #include @@ -623,149 +624,67 @@ In Unix-syntax, this function just removes the final slash. */) return val; } -static const char make_temp_name_tbl[64] = -{ - 'A','B','C','D','E','F','G','H', - 'I','J','K','L','M','N','O','P', - 'Q','R','S','T','U','V','W','X', - 'Y','Z','a','b','c','d','e','f', - 'g','h','i','j','k','l','m','n', - 'o','p','q','r','s','t','u','v', - 'w','x','y','z','0','1','2','3', - '4','5','6','7','8','9','-','_' -}; - -static unsigned make_temp_name_count, make_temp_name_count_initialized_p; - -/* Value is a temporary file name starting with PREFIX, a string. +DEFUN ("fileio--make-temp-file", Ffileio__make_temp_file, + Sfileio__make_temp_file, 3, 3, 0, + doc: /* Generate a new file whose name starts with PREFIX, a string. +Return the name of the generated file. If DIR-FLAG is zero, do not +create the file, just its name. Otherwise, if DIR-FLAG is non-nil, +create an empty directory. The file name should end in SUFFIX. - The Emacs process number forms part of the result, so there is - no danger of generating a name being used by another process. - In addition, this function makes an attempt to choose a name - which has no existing file. To make this work, PREFIX should be - an absolute file name. +Signal an error if the file could not be created. - BASE64_P means add the pid as 3 characters in base64 - encoding. In this case, 6 characters will be added to PREFIX to - form the file name. Otherwise, if Emacs is running on a system - with long file names, add the pid as a decimal number. - - This function signals an error if no unique file name could be - generated. */ - -Lisp_Object -make_temp_name (Lisp_Object prefix, bool base64_p) +This function does not grok magic file names. */) + (Lisp_Object prefix, Lisp_Object dir_flag, Lisp_Object suffix) { - Lisp_Object val, encoded_prefix; - ptrdiff_t len; - printmax_t pid; - char *p, *data; - char pidbuf[INT_BUFSIZE_BOUND (printmax_t)]; - int pidlen; - - CHECK_STRING (prefix); - - /* VAL is created by adding 6 characters to PREFIX. The first - three are the PID of this process, in base 64, and the second - three are incremented if the file already exists. This ensures - 262144 unique file names per PID per PREFIX. */ - - pid = getpid (); - - if (base64_p) - { - pidbuf[0] = make_temp_name_tbl[pid & 63], pid >>= 6; - pidbuf[1] = make_temp_name_tbl[pid & 63], pid >>= 6; - pidbuf[2] = make_temp_name_tbl[pid & 63], pid >>= 6; - pidlen = 3; - } - else + bool make_temp_name = EQ (dir_flag, make_number (0)); + CHECK_STRING (suffix); + if (!make_temp_name) + prefix = Fexpand_file_name (prefix, Vtemporary_file_directory); + + Lisp_Object encoded_prefix = ENCODE_FILE (prefix); + Lisp_Object encoded_suffix = ENCODE_FILE (suffix); + ptrdiff_t prefix_len = SBYTES (encoded_prefix); + ptrdiff_t suffix_len = SBYTES (encoded_suffix); + if (INT_MAX < suffix_len) + args_out_of_range (prefix, suffix); + int nX = 6; + Lisp_Object val = make_uninit_string (prefix_len + nX + suffix_len); + char *data = SSDATA (val); + memcpy (data, SSDATA (encoded_prefix), prefix_len); + memset (data + prefix_len, 'X', nX); + memcpy (data + prefix_len + nX, SSDATA (encoded_suffix), suffix_len); + int kind = (NILP (dir_flag) ? GT_FILE + : make_temp_name ? GT_NOCREATE + : GT_DIR); + int fd = gen_tempname (data, suffix_len, O_BINARY | O_CLOEXEC, kind); + if (fd < 0 || (NILP (dir_flag) && emacs_close (fd) != 0)) { -#ifdef HAVE_LONG_FILE_NAMES - pidlen = sprintf (pidbuf, "%"pMd, pid); -#else - pidbuf[0] = make_temp_name_tbl[pid & 63], pid >>= 6; - pidbuf[1] = make_temp_name_tbl[pid & 63], pid >>= 6; - pidbuf[2] = make_temp_name_tbl[pid & 63], pid >>= 6; - pidlen = 3; -#endif - } - - encoded_prefix = ENCODE_FILE (prefix); - len = SBYTES (encoded_prefix); - val = make_uninit_string (len + 3 + pidlen); - data = SSDATA (val); - memcpy (data, SSDATA (encoded_prefix), len); - p = data + len; - - memcpy (p, pidbuf, pidlen); - p += pidlen; - - /* Here we try to minimize useless stat'ing when this function is - invoked many times successively with the same PREFIX. We achieve - this by initializing count to a random value, and incrementing it - afterwards. - - We don't want make-temp-name to be called while dumping, - because then make_temp_name_count_initialized_p would get set - and then make_temp_name_count would not be set when Emacs starts. */ - - if (!make_temp_name_count_initialized_p) - { - make_temp_name_count = time (NULL); - make_temp_name_count_initialized_p = 1; - } - - while (1) - { - unsigned num = make_temp_name_count; - - p[0] = make_temp_name_tbl[num & 63], num >>= 6; - p[1] = make_temp_name_tbl[num & 63], num >>= 6; - p[2] = make_temp_name_tbl[num & 63], num >>= 6; - - /* Poor man's congruential RN generator. Replace with - ++make_temp_name_count for debugging. */ - make_temp_name_count += 25229; - make_temp_name_count %= 225307; - - if (!check_existing (data)) + static char const kind_message[][32] = { - /* We want to return only if errno is ENOENT. */ - if (errno == ENOENT) - return DECODE_FILE (val); - else - /* The error here is dubious, but there is little else we - can do. The alternatives are to return nil, which is - as bad as (and in many cases worse than) throwing the - error, or to ignore the error, which will likely result - in looping through 225307 stat's, which is not only - dog-slow, but also useless since eventually nil would - have to be returned anyway. */ - report_file_error ("Cannot create temporary name for prefix", - prefix); - /* not reached */ - } + [GT_FILE] = "Creating file with prefix", + [GT_DIR] = "Creating directory with prefix", + [GT_NOCREATE] = "Creating file name with prefix" + }; + report_file_error (kind_message[kind], prefix); } + return DECODE_FILE (val); } DEFUN ("make-temp-name", Fmake_temp_name, Smake_temp_name, 1, 1, 0, doc: /* Generate temporary file name (string) starting with PREFIX (a string). -The Emacs process number forms part of the result, so there is no -danger of generating a name being used by another Emacs process -\(so long as only a single host can access the containing directory...). This function tries to choose a name that has no existing file. For this to work, PREFIX should be an absolute file name, and PREFIX and the returned string should both be non-magic. -There is a race condition between calling `make-temp-name' and creating the -file, which opens all kinds of security holes. For that reason, you should -normally use `make-temp-file' instead. */) +There is a race condition between calling `make-temp-name' and +later creating the file, which opens all kinds of security holes. +For that reason, you should normally use `make-temp-file' instead. */) (Lisp_Object prefix) { - return make_temp_name (prefix, 0); + return Ffileio__make_temp_file (prefix, make_number (0), + empty_unibyte_string); } DEFUN ("expand-file-name", Fexpand_file_name, Sexpand_file_name, 1, 2, 0, @@ -6167,6 +6086,7 @@ This includes interactive calls to `delete-file' and defsubr (&Sunhandled_file_name_directory); defsubr (&Sfile_name_as_directory); defsubr (&Sdirectory_file_name); + defsubr (&Sfileio__make_temp_file); defsubr (&Smake_temp_name); defsubr (&Sexpand_file_name); defsubr (&Ssubstitute_in_file_name); diff --git a/src/filelock.c b/src/filelock.c index dd8cb28c42..5614657985 100644 --- a/src/filelock.c +++ b/src/filelock.c @@ -206,14 +206,11 @@ get_boot_time (void) WTMP_FILE, counter); if (! NILP (Ffile_exists_p (tempname))) { - /* The utmp functions on mescaline.gnu.org accept only - file names up to 8 characters long. Choose a 2 - character long prefix, and call make_temp_file with - second arg non-zero, so that it will add not more - than 6 characters to the prefix. */ - filename = Fexpand_file_name (build_string ("wt"), - Vtemporary_file_directory); - filename = make_temp_name (filename, 1); + /* The utmp functions on older systems accept only file + names up to 8 bytes long. Choose a 2 byte prefix, so + the 6-byte suffix does not make the name too long. */ + filename = Ffileio__make_temp_file (build_string ("wt"), Qnil, + empty_unibyte_string); CALLN (Fcall_process, build_string ("gzip"), Qnil, list2 (QCfile, filename), Qnil, build_string ("-cd"), tempname); diff --git a/src/lisp.h b/src/lisp.h index 4de6fc85ec..a65cc9684a 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -4013,7 +4013,6 @@ extern bool file_directory_p (const char *); extern bool file_accessible_directory_p (Lisp_Object); extern void init_fileio (void); extern void syms_of_fileio (void); -extern Lisp_Object make_temp_name (Lisp_Object, bool); /* Defined in search.c. */ extern void shrink_regexp_cache (void); -- 2.13.4