From 6772211202c57f0dc96e5725f882a2fa4ee98d2d Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Thu, 17 Jan 2013 21:12:08 -0800 Subject: [PATCH] Close a race when statting and reading files. * fileio.c (Finsert_file_contents): Use open+fstat, not stat+open. This avoids a race if the file is renamed between stat and open. This race is not the problem originally noted in Bug#13149; see and later messages in the thread. --- src/ChangeLog | 8 ++++++++ src/fileio.c | 44 +++++++++++++------------------------------- 2 files changed, 21 insertions(+), 31 deletions(-) diff --git a/src/ChangeLog b/src/ChangeLog index c85e0a789ea..f53b9a70e97 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,11 @@ +2013-01-17 Paul Eggert + + Close a race when statting and reading files (Bug#13149). + * fileio.c (Finsert_file_contents): Use open+fstat, not stat+open. + This avoids a race if the file is renamed between stat and open. + This race is not the problem originally noted in Bug#13149; + see and later messages in the thread. + 2013-01-17 Dmitry Antipov * lisp.h (toplevel): Add comment about using Lisp_Save_Value diff --git a/src/fileio.c b/src/fileio.c index 8d711e8e6bf..41b8ae388d1 100644 --- a/src/fileio.c +++ b/src/fileio.c @@ -3492,7 +3492,6 @@ by calling `format-decode', which see. */) (Lisp_Object filename, Lisp_Object visit, Lisp_Object beg, Lisp_Object end, Lisp_Object replace) { struct stat st; - int file_status; EMACS_TIME mtime; int fd; ptrdiff_t inserted = 0; @@ -3554,26 +3553,9 @@ by calling `format-decode', which see. */) orig_filename = filename; filename = ENCODE_FILE (filename); - fd = -1; - -#ifdef WINDOWSNT - { - Lisp_Object tem = Vw32_get_true_file_attributes; - - /* Tell stat to use expensive method to get accurate info. */ - Vw32_get_true_file_attributes = Qt; - file_status = stat (SSDATA (filename), &st); - Vw32_get_true_file_attributes = tem; - } -#else - file_status = stat (SSDATA (filename), &st); -#endif /* WINDOWSNT */ - - if (file_status == 0) - mtime = get_stat_mtime (&st); - else + fd = emacs_open (SSDATA (filename), O_RDONLY, 0); + if (fd < 0) { - badopen: save_errno = errno; if (NILP (visit)) report_file_error ("Opening input file", Fcons (orig_filename, Qnil)); @@ -3585,6 +3567,17 @@ by calling `format-decode', which see. */) goto notfound; } + /* Replacement should preserve point as it preserves markers. */ + if (!NILP (replace)) + record_unwind_protect (restore_point_unwind, Fpoint_marker ()); + + record_unwind_protect (close_file_unwind, make_number (fd)); + + if (fstat (fd, &st) != 0) + report_file_error ("Getting input file status", + Fcons (orig_filename, Qnil)); + mtime = get_stat_mtime (&st); + /* This code will need to be changed in order to work on named pipes, and it's probably just not worth it. So we should at least signal an error. */ @@ -3600,17 +3593,6 @@ by calling `format-decode', which see. */) build_string ("not a regular file"), orig_filename); } - if (fd < 0) - if ((fd = emacs_open (SSDATA (filename), O_RDONLY, 0)) < 0) - goto badopen; - - /* Replacement should preserve point as it preserves markers. */ - if (!NILP (replace)) - record_unwind_protect (restore_point_unwind, Fpoint_marker ()); - - record_unwind_protect (close_file_unwind, make_number (fd)); - - if (!NILP (visit)) { if (!NILP (beg) || !NILP (end)) -- 2.39.5