* bug#28023: fix make-temp-file race on local host @ 2017-08-09 5:38 Paul Eggert 2017-08-09 6:51 ` Michael Albinus 2017-08-09 16:05 ` Eli Zaretskii 0 siblings, 2 replies; 23+ messages in thread From: Paul Eggert @ 2017-08-09 5:38 UTC (permalink / raw) To: 28023 [-- Attachment #1: Type: text/plain, Size: 333 bytes --] Tags: patch Severity: important I plan to install the attached patch in master soon, and am sending it for review to bug-gnu-emacs first in case there's some problem with it on MS-Windows. Although this patch does not fix all instances of the race in Gnu Emacs (notably, Tramp access is still affected) it's a good start. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Fix-make-temp-file-race-on-local-files.patch --] [-- Type: text/x-patch; name="0001-Fix-make-temp-file-race-on-local-files.patch", Size: 22211 bytes --] From 0f536834ae618adc38883fc331f75a2847a8411c Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Tue, 8 Aug 2017 22:22:48 -0700 Subject: [PATCH] Fix make-temp-file race on local files For the motivation behind this patch, please see: 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 | 8 +-- lisp/files.el | 15 +++++ m4/gnulib-comp.m4 | 34 +++-------- src/buffer.c | 1 - src/fileio.c | 170 ++++++++++++++--------------------------------------- src/filelock.c | 11 ++-- src/lisp.h | 1 - 10 files changed, 75 insertions(+), 173 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 c23e8a40ea..c7a7a1d7c9 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 a385c8c838..8b34ee6354 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 @@ -903,9 +903,7 @@ gl_GNULIB_ENABLED_dosname = @gl_GNULIB_ENABLED_dosname@ 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_secure_getenv = @gl_GNULIB_ENABLED_secure_getenv@ 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@ @@ -1915,9 +1913,7 @@ endif ## begin gnulib module secure_getenv ifeq (,$(OMIT_GNULIB_MODULE_secure_getenv)) -ifneq (,$(gl_GNULIB_ENABLED_secure_getenv)) -endif EXTRA_DIST += secure_getenv.c EXTRA_libgnu_a_SOURCES += secure_getenv.c @@ -2715,10 +2711,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 188c116c85..fc86237ee9 100644 --- a/m4/gnulib-comp.m4 +++ b/m4/gnulib-comp.m4 @@ -347,6 +347,12 @@ AC_DEFUN AC_LIBOBJ([readlinkat]) fi gl_UNISTD_MODULE_INDICATOR([readlinkat]) + gl_FUNC_SECURE_GETENV + if test $HAVE_SECURE_GETENV = 0; then + AC_LIBOBJ([secure_getenv]) + gl_PREREQ_SECURE_GETENV + fi + gl_STDLIB_MODULE_INDICATOR([secure_getenv]) gl_FUNC_SIG2STR if test $ac_cv_func_sig2str = no; then AC_LIBOBJ([sig2str]) @@ -388,6 +394,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,9 +431,7 @@ AC_DEFUN gl_gnulib_enabled_5264294aa0a5557541b53c8c741f7f31=false gl_gnulib_enabled_03e0aaad4cb89ca757653bd367a6ccb7=false gl_gnulib_enabled_6099e9737f757db36c47fa9d9f02e88c=false - gl_gnulib_enabled_secure_getenv=false gl_gnulib_enabled_strtoll=false - gl_gnulib_enabled_tempname=false gl_gnulib_enabled_682e609604ccaac6be382e4ee3a4eaec=false func_gl_gnulib_m4code_260941c0e5dc67ec9e87d1fb321c300b () { @@ -550,18 +555,6 @@ AC_DEFUN gl_gnulib_enabled_6099e9737f757db36c47fa9d9f02e88c=true fi } - func_gl_gnulib_m4code_secure_getenv () - { - if ! $gl_gnulib_enabled_secure_getenv; then - gl_FUNC_SECURE_GETENV - if test $HAVE_SECURE_GETENV = 0; then - AC_LIBOBJ([secure_getenv]) - gl_PREREQ_SECURE_GETENV - fi - gl_STDLIB_MODULE_INDICATOR([secure_getenv]) - gl_gnulib_enabled_secure_getenv=true - fi - } func_gl_gnulib_m4code_strtoll () { if ! $gl_gnulib_enabled_strtoll; then @@ -574,14 +567,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 - func_gl_gnulib_m4code_secure_getenv - fi - } func_gl_gnulib_m4code_682e609604ccaac6be382e4ee3a4eaec () { if ! $gl_gnulib_enabled_682e609604ccaac6be382e4ee3a4eaec; then @@ -627,9 +612,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 @@ -658,9 +640,7 @@ AC_DEFUN AM_CONDITIONAL([gl_GNULIB_ENABLED_5264294aa0a5557541b53c8c741f7f31], [$gl_gnulib_enabled_5264294aa0a5557541b53c8c741f7f31]) 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_secure_getenv], [$gl_gnulib_enabled_secure_getenv]) 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 <http://www.gnu.org/licenses/>. */ #include <allocator.h> #include <careadlinkat.h> #include <stat-time.h> +#include <tempname.h> #include <binary-io.h> @@ -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..de0ca01052 100644 --- a/src/filelock.c +++ b/src/filelock.c @@ -206,14 +206,13 @@ 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. */ + /* 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 = Fexpand_file_name (build_string ("wt"), Vtemporary_file_directory); - filename = make_temp_name (filename, 1); + filename = Ffileio__make_temp_file (filename, 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.3 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* bug#28023: fix make-temp-file race on local host 2017-08-09 5:38 bug#28023: fix make-temp-file race on local host Paul Eggert @ 2017-08-09 6:51 ` Michael Albinus 2017-08-09 9:22 ` Paul Eggert 2017-08-09 16:05 ` Eli Zaretskii 1 sibling, 1 reply; 23+ messages in thread From: Michael Albinus @ 2017-08-09 6:51 UTC (permalink / raw) To: Paul Eggert; +Cc: 28023 Paul Eggert <eggert@cs.ucla.edu> writes: Hi Paul, > I plan to install the attached patch in master soon, and am sending it > for review to bug-gnu-emacs first in case there's some problem with it > on MS-Windows. Although this patch does not fix all instances of the > race in Gnu Emacs (notably, Tramp access is still affected) it's a > good start. I haven't followed the whole thread, sorry. Could you pls elaborate, what you do expect from Tramp? Best regards, Michael. ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#28023: fix make-temp-file race on local host 2017-08-09 6:51 ` Michael Albinus @ 2017-08-09 9:22 ` Paul Eggert 2017-08-09 11:21 ` Michael Albinus 2017-08-12 10:35 ` Michael Albinus 0 siblings, 2 replies; 23+ messages in thread From: Paul Eggert @ 2017-08-09 9:22 UTC (permalink / raw) To: Michael Albinus; +Cc: 28023 Michael Albinus wrote: > what you do expect from Tramp? Tramp needs to support a new method make-temp-file that creates a file (or directory) atomically, as make-temp-file does locally now. Tramp also needs to support the excl flag of write-region (currently it ignores that flag). I was planning to write this up as a bug report after the patch goes in. ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#28023: fix make-temp-file race on local host 2017-08-09 9:22 ` Paul Eggert @ 2017-08-09 11:21 ` Michael Albinus 2017-08-13 7:21 ` Paul Eggert 2017-08-12 10:35 ` Michael Albinus 1 sibling, 1 reply; 23+ messages in thread From: Michael Albinus @ 2017-08-09 11:21 UTC (permalink / raw) To: Paul Eggert; +Cc: 28023 Paul Eggert <eggert@cs.ucla.edu> writes: > Tramp needs to support a new method make-temp-file that creates a file > (or directory) atomically, as make-temp-file does locally now. That means, make-temp-file shall be converted into a magic file name operation. I'll do my best, but I don't know whether we could get an implementation for all Tramp methods w/o a race condition. > Tramp also needs to support the excl flag of write-region (currently > it ignores that flag). This sounds trivial. And yes, it will help. > I was planning to write this up as a bug report after the patch goes > in. Pls do. Best regards, Michael. ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#28023: fix make-temp-file race on local host 2017-08-09 11:21 ` Michael Albinus @ 2017-08-13 7:21 ` Paul Eggert 0 siblings, 0 replies; 23+ messages in thread From: Paul Eggert @ 2017-08-13 7:21 UTC (permalink / raw) To: Michael Albinus; +Cc: 28023-done Michael Albinus wrote: > Paul Eggert<eggert@cs.ucla.edu> writes: > >> Tramp needs to support a new method make-temp-file that creates a file >> (or directory) atomically, as make-temp-file does locally now. > That means, make-temp-file shall be converted into a magic file name > operation. I'll do my best, but I don't know whether we could get an > implementation for all Tramp methods w/o a race condition. > >> Tramp also needs to support the excl flag of write-region (currently >> it ignores that flag). > This sounds trivial. And yes, it will help. Thanks for doing that. Because of that, I'm no longer seeing a race in make-temp-file, even for Tramp files. That is, although my proposed patch is still a performance win on local files, I don't see how it is a correctness win any more. It's still worth installing for the performance reasons, though, so I did that and I am marking this bug as done. >> I was planning to write this up as a bug report after the patch goes >> in. > Pls do. To some extent this is moot now, if I understand things correctly. That is, the only reason to write this up now would be for performance reasons, not a race condition. There is still a race involving destination directories, for both Tramp and non-Tramp versions. I plan to take a look at that next. ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#28023: fix make-temp-file race on local host 2017-08-09 9:22 ` Paul Eggert 2017-08-09 11:21 ` Michael Albinus @ 2017-08-12 10:35 ` Michael Albinus 2017-08-12 15:55 ` Paul Eggert 1 sibling, 1 reply; 23+ messages in thread From: Michael Albinus @ 2017-08-12 10:35 UTC (permalink / raw) To: Paul Eggert; +Cc: 28023 Paul Eggert <eggert@cs.ucla.edu> writes: Hi Paul, > Tramp also needs to support the excl flag of write-region (currently > it ignores that flag). I've implemented this, commit ec5cfaa456. Best regards, Michael. ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#28023: fix make-temp-file race on local host 2017-08-12 10:35 ` Michael Albinus @ 2017-08-12 15:55 ` Paul Eggert 2017-08-12 16:09 ` Eli Zaretskii 2017-08-12 17:51 ` Michael Albinus 0 siblings, 2 replies; 23+ messages in thread From: Paul Eggert @ 2017-08-12 15:55 UTC (permalink / raw) To: Michael Albinus; +Cc: 28023 [-- Attachment #1: Type: text/plain, Size: 354 bytes --] Michael Albinus wrote: > I've implemented this, commit ec5cfaa456. Thanks. Unfortunately this caused "make check" to fail on Fedora 26. I installed the attached patch, which causes the test to pass. Can you please look over it to see whether it makes sense, and look for similar problems elsewhere? (I'm not that familiar with Tramp.) Thanks. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Adjust-jka-compr-to-recent-Tramp-changes.patch --] [-- Type: text/x-patch; name="0001-Adjust-jka-compr-to-recent-Tramp-changes.patch", Size: 1703 bytes --] From bbf52c142afbb9e10bf2ae20b3c77993fda26b43 Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Sat, 12 Aug 2017 08:52:25 -0700 Subject: [PATCH] Adjust jka-compr to recent Tramp changes. * lisp/jka-compr.el (jka-compr-write-region): Two new args LOCKNAME and MUSTBENEW. --- lisp/jka-compr.el | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lisp/jka-compr.el b/lisp/jka-compr.el index 26a7cf5..9e780f8 100644 --- a/lisp/jka-compr.el +++ b/lisp/jka-compr.el @@ -252,7 +252,8 @@ jka-compr-make-temp-name "This routine will return the name of a new file." (make-temp-file jka-compr-temp-name-template)) -(defun jka-compr-write-region (start end file &optional append visit) +(defun jka-compr-write-region (start end file &optional + append visit lockname mustbenew) (let* ((filename (expand-file-name file)) (visit-file (if (stringp visit) (expand-file-name visit) filename)) (info (jka-compr-get-compression-info visit-file)) @@ -334,7 +335,8 @@ jka-compr-write-region (jka-compr-run-real-handler 'write-region (list (point-min) (point-max) filename - (and append can-append) 'dont)) + (and append can-append) 'dont + lockname mustbenew)) (erase-buffer)) ) (delete-file temp-file) @@ -365,7 +367,8 @@ jka-compr-write-region nil) (jka-compr-run-real-handler 'write-region - (list start end filename append visit))))) + (list start end filename append visit + lockname mustbenew))))) (defun jka-compr-insert-file-contents (file &optional visit beg end replace) -- 2.7.4 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* bug#28023: fix make-temp-file race on local host 2017-08-12 15:55 ` Paul Eggert @ 2017-08-12 16:09 ` Eli Zaretskii 2017-08-12 16:25 ` Paul Eggert 2017-08-12 17:51 ` Michael Albinus 1 sibling, 1 reply; 23+ messages in thread From: Eli Zaretskii @ 2017-08-12 16:09 UTC (permalink / raw) To: Paul Eggert; +Cc: 28023, michael.albinus Paul, Looking back at your original patch, I think the function you are adding should be named make-temp-file-internal, as we do with other functions whose low-level parts are implemented in C. fileio--SOMETHING sounds like something we never had, so I think it's better to a void a new prefix. Thanks. ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#28023: fix make-temp-file race on local host 2017-08-12 16:09 ` Eli Zaretskii @ 2017-08-12 16:25 ` Paul Eggert 2017-08-12 16:52 ` Eli Zaretskii 0 siblings, 1 reply; 23+ messages in thread From: Paul Eggert @ 2017-08-12 16:25 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 28023, michael.albinus Eli Zaretskii wrote: > I think the function you are > adding should be named make-temp-file-internal, as we do with other > functions whose low-level parts are implemented in C. I was following the lead of names like lread--substitute-command-keys, print--preprocess, and thread--blocker, all low-level C functions whose first part identifies which C module they're in. Although I see that the "-internal" suffix is more popular for this sort of thing, isn't that a revenant of the old days, before we instituted the convention of using PREFIX--NAME for private names? Or is the "-internal" suffix a separate naming convention, used by both Lisp and C code, that has a different semantics from PREFIX--NAME? If so, it would be nice to have advice somewhere as to when to use the -internal suffix vs when to use PREFIX--NAME. ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#28023: fix make-temp-file race on local host 2017-08-12 16:25 ` Paul Eggert @ 2017-08-12 16:52 ` Eli Zaretskii 2017-08-12 17:57 ` Paul Eggert 0 siblings, 1 reply; 23+ messages in thread From: Eli Zaretskii @ 2017-08-12 16:52 UTC (permalink / raw) To: Paul Eggert; +Cc: 28023, michael.albinus > Cc: michael.albinus@gmx.de, 28023@debbugs.gnu.org > From: Paul Eggert <eggert@cs.ucla.edu> > Date: Sat, 12 Aug 2017 09:25:37 -0700 > > I was following the lead of names like lread--substitute-command-keys, > print--preprocess, and thread--blocker, all low-level C functions whose first > part identifies which C module they're in. Although I see that the "-internal" > suffix is more popular for this sort of thing, isn't that a revenant of the old > days, before we instituted the convention of using PREFIX--NAME for private > names? Or is the "-internal" suffix a separate naming convention, used by both > Lisp and C code, that has a different semantics from PREFIX--NAME? If so, it > would be nice to have advice somewhere as to when to use the -internal suffix vs > when to use PREFIX--NAME. I think the prefix that comes from the file where the function is defined is more of a Lisp convention, and the -internal convention is more for C implementations. ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#28023: fix make-temp-file race on local host 2017-08-12 16:52 ` Eli Zaretskii @ 2017-08-12 17:57 ` Paul Eggert 2017-08-12 18:07 ` Eli Zaretskii 0 siblings, 1 reply; 23+ messages in thread From: Paul Eggert @ 2017-08-12 17:57 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 28023, michael.albinus [-- Attachment #1: Type: text/plain, Size: 523 bytes --] Eli Zaretskii wrote: > I think the prefix that comes from the file where the function is > defined is more of a Lisp convention, and the -internal convention is > more for C implementations. I see lots of exceptions to these conventions, presumably either because developers don't know the intent or because the code was written before the conventions were established. Still, it's better to document the intent so I installed the attached patch to do that. I'll adjust the make-temp-file patch accordingly. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Document-internal-use-naming-conventions.patch --] [-- Type: text/x-patch; name="0001-Document-internal-use-naming-conventions.patch", Size: 2247 bytes --] From 57c2de6fce73b77081e59d0527b12f158bac758a Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Sat, 12 Aug 2017 10:54:32 -0700 Subject: [PATCH] Document internal-use naming conventions * doc/lispref/functions.texi (Function Names): * doc/lispref/variables.texi (Tips for Defining): Document naming conventions for internal-use functions and vars. See Bug#28023#59. --- doc/lispref/functions.texi | 9 +++++++++ doc/lispref/variables.texi | 10 ++++++++++ 2 files changed, 19 insertions(+) diff --git a/doc/lispref/functions.texi b/doc/lispref/functions.texi index 283f74f..06de2e2 100644 --- a/doc/lispref/functions.texi +++ b/doc/lispref/functions.texi @@ -544,6 +544,15 @@ Function Names conflict. (This is not the case in some dialects of Lisp, like Scheme.) + By convention, if a function's symbol consists of two names +separated by @samp{--}, the function is intended for internal use and +the first part names the file defining the function. For example, a +function named @code{vc-git--rev-parse} is an internal function +defined in @file{vc-git.el}. Internal-use functions written in C have +names ending in @samp{-internal}, e.g., @code{bury-buffer-internal}. +Emacs code contributed before 2018 may follow other internal-use +naming conventions, which are being phased out. + @node Defining Functions @section Defining Functions @cindex defining a function diff --git a/doc/lispref/variables.texi b/doc/lispref/variables.texi index 2818ea0..7650ed4 100644 --- a/doc/lispref/variables.texi +++ b/doc/lispref/variables.texi @@ -574,6 +574,16 @@ Tips for Defining @item @dots{}-switches The value specifies options for a command. + +@item @var{prefix}--@dots{} +The variable is intended for internal use and is defined in the file +@file{@var{prefix}.el}. (Emacs code contributed before 2018 may +follow other conventions, which are being phased out.) + +@item @dots{}-internal +The variable is intended for internal use and is defined in C code. +(Emacs code contributed before 2018 may follow other conventions, +which are being phased out.) @end table When you define a variable, always consider whether you should mark -- 2.7.4 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* bug#28023: fix make-temp-file race on local host 2017-08-12 17:57 ` Paul Eggert @ 2017-08-12 18:07 ` Eli Zaretskii 0 siblings, 0 replies; 23+ messages in thread From: Eli Zaretskii @ 2017-08-12 18:07 UTC (permalink / raw) To: Paul Eggert; +Cc: 28023, michael.albinus > Cc: michael.albinus@gmx.de, 28023@debbugs.gnu.org > From: Paul Eggert <eggert@cs.ucla.edu> > Date: Sat, 12 Aug 2017 10:57:26 -0700 > > I see lots of exceptions to these conventions, presumably either because > developers don't know the intent or because the code was written before the > conventions were established. Still, it's better to document the intent so I > installed the attached patch to do that. Thanks, I agree this should be documented. > I'll adjust the make-temp-file patch accordingly. Thanks. ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#28023: fix make-temp-file race on local host 2017-08-12 15:55 ` Paul Eggert 2017-08-12 16:09 ` Eli Zaretskii @ 2017-08-12 17:51 ` Michael Albinus 1 sibling, 0 replies; 23+ messages in thread From: Michael Albinus @ 2017-08-12 17:51 UTC (permalink / raw) To: Paul Eggert; +Cc: 28023 Paul Eggert <eggert@cs.ucla.edu> writes: Hi Paul, > Thanks. Unfortunately this caused "make check" to fail on Fedora 26. I > installed the attached patch, which causes the test to pass. Can you > please look over it to see whether it makes sense, and look for > similar problems elsewhere? (I'm not that familiar with Tramp.) The patch is OK. Sorry for the trouble, I've searched for "-write-region" in the whole lisp directory for such cases, but I must have missed jka-compr.el. > Thanks. Best regards, Michael. ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#28023: fix make-temp-file race on local host 2017-08-09 5:38 bug#28023: fix make-temp-file race on local host Paul Eggert 2017-08-09 6:51 ` Michael Albinus @ 2017-08-09 16:05 ` Eli Zaretskii 2017-08-09 18:47 ` Paul Eggert 1 sibling, 1 reply; 23+ messages in thread From: Eli Zaretskii @ 2017-08-09 16:05 UTC (permalink / raw) To: Paul Eggert; +Cc: 28023 > From: Paul Eggert <eggert@cs.ucla.edu> > Date: Tue, 8 Aug 2017 22:38:05 -0700 > > I plan to install the attached patch in master soon, and am sending it for > review to bug-gnu-emacs first in case there's some problem with it on > MS-Windows. Thanks for the heads-up. Unfortunately, it isn't as simple as we'd like it to be, because Gnulib doesn't support file names encoded in UTF-8 (or any other encoding except the current system codepage) on MS-Windows, while Emacs does. We could resolve this problem by providing a way for the Windows build to specify its own replacements for 'open', 'mkdir', and 'lstat' (Emacs already have such replacements in w32.c), which tempname.c calls, but that would require minor changes in Gnulib's tempname.c. Or we could steal the relevant code from tempname.c into a Windows-specific implementation in w32.c, and refrain from using the Gnulib version on Windows. Which way would you prefer to go? Or maybe you have yet another idea? > @@ -1915,9 +1913,7 @@ endif > ## begin gnulib module secure_getenv > ifeq (,$(OMIT_GNULIB_MODULE_secure_getenv)) > > -ifneq (,$(gl_GNULIB_ENABLED_secure_getenv)) > > -endif > EXTRA_DIST += secure_getenv.c This seems to say that we will sometimes use secure_getenv, but the relevant code in tempname.c is conditioned on _LIBC, which AFAIU is only true when building glibc, so why do we need that? Or did I miss something? ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#28023: fix make-temp-file race on local host 2017-08-09 16:05 ` Eli Zaretskii @ 2017-08-09 18:47 ` Paul Eggert 2017-08-09 19:09 ` Eli Zaretskii 0 siblings, 1 reply; 23+ messages in thread From: Paul Eggert @ 2017-08-09 18:47 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 28023 [-- Attachment #1: Type: text/plain, Size: 945 bytes --] On 08/09/2017 09:05 AM, Eli Zaretskii wrote: > We could resolve this problem by providing a way for the Windows build > to specify its own replacements for 'open', 'mkdir', and 'lstat' > (Emacs already have such replacements in w32.c), which tempname.c > calls, but that would require minor changes in Gnulib's tempname.c. This sounds like a better way to go. What minor changes would be needed? tempname.c already includes config.h, which includes ms-w32.h, which redefines open, mkdir, and lstat, so I thought it would work as-is. > This seems to say that we will sometimes use secure_getenv, but the > relevant code in tempname.c is conditioned on _LIBC, which AFAIU is > only true when building glibc, so why do we need that? Or did I miss > something? No, good catch, Emacs never needs secure_getenv. I fixed Gnulib, installed the attached patch to propagate that fix, and will adjust the proposed make-temp-file patch accordingly. [-- Attachment #2: 0001-Merge-from-gnulib.patch --] [-- Type: text/x-patch, Size: 14696 bytes --] From 8daab9f4a0ff6dbcfbd0c6f9262e41753de3484b Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Wed, 9 Aug 2017 11:38:04 -0700 Subject: [PATCH] Merge from gnulib This incorporates: 2017-08-09 tempname: do not depend on secure_getenv 2017-08-08 extensions: add _OPENBSD_SOURCE 2017-08-06 manywarnings: Add support for C++ 2017-08-06 warnings, manywarnings: Add support for multiple languages * admin/merge-gnulib: Don't use m4/manywarnings-c++.m4. * lib/gnulib.mk.in, m4/gnulib-comp.m4: Regenerate. * lib/secure_getenv.c, m4/secure_getenv.m4: Remove. * lib/tempname.c, m4/extensions.m4, m4/manywarnings.m4, m4/warnings.m4: Copy from gnulib. --- admin/merge-gnulib | 1 + lib/gnulib.mk.in | 14 -------------- lib/secure_getenv.c | 54 ----------------------------------------------------- lib/tempname.c | 1 - m4/extensions.m4 | 7 ++++++- m4/gnulib-comp.m4 | 19 +------------------ m4/manywarnings.m4 | 18 +++++++++++++++++- m4/secure_getenv.m4 | 26 -------------------------- m4/warnings.m4 | 47 +++++++++++++++++++++++++++++++++++----------- 9 files changed, 61 insertions(+), 126 deletions(-) delete mode 100644 lib/secure_getenv.c delete mode 100644 m4/secure_getenv.m4 diff --git a/admin/merge-gnulib b/admin/merge-gnulib index c23e8a40ea..a16d7fa53e 100755 --- a/admin/merge-gnulib +++ b/admin/merge-gnulib @@ -106,6 +106,7 @@ avoided_flags= rm -- "$src"lib/gl_openssl.h "$src"m4/fcntl-o.m4 \ "$src"m4/gl-openssl.m4 \ "$src"m4/gnulib-cache.m4 "$src"m4/gnulib-tool.m4 \ + "$src"m4/manywarnings-c++.m4 \ "$src"m4/warn-on-use.m4 "$src"m4/wint_t.m4 && cp -- "$gnulib_srcdir"/build-aux/texinfo.tex "$src"doc/misc && cp -- "$gnulib_srcdir"/build-aux/config.guess \ diff --git a/lib/gnulib.mk.in b/lib/gnulib.mk.in index a385c8c838..c5df3f42e4 100644 --- a/lib/gnulib.mk.in +++ b/lib/gnulib.mk.in @@ -903,7 +903,6 @@ gl_GNULIB_ENABLED_dosname = @gl_GNULIB_ENABLED_dosname@ 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_secure_getenv = @gl_GNULIB_ENABLED_secure_getenv@ gl_GNULIB_ENABLED_strtoll = @gl_GNULIB_ENABLED_strtoll@ gl_GNULIB_ENABLED_tempname = @gl_GNULIB_ENABLED_tempname@ gl_LIBOBJS = @gl_LIBOBJS@ @@ -1912,19 +1911,6 @@ EXTRA_DIST += root-uid.h endif ## end gnulib module root-uid -## begin gnulib module secure_getenv -ifeq (,$(OMIT_GNULIB_MODULE_secure_getenv)) - -ifneq (,$(gl_GNULIB_ENABLED_secure_getenv)) - -endif -EXTRA_DIST += secure_getenv.c - -EXTRA_libgnu_a_SOURCES += secure_getenv.c - -endif -## end gnulib module secure_getenv - ## begin gnulib module sig2str ifeq (,$(OMIT_GNULIB_MODULE_sig2str)) diff --git a/lib/secure_getenv.c b/lib/secure_getenv.c deleted file mode 100644 index df53dea0b2..0000000000 --- a/lib/secure_getenv.c +++ /dev/null @@ -1,54 +0,0 @@ -/* Look up an environment variable, returning NULL in insecure situations. - - Copyright 2013-2017 Free Software Foundation, Inc. - - This program is free software: you can redistribute it and/or modify it - under the terms of the GNU General Public License as published - by the Free Software Foundation; either version 3 of the License, or - (at your option) any later version. - - This program is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - General Public License for more details. - - You should have received a copy of the GNU General Public License - along with this program. If not, see <http://www.gnu.org/licenses/>. */ - -#include <config.h> - -#include <stdlib.h> - -#if !HAVE___SECURE_GETENV -# if HAVE_ISSETUGID || (HAVE_GETUID && HAVE_GETEUID && HAVE_GETGID && HAVE_GETEGID) -# include <unistd.h> -# endif -#endif - -char * -secure_getenv (char const *name) -{ -#if HAVE___SECURE_GETENV /* glibc */ - return __secure_getenv (name); -#elif HAVE_ISSETUGID /* OS X, FreeBSD, NetBSD, OpenBSD */ - if (issetugid ()) - return NULL; - return getenv (name); -#elif HAVE_GETUID && HAVE_GETEUID && HAVE_GETGID && HAVE_GETEGID /* other Unix */ - if (geteuid () != getuid () || getegid () != getgid ()) - return NULL; - return getenv (name); -#elif (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__ /* native Windows */ - /* On native Windows, there is no such concept as setuid or setgid binaries. - - Programs launched as system services have high privileges, but they don't - inherit environment variables from a user. - - Programs launched by a user with "Run as Administrator" have high - privileges and use the environment variables, but the user has been asked - whether he agrees. - - Programs launched by a user without "Run as Administrator" cannot gain - high privileges, therefore there is no risk. */ - return getenv (name); -#else - return NULL; -#endif -} diff --git a/lib/tempname.c b/lib/tempname.c index 9c4a3c2a54..c274b8dd4e 100644 --- a/lib/tempname.c +++ b/lib/tempname.c @@ -69,7 +69,6 @@ # define __mkdir mkdir # define __open open # define __lxstat64(version, file, buf) lstat (file, buf) -# define __secure_getenv secure_getenv #endif #ifdef _LIBC diff --git a/m4/extensions.m4 b/m4/extensions.m4 index c60f537db1..0c16bb832f 100644 --- a/m4/extensions.m4 +++ b/m4/extensions.m4 @@ -1,4 +1,4 @@ -# serial 15 -*- Autoconf -*- +# serial 16 -*- Autoconf -*- # Enable extensions on systems that normally disable them. # Copyright (C) 2003, 2006-2017 Free Software Foundation, Inc. @@ -68,6 +68,10 @@ AC_DEFUN_ONCE #ifndef _GNU_SOURCE # undef _GNU_SOURCE #endif +/* Enable OpenBSD extensions on NetBSD. */ +#ifndef _OPENBSD_SOURCE +# undef _OPENBSD_SOURCE +#endif /* Enable threading extensions on Solaris. */ #ifndef _POSIX_PTHREAD_SEMANTICS # undef _POSIX_PTHREAD_SEMANTICS @@ -128,6 +132,7 @@ AC_DEFUN_ONCE AC_DEFINE([_ALL_SOURCE]) AC_DEFINE([_DARWIN_C_SOURCE]) AC_DEFINE([_GNU_SOURCE]) + AC_DEFINE([_OPENBSD_SOURCE]) AC_DEFINE([_POSIX_PTHREAD_SEMANTICS]) AC_DEFINE([__STDC_WANT_IEC_60559_ATTRIBS_EXT__]) AC_DEFINE([__STDC_WANT_IEC_60559_BFP_EXT__]) diff --git a/m4/gnulib-comp.m4 b/m4/gnulib-comp.m4 index 188c116c85..69d77229bf 100644 --- a/m4/gnulib-comp.m4 +++ b/m4/gnulib-comp.m4 @@ -124,7 +124,6 @@ AC_DEFUN # Code from module readlink: # Code from module readlinkat: # Code from module root-uid: - # Code from module secure_getenv: # Code from module sig2str: # Code from module signal-h: # Code from module snippet/_Noreturn: @@ -424,7 +423,6 @@ AC_DEFUN gl_gnulib_enabled_5264294aa0a5557541b53c8c741f7f31=false gl_gnulib_enabled_03e0aaad4cb89ca757653bd367a6ccb7=false gl_gnulib_enabled_6099e9737f757db36c47fa9d9f02e88c=false - gl_gnulib_enabled_secure_getenv=false gl_gnulib_enabled_strtoll=false gl_gnulib_enabled_tempname=false gl_gnulib_enabled_682e609604ccaac6be382e4ee3a4eaec=false @@ -550,18 +548,6 @@ AC_DEFUN gl_gnulib_enabled_6099e9737f757db36c47fa9d9f02e88c=true fi } - func_gl_gnulib_m4code_secure_getenv () - { - if ! $gl_gnulib_enabled_secure_getenv; then - gl_FUNC_SECURE_GETENV - if test $HAVE_SECURE_GETENV = 0; then - AC_LIBOBJ([secure_getenv]) - gl_PREREQ_SECURE_GETENV - fi - gl_STDLIB_MODULE_INDICATOR([secure_getenv]) - gl_gnulib_enabled_secure_getenv=true - fi - } func_gl_gnulib_m4code_strtoll () { if ! $gl_gnulib_enabled_strtoll; then @@ -579,7 +565,6 @@ AC_DEFUN if ! $gl_gnulib_enabled_tempname; then gl_FUNC_GEN_TEMPNAME gl_gnulib_enabled_tempname=true - func_gl_gnulib_m4code_secure_getenv fi } func_gl_gnulib_m4code_682e609604ccaac6be382e4ee3a4eaec () @@ -658,7 +643,6 @@ AC_DEFUN AM_CONDITIONAL([gl_GNULIB_ENABLED_5264294aa0a5557541b53c8c741f7f31], [$gl_gnulib_enabled_5264294aa0a5557541b53c8c741f7f31]) 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_secure_getenv], [$gl_gnulib_enabled_secure_getenv]) 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]) @@ -907,7 +891,6 @@ AC_DEFUN lib/readlink.c lib/readlinkat.c lib/root-uid.h - lib/secure_getenv.c lib/set-permissions.c lib/sha1.c lib/sha1.h @@ -1008,6 +991,7 @@ AC_DEFUN m4/localtime-buffer.m4 m4/longlong.m4 m4/lstat.m4 + m4/manywarnings-c++.m4 m4/manywarnings.m4 m4/md5.m4 m4/memrchr.m4 @@ -1024,7 +1008,6 @@ AC_DEFUN m4/putenv.m4 m4/readlink.m4 m4/readlinkat.m4 - m4/secure_getenv.m4 m4/sha1.m4 m4/sha256.m4 m4/sha512.m4 diff --git a/m4/manywarnings.m4 b/m4/manywarnings.m4 index 6a8939b2c1..a3d255a940 100644 --- a/m4/manywarnings.m4 +++ b/m4/manywarnings.m4 @@ -1,4 +1,4 @@ -# manywarnings.m4 serial 10 +# manywarnings.m4 serial 11 dnl Copyright (C) 2008-2017 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, @@ -33,8 +33,16 @@ AC_DEFUN # Add all documented GCC warning parameters to variable VARIABLE. # Note that you need to test them using gl_WARN_ADD if you want to # make sure your gcc understands it. +# +# The effects of this macro depend on the current language (_AC_LANG). AC_DEFUN([gl_MANYWARN_ALL_GCC], +[_AC_LANG_DISPATCH([$0], _AC_LANG, $@)]) + +# Specialization for _AC_LANG = C. +AC_DEFUN([gl_MANYWARN_ALL_GCC(C)], [ + AC_LANG_PUSH([C]) + dnl First, check for some issues that only occur when combining multiple dnl gcc warning categories. AC_REQUIRE([AC_PROG_CC]) @@ -303,4 +311,12 @@ AC_DEFUN fi $1=$gl_manywarn_set + + AC_LANG_POP([C]) +]) + +# Specialization for _AC_LANG = C++. +AC_DEFUN([gl_MANYWARN_ALL_GCC(C++)], +[ + gl_MANYWARN_ALL_GCC_CXX_IMPL([$1]) ]) diff --git a/m4/secure_getenv.m4 b/m4/secure_getenv.m4 deleted file mode 100644 index 6bd4afd9c1..0000000000 --- a/m4/secure_getenv.m4 +++ /dev/null @@ -1,26 +0,0 @@ -# Look up an environment variable more securely. -dnl Copyright 2013-2017 Free Software Foundation, Inc. -dnl This file is free software; the Free Software Foundation -dnl gives unlimited permission to copy and/or distribute it, -dnl with or without modifications, as long as this notice is preserved. - -AC_DEFUN([gl_FUNC_SECURE_GETENV], -[ - dnl Persuade glibc <stdlib.h> to declare secure_getenv(). - AC_REQUIRE([AC_USE_SYSTEM_EXTENSIONS]) - - AC_REQUIRE([gl_STDLIB_H_DEFAULTS]) - AC_CHECK_FUNCS_ONCE([secure_getenv]) - if test $ac_cv_func_secure_getenv = no; then - HAVE_SECURE_GETENV=0 - fi -]) - -# Prerequisites of lib/secure_getenv.c. -AC_DEFUN([gl_PREREQ_SECURE_GETENV], [ - AC_CHECK_FUNCS([__secure_getenv]) - if test $ac_cv_func___secure_getenv = no; then - AC_CHECK_FUNCS([issetugid]) - fi - AC_CHECK_FUNCS_ONCE([getuid geteuid getgid getegid]) -]) diff --git a/m4/warnings.m4 b/m4/warnings.m4 index e697174edd..aa2735b77f 100644 --- a/m4/warnings.m4 +++ b/m4/warnings.m4 @@ -1,4 +1,4 @@ -# warnings.m4 serial 11 +# warnings.m4 serial 12 dnl Copyright (C) 2008-2017 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, @@ -20,10 +20,12 @@ # ----------------------------------------------------------------- # Check if the compiler supports OPTION when compiling PROGRAM. # -# FIXME: gl_Warn must be used unquoted until we can assume Autoconf -# 2.64 or newer. +# The effects of this macro depend on the current language (_AC_LANG). AC_DEFUN([gl_COMPILER_OPTION_IF], -[AS_VAR_PUSHDEF([gl_Warn], [gl_cv_warn_[]_AC_LANG_ABBREV[]_$1])dnl +[ +dnl FIXME: gl_Warn must be used unquoted until we can assume Autoconf +dnl 2.64 or newer. +AS_VAR_PUSHDEF([gl_Warn], [gl_cv_warn_[]_AC_LANG_ABBREV[]_$1])dnl AS_VAR_PUSHDEF([gl_Flags], [_AC_LANG_PREFIX[]FLAGS])dnl AS_LITERAL_IF([$1], [m4_pushdef([gl_Positive], m4_bpatsubst([$1], [^-Wno-], [-W]))], @@ -51,27 +53,50 @@ AC_DEFUN # ------------------------------ # Clang doesn't complain about unknown warning options unless one also # specifies -Wunknown-warning-option -Werror. Detect this. +# +# The effects of this macro depend on the current language (_AC_LANG). AC_DEFUN([gl_UNKNOWN_WARNINGS_ARE_ERRORS], +[_AC_LANG_DISPATCH([$0], _AC_LANG, $@)]) + +# Specialization for _AC_LANG = C. This macro can be AC_REQUIREd. +AC_DEFUN([gl_UNKNOWN_WARNINGS_ARE_ERRORS(C)], +[ + AC_LANG_PUSH([C]) + gl_UNKNOWN_WARNINGS_ARE_ERRORS_IMPL + AC_LANG_POP([C]) +]) + +# Specialization for _AC_LANG = C++. This macro can be AC_REQUIREd. +AC_DEFUN([gl_UNKNOWN_WARNINGS_ARE_ERRORS(C++)], +[ + AC_LANG_PUSH([C++]) + gl_UNKNOWN_WARNINGS_ARE_ERRORS_IMPL + AC_LANG_POP([C++]) +]) + +AC_DEFUN([gl_UNKNOWN_WARNINGS_ARE_ERRORS_IMPL], [gl_COMPILER_OPTION_IF([-Werror -Wunknown-warning-option], [gl_unknown_warnings_are_errors='-Wunknown-warning-option -Werror'], [gl_unknown_warnings_are_errors=])]) -# gl_WARN_ADD(OPTION, [VARIABLE = WARN_CFLAGS], +# gl_WARN_ADD(OPTION, [VARIABLE = WARN_CFLAGS/WARN_CXXFLAGS], # [PROGRAM = AC_LANG_PROGRAM()]) -# --------------------------------------------- -# Adds parameter to WARN_CFLAGS if the compiler supports it when -# compiling PROGRAM. For example, gl_WARN_ADD([-Wparentheses]). +# ----------------------------------------------------------- +# Adds parameter to WARN_CFLAGS/WARN_CXXFLAGS if the compiler supports it +# when compiling PROGRAM. For example, gl_WARN_ADD([-Wparentheses]). # # If VARIABLE is a variable name, AC_SUBST it. +# +# The effects of this macro depend on the current language (_AC_LANG). AC_DEFUN([gl_WARN_ADD], -[AC_REQUIRE([gl_UNKNOWN_WARNINGS_ARE_ERRORS]) +[AC_REQUIRE([gl_UNKNOWN_WARNINGS_ARE_ERRORS(]_AC_LANG[)]) gl_COMPILER_OPTION_IF([$1], - [gl_AS_VAR_APPEND(m4_if([$2], [], [[WARN_CFLAGS]], [[$2]]), [" $1"])], + [gl_AS_VAR_APPEND(m4_if([$2], [], [[WARN_]_AC_LANG_PREFIX[FLAGS]], [[$2]]), [" $1"])], [], [$3]) m4_ifval([$2], [AS_LITERAL_IF([$2], [AC_SUBST([$2])])], - [AC_SUBST([WARN_CFLAGS])])dnl + [AC_SUBST([WARN_]_AC_LANG_PREFIX[FLAGS])])dnl ]) # Local Variables: -- 2.13.4 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* bug#28023: fix make-temp-file race on local host 2017-08-09 18:47 ` Paul Eggert @ 2017-08-09 19:09 ` Eli Zaretskii 2017-08-09 23:36 ` Paul Eggert 0 siblings, 1 reply; 23+ messages in thread From: Eli Zaretskii @ 2017-08-09 19:09 UTC (permalink / raw) To: Paul Eggert; +Cc: 28023 > Cc: 28023@debbugs.gnu.org > From: Paul Eggert <eggert@cs.ucla.edu> > Date: Wed, 9 Aug 2017 11:47:36 -0700 > > On 08/09/2017 09:05 AM, Eli Zaretskii wrote: > > We could resolve this problem by providing a way for the Windows build > > to specify its own replacements for 'open', 'mkdir', and 'lstat' > > (Emacs already have such replacements in w32.c), which tempname.c > > calls, but that would require minor changes in Gnulib's tempname.c. > > This sounds like a better way to go. What minor changes would be needed? > tempname.c already includes config.h, which includes ms-w32.h, which > redefines open, mkdir, and lstat, so I thought it would work as-is. Perhaps you are right, and it will "just work". I will have to try (for now, I just looked at the sources and the changes, but didn't try building). > > This seems to say that we will sometimes use secure_getenv, but the > > relevant code in tempname.c is conditioned on _LIBC, which AFAIU is > > only true when building glibc, so why do we need that? Or did I miss > > something? > No, good catch, Emacs never needs secure_getenv. I fixed Gnulib, > installed the attached patch to propagate that fix, and will adjust the > proposed make-temp-file patch accordingly. Thanks. ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#28023: fix make-temp-file race on local host 2017-08-09 19:09 ` Eli Zaretskii @ 2017-08-09 23:36 ` Paul Eggert 2017-08-10 15:35 ` Eli Zaretskii 0 siblings, 1 reply; 23+ messages in thread From: Paul Eggert @ 2017-08-09 23:36 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 28023 [-- Attachment #1: Type: text/plain, Size: 502 bytes --] Eli Zaretskii wrote: > Perhaps you are right, and it will "just work". I will have to try > (for now, I just looked at the sources and the changes, but didn't try > building). It looks to me like it should just work. You might also consider the second attached patch: it removes the MS-Windows implementation of mkostemp, since the generic code should just work as well. I've tested only the first attached patch, though: it is basically the same as before except it's rebased to current master. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Fix-make-temp-file-race-on-local-files.patch --] [-- Type: text/x-patch; name="0001-Fix-make-temp-file-race-on-local-files.patch", Size: 20531 bytes --] From 7fe4062aebf56a7de1ae389907098cd461f8d485 Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> 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 <http://www.gnu.org/licenses/>. */ #include <allocator.h> #include <careadlinkat.h> #include <stat-time.h> +#include <tempname.h> #include <binary-io.h> @@ -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 [-- Attachment #3: 0002-Remove-ms-w32-mkostemp.patch --] [-- Type: text/x-patch, Size: 5692 bytes --] From 130f81bd123de154be5247d3237610d1eeab5dca Mon Sep 17 00:00:00 2001 From: Paul Eggert <eggert@cs.ucla.edu> Date: Wed, 9 Aug 2017 16:34:40 -0700 Subject: [PATCH 2/2] Remove ms-w32 mkostemp * lib-src/ntlib.c, nt/inc/ms-w32.h, src/w32.c (mkostemp): * nt/gnulib-cfg.mk (OMIT_GNULIB_MODULE_mkostemp): * nt/mingw-cfg.site (ac_cv_func_mkostemp): Remove, since the generic mkostemp should work on MS-Windows. --- lib-src/ntlib.c | 56 ------------------------------------------------------- nt/gnulib-cfg.mk | 1 - nt/inc/ms-w32.h | 2 -- nt/mingw-cfg.site | 1 - src/w32.c | 55 ------------------------------------------------------ 5 files changed, 115 deletions(-) diff --git a/lib-src/ntlib.c b/lib-src/ntlib.c index 78ba9061f6..078a43e79c 100644 --- a/lib-src/ntlib.c +++ b/lib-src/ntlib.c @@ -37,7 +37,6 @@ along with GNU Emacs. If not, see <http://www.gnu.org/licenses/>. */ char *sys_ctime (const time_t *); FILE *sys_fopen (const char *, const char *); int sys_chdir (const char *); -int mkostemp (char *, int); int sys_rename (const char *, const char *); /* MinGW64 defines _TIMEZONE_DEFINED and defines 'struct timespec' in @@ -396,61 +395,6 @@ lstat (const char * path, struct stat * buf) return stat (path, buf); } -/* Implementation of mkostemp for MS-Windows, to avoid race conditions - when using mktemp. Copied from w32.c. - - This is used only in update-game-score.c. It is overkill for that - use case, since update-game-score renames the temporary file into - the game score file, which isn't atomic on MS-Windows anyway, when - the game score already existed before running the program, which it - almost always does. But using a simpler implementation just to - make a point is uneconomical... */ - -int -mkostemp (char * template, int flags) -{ - char * p; - int i, fd = -1; - unsigned uid = GetCurrentThreadId (); - int save_errno = errno; - static char first_char[] = "abcdefghijklmnopqrstuvwyz0123456789!%-_@#"; - - errno = EINVAL; - if (template == NULL) - return -1; - - p = template + strlen (template); - i = 5; - /* replace up to the last 5 X's with uid in decimal */ - while (--p >= template && p[0] == 'X' && --i >= 0) - { - p[0] = '0' + uid % 10; - uid /= 10; - } - - if (i < 0 && p[0] == 'X') - { - i = 0; - do - { - p[0] = first_char[i]; - if ((fd = open (template, - flags | _O_CREAT | _O_EXCL | _O_RDWR, - S_IRUSR | S_IWUSR)) >= 0 - || errno != EEXIST) - { - if (fd >= 0) - errno = save_errno; - return fd; - } - } - while (++i < sizeof (first_char)); - } - - /* Template is badly formed or else we can't generate a unique name. */ - return -1; -} - /* On Windows, you cannot rename into an existing file. */ int sys_rename (const char *from, const char *to) diff --git a/nt/gnulib-cfg.mk b/nt/gnulib-cfg.mk index 175329fb9e..ad798bbe37 100644 --- a/nt/gnulib-cfg.mk +++ b/nt/gnulib-cfg.mk @@ -50,7 +50,6 @@ OMIT_GNULIB_MODULE_dirfd = OMIT_GNULIB_MODULE_fcntl = true OMIT_GNULIB_MODULE_fcntl-h = true OMIT_GNULIB_MODULE_inttypes-incomplete = true -OMIT_GNULIB_MODULE_mkostemp = true OMIT_GNULIB_MODULE_pipe2 = true OMIT_GNULIB_MODULE_secure_getenv = true OMIT_GNULIB_MODULE_signal-h = true diff --git a/nt/inc/ms-w32.h b/nt/inc/ms-w32.h index 957d8c6bdb..6cbbb9f869 100644 --- a/nt/inc/ms-w32.h +++ b/nt/inc/ms-w32.h @@ -518,8 +518,6 @@ extern int getpagesize (void); extern void * memrchr (void const *, int, size_t); -extern int mkostemp (char *, int); - #if defined (__MINGW32__) diff --git a/nt/mingw-cfg.site b/nt/mingw-cfg.site index a106717979..d9a824008c 100644 --- a/nt/mingw-cfg.site +++ b/nt/mingw-cfg.site @@ -79,7 +79,6 @@ ac_cv_func_getaddrinfo=yes # Implemented as an inline function in ws2tcpip.h ac_cv_func_gai_strerror=yes # Implemented in w32.c -ac_cv_func_mkostemp=yes ac_cv_func_readlink=yes ac_cv_func_symlink=yes # Avoid run-time tests of readlink and symlink, which will fail diff --git a/src/w32.c b/src/w32.c index fa3cbe183f..1c58f92d57 100644 --- a/src/w32.c +++ b/src/w32.c @@ -4397,61 +4397,6 @@ sys_open (const char * path, int oflag, int mode) return res; } -/* Implementation of mkostemp for MS-Windows, to avoid race conditions - when using mktemp. - - Standard algorithm for generating a temporary file name seems to be - use pid or tid with a letter on the front (in place of the 6 X's) - and cycle through the letters to find a unique name. We extend - that to allow any reasonable character as the first of the 6 X's, - so that the number of simultaneously used temporary files will be - greater. */ - -int -mkostemp (char * template, int flags) -{ - char * p; - int i, fd = -1; - unsigned uid = GetCurrentThreadId (); - int save_errno = errno; - static char first_char[] = "abcdefghijklmnopqrstuvwyz0123456789!%-_@#"; - - errno = EINVAL; - if (template == NULL) - return -1; - - p = template + strlen (template); - i = 5; - /* replace up to the last 5 X's with uid in decimal */ - while (--p >= template && p[0] == 'X' && --i >= 0) - { - p[0] = '0' + uid % 10; - uid /= 10; - } - - if (i < 0 && p[0] == 'X') - { - i = 0; - do - { - p[0] = first_char[i]; - if ((fd = sys_open (template, - flags | _O_CREAT | _O_EXCL | _O_RDWR, - S_IRUSR | S_IWUSR)) >= 0 - || errno != EEXIST) - { - if (fd >= 0) - errno = save_errno; - return fd; - } - } - while (++i < sizeof (first_char)); - } - - /* Template is badly formed or else we can't generate a unique name. */ - return -1; -} - int fchmod (int fd, mode_t mode) { -- 2.13.4 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* bug#28023: fix make-temp-file race on local host 2017-08-09 23:36 ` Paul Eggert @ 2017-08-10 15:35 ` Eli Zaretskii 2017-08-10 22:24 ` Paul Eggert 0 siblings, 1 reply; 23+ messages in thread From: Eli Zaretskii @ 2017-08-10 15:35 UTC (permalink / raw) To: Paul Eggert; +Cc: 28023 > Cc: 28023@debbugs.gnu.org > From: Paul Eggert <eggert@cs.ucla.edu> > Date: Wed, 9 Aug 2017 16:36:55 -0700 > > Eli Zaretskii wrote: > > Perhaps you are right, and it will "just work". I will have to try > > (for now, I just looked at the sources and the changes, but didn't try > > building). > > It looks to me like it should just work. Well, it's not that easy after all. First, nt/gnulib-cfg.mk needs to be fixed to remove the lines that disable building tempname.c and mkostmp.c, and nt/mingw-cfg.site needs to remove the line which claims that mkostmp is available. Otherwise, tempname.c will not be compiled. This is easy to fix. Then it turns out ms-w32.h only makes its redirections when compiling with -Demacs, which is not what happens when Gnulib sources are compiled. I can fix this, but it will need a few changes in lib-src, which currently doesn't expect 'open' to be redirected. And finally, tempname.c calls mkdir with 2 arguments, whereas mkdir on Windows accepts only one. (This means Gnulib lacks a dependency, since the tempname module should then depend on mkdir, which isn't in lib/.) Since we don't want the Gnulib replacement for mkdir, we will have to change w32.c:sys_mkdir to accept 2 arguments to fix this, unless you have a better idea. How do you propose to proceed with this? > You might also consider the second > attached patch: it removes the MS-Windows implementation of mkostemp, since the > generic code should just work as well. Once the above problems are taken care of, the mkostemp part should be easy. Thanks. ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#28023: fix make-temp-file race on local host 2017-08-10 15:35 ` Eli Zaretskii @ 2017-08-10 22:24 ` Paul Eggert 2017-08-11 6:21 ` Eli Zaretskii 0 siblings, 1 reply; 23+ messages in thread From: Paul Eggert @ 2017-08-10 22:24 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 28023 Eli Zaretskii wrote: > And finally, tempname.c calls mkdir with 2 arguments, whereas mkdir on > Windows accepts only one. (This means Gnulib lacks a dependency, > since the tempname module should then depend on mkdir, which isn't in > lib/.) Since we don't want the Gnulib replacement for mkdir, we will > have to change w32.c:sys_mkdir to accept 2 arguments to fix this, > unless you have a better idea. No, your ideas all sound good. It's simpler for the generic code, if the mkdir replacement acts like POSIX mkdir. ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#28023: fix make-temp-file race on local host 2017-08-10 22:24 ` Paul Eggert @ 2017-08-11 6:21 ` Eli Zaretskii 2017-08-11 7:44 ` Paul Eggert 0 siblings, 1 reply; 23+ messages in thread From: Eli Zaretskii @ 2017-08-11 6:21 UTC (permalink / raw) To: Paul Eggert; +Cc: 28023 > Cc: 28023@debbugs.gnu.org > From: Paul Eggert <eggert@cs.ucla.edu> > Date: Thu, 10 Aug 2017 15:24:13 -0700 > > No, your ideas all sound good. It's simpler for the generic code, if the mkdir > replacement acts like POSIX mkdir. So how to proceed? Do you want to wait until I make all those changes on master? Or push a branch with the changes? Something else? ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#28023: fix make-temp-file race on local host 2017-08-11 6:21 ` Eli Zaretskii @ 2017-08-11 7:44 ` Paul Eggert 2017-08-11 8:03 ` Eli Zaretskii 0 siblings, 1 reply; 23+ messages in thread From: Paul Eggert @ 2017-08-11 7:44 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 28023 Eli Zaretskii wrote: > So how to proceed? Do you want to wait until I make all those changes > on master? Or push a branch with the changes? Something else? I'd rather not mess with a branch. If it's easy to make those changes to master please do so. If not, I can modify the patch to keep the current algorithm for DOS_NT, though this will be ugly. ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#28023: fix make-temp-file race on local host 2017-08-11 7:44 ` Paul Eggert @ 2017-08-11 8:03 ` Eli Zaretskii 2017-08-12 8:31 ` Eli Zaretskii 0 siblings, 1 reply; 23+ messages in thread From: Eli Zaretskii @ 2017-08-11 8:03 UTC (permalink / raw) To: Paul Eggert; +Cc: 28023 > Cc: 28023@debbugs.gnu.org > From: Paul Eggert <eggert@cs.ucla.edu> > Date: Fri, 11 Aug 2017 00:44:36 -0700 > > If it's easy to make those changes to master please do so. OK, will do. ^ permalink raw reply [flat|nested] 23+ messages in thread
* bug#28023: fix make-temp-file race on local host 2017-08-11 8:03 ` Eli Zaretskii @ 2017-08-12 8:31 ` Eli Zaretskii 0 siblings, 0 replies; 23+ messages in thread From: Eli Zaretskii @ 2017-08-12 8:31 UTC (permalink / raw) To: eggert; +Cc: 28023 > Date: Fri, 11 Aug 2017 11:03:04 +0300 > From: Eli Zaretskii <eliz@gnu.org> > Cc: 28023@debbugs.gnu.org > > > Cc: 28023@debbugs.gnu.org > > From: Paul Eggert <eggert@cs.ucla.edu> > > Date: Fri, 11 Aug 2017 00:44:36 -0700 > > > > If it's easy to make those changes to master please do so. > > OK, will do. Done. ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2017-08-13 7:21 UTC | newest] Thread overview: 23+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-08-09 5:38 bug#28023: fix make-temp-file race on local host Paul Eggert 2017-08-09 6:51 ` Michael Albinus 2017-08-09 9:22 ` Paul Eggert 2017-08-09 11:21 ` Michael Albinus 2017-08-13 7:21 ` Paul Eggert 2017-08-12 10:35 ` Michael Albinus 2017-08-12 15:55 ` Paul Eggert 2017-08-12 16:09 ` Eli Zaretskii 2017-08-12 16:25 ` Paul Eggert 2017-08-12 16:52 ` Eli Zaretskii 2017-08-12 17:57 ` Paul Eggert 2017-08-12 18:07 ` Eli Zaretskii 2017-08-12 17:51 ` Michael Albinus 2017-08-09 16:05 ` Eli Zaretskii 2017-08-09 18:47 ` Paul Eggert 2017-08-09 19:09 ` Eli Zaretskii 2017-08-09 23:36 ` Paul Eggert 2017-08-10 15:35 ` Eli Zaretskii 2017-08-10 22:24 ` Paul Eggert 2017-08-11 6:21 ` Eli Zaretskii 2017-08-11 7:44 ` Paul Eggert 2017-08-11 8:03 ` Eli Zaretskii 2017-08-12 8:31 ` Eli Zaretskii
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).