unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#10403: epg--make-temp-file permissions race condition fixes
@ 2011-12-29 22:22 Paul Eggert
  2012-01-07  7:12 ` Chong Yidong
  0 siblings, 1 reply; 6+ messages in thread
From: Paul Eggert @ 2011-12-29 22:22 UTC (permalink / raw)
  To: 10403

Tags: patch

=== modified file 'lisp/ChangeLog'
--- lisp/ChangeLog	2011-12-29 21:55:33 +0000
+++ lisp/ChangeLog	2011-12-29 22:08:29 +0000
@@ -1,5 +1,8 @@
 2011-12-29  Paul Eggert  <eggert@cs.ucla.edu>
 
+	* epg.el (epg--make-temp-file): Avoid permission race conditions
+	when creating temporary directories and files on older Emacs.
+
 	* files.el (move-file-to-trash): Preserve default file modes on error.
 	(Bug#10401)
 

=== modified file 'lisp/epg.el'
--- lisp/epg.el	2011-11-23 07:03:56 +0000
+++ lisp/epg.el	2011-12-29 22:08:29 +0000
@@ -1951,14 +1951,16 @@
 of PREFIX, and expanding against `temporary-file-directory' if necessary),
 is guaranteed to point to a newly created empty file.
 You can then use `write-region' to write new data into the file."
-      (let (tempdir tempfile)
+      (let (tempdir tempfile orig-modes)
 	(setq prefix (expand-file-name prefix
 				       (if (featurep 'xemacs)
 					   (temp-directory)
 					 temporary-file-directory)))
+	(setq orig-modes (default-file-modes))
 	(unwind-protect
 	    (let (file)
 	      ;; First, create a temporary directory.
+	      (set-default-file-modes #o700)
 	      (while (condition-case ()
 			 (progn
 			   (setq tempdir (make-temp-name
@@ -1969,14 +1971,12 @@
 			   (make-directory tempdir))
 		       ;; let's try again.
 		       (file-already-exists t)))
-	      (set-file-modes tempdir 448)
 	      ;; Second, create a temporary file in the tempdir.
 	      ;; There *is* a race condition between `make-temp-name'
 	      ;; and `write-region', but we don't care it since we are
 	      ;; in a private directory now.
 	      (setq tempfile (make-temp-name (concat tempdir "/EMU")))
 	      (write-region "" nil tempfile nil 'silent)
-	      (set-file-modes tempfile 384)
 	      ;; Finally, make a hard-link from the tempfile.
 	      (while (condition-case ()
 			 (progn
@@ -1986,6 +1986,7 @@
 		       ;; let's try again.
 		       (file-already-exists t)))
 	      file)
+	  (set-default-file-modes orig-modes)
 	  ;; Cleanup the tempfile.
 	  (and tempfile
 	       (file-exists-p tempfile)






^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#10403: epg--make-temp-file permissions race condition fixes
  2011-12-29 22:22 bug#10403: epg--make-temp-file permissions race condition fixes Paul Eggert
@ 2012-01-07  7:12 ` Chong Yidong
  2012-01-07 18:19   ` Stefan Monnier
  2012-01-10  2:02   ` Daiki Ueno
  0 siblings, 2 replies; 6+ messages in thread
From: Chong Yidong @ 2012-01-07  7:12 UTC (permalink / raw)
  To: Daiki Ueno; +Cc: 10403, Paul Eggert

Could you review the patch posted by Paul?  Thanks.


Paul Eggert <eggert@cs.ucla.edu> writes:

> === modified file 'lisp/ChangeLog'
> --- lisp/ChangeLog	2011-12-29 21:55:33 +0000
> +++ lisp/ChangeLog	2011-12-29 22:08:29 +0000
> @@ -1,5 +1,8 @@
>  2011-12-29  Paul Eggert  <eggert@cs.ucla.edu>
>  
> +	* epg.el (epg--make-temp-file): Avoid permission race conditions
> +	when creating temporary directories and files on older Emacs.
> +
>  	* files.el (move-file-to-trash): Preserve default file modes on error.
>  	(Bug#10401)
>  
>
> === modified file 'lisp/epg.el'
> --- lisp/epg.el	2011-11-23 07:03:56 +0000
> +++ lisp/epg.el	2011-12-29 22:08:29 +0000
> @@ -1951,14 +1951,16 @@
>  of PREFIX, and expanding against `temporary-file-directory' if necessary),
>  is guaranteed to point to a newly created empty file.
>  You can then use `write-region' to write new data into the file."
> -      (let (tempdir tempfile)
> +      (let (tempdir tempfile orig-modes)
>  	(setq prefix (expand-file-name prefix
>  				       (if (featurep 'xemacs)
>  					   (temp-directory)
>  					 temporary-file-directory)))
> +	(setq orig-modes (default-file-modes))
>  	(unwind-protect
>  	    (let (file)
>  	      ;; First, create a temporary directory.
> +	      (set-default-file-modes #o700)
>  	      (while (condition-case ()
>  			 (progn
>  			   (setq tempdir (make-temp-name
> @@ -1969,14 +1971,12 @@
>  			   (make-directory tempdir))
>  		       ;; let's try again.
>  		       (file-already-exists t)))
> -	      (set-file-modes tempdir 448)
>  	      ;; Second, create a temporary file in the tempdir.
>  	      ;; There *is* a race condition between `make-temp-name'
>  	      ;; and `write-region', but we don't care it since we are
>  	      ;; in a private directory now.
>  	      (setq tempfile (make-temp-name (concat tempdir "/EMU")))
>  	      (write-region "" nil tempfile nil 'silent)
> -	      (set-file-modes tempfile 384)
>  	      ;; Finally, make a hard-link from the tempfile.
>  	      (while (condition-case ()
>  			 (progn
> @@ -1986,6 +1986,7 @@
>  		       ;; let's try again.
>  		       (file-already-exists t)))
>  	      file)
> +	  (set-default-file-modes orig-modes)
>  	  ;; Cleanup the tempfile.
>  	  (and tempfile
>  	       (file-exists-p tempfile)





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#10403: epg--make-temp-file permissions race condition fixes
  2012-01-07  7:12 ` Chong Yidong
@ 2012-01-07 18:19   ` Stefan Monnier
  2012-01-10  2:57     ` Daiki Ueno
  2012-01-10  2:02   ` Daiki Ueno
  1 sibling, 1 reply; 6+ messages in thread
From: Stefan Monnier @ 2012-01-07 18:19 UTC (permalink / raw)
  To: Daiki Ueno; +Cc: 10403, Paul Eggert, Chong Yidong

> Could you review the patch posted by Paul?  Thanks.
> Paul Eggert <eggert@cs.ucla.edu> writes:

BTW, why copy the code from poe.el rather than from Emacs itself?
Also, why use `eval-and-compile' since the function is not used
during compilation?
I think what you wanted to do instead is

(defalias (if (fboundp 'make-temp-file) 'make-temp-file
            (lambda (...) ...the copied fallback code...)))


-- Stefan





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#10403: epg--make-temp-file permissions race condition fixes
  2012-01-07  7:12 ` Chong Yidong
  2012-01-07 18:19   ` Stefan Monnier
@ 2012-01-10  2:02   ` Daiki Ueno
  2012-01-14  7:09     ` Chong Yidong
  1 sibling, 1 reply; 6+ messages in thread
From: Daiki Ueno @ 2012-01-10  2:02 UTC (permalink / raw)
  To: Chong Yidong; +Cc: 10403, Paul Eggert

Chong Yidong <cyd@gnu.org> writes:

> Could you review the patch posted by Paul?  Thanks.

Thanks.  It looks good to me.
I don't quite remember the intent of that permission logic though...

For the record, the original code is:
http://cvs.m17n.org/viewcvs/root/apel/poe.el?revision=1.90&view=markup
(the copyright has been assigned to FSF just in case)

Regards,
-- 
Daiki Ueno





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#10403: epg--make-temp-file permissions race condition fixes
  2012-01-07 18:19   ` Stefan Monnier
@ 2012-01-10  2:57     ` Daiki Ueno
  0 siblings, 0 replies; 6+ messages in thread
From: Daiki Ueno @ 2012-01-10  2:57 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 10403, Paul Eggert, Chong Yidong

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Could you review the patch posted by Paul?  Thanks.
>> Paul Eggert <eggert@cs.ucla.edu> writes:
>
> BTW, why copy the code from poe.el rather than from Emacs itself?

Maybe because poe.el's code seems to provide better compatibility with
ancient Emacs and XEmacs.  For example, in the Emacs definition of
make-temp-file, I see MUSTBENEW arg of write-region is used, which is
missing in XEmacs 21.4.

> Also, why use `eval-and-compile' since the function is not used
> during compilation?

Right.  I think we could get rid of such a eval-and-compile from several
places.

Regards,
-- 
Daiki Ueno





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#10403: epg--make-temp-file permissions race condition fixes
  2012-01-10  2:02   ` Daiki Ueno
@ 2012-01-14  7:09     ` Chong Yidong
  0 siblings, 0 replies; 6+ messages in thread
From: Chong Yidong @ 2012-01-14  7:09 UTC (permalink / raw)
  To: Daiki Ueno; +Cc: 10403, Paul Eggert

Daiki Ueno <ueno@unixuser.org> writes:

> Chong Yidong <cyd@gnu.org> writes:
>
>> Could you review the patch posted by Paul?  Thanks.
>
> Thanks.  It looks good to me.
> I don't quite remember the intent of that permission logic though...

Thanks, committed.  Closing the bug.






^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-01-14  7:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-29 22:22 bug#10403: epg--make-temp-file permissions race condition fixes Paul Eggert
2012-01-07  7:12 ` Chong Yidong
2012-01-07 18:19   ` Stefan Monnier
2012-01-10  2:57     ` Daiki Ueno
2012-01-10  2:02   ` Daiki Ueno
2012-01-14  7:09     ` Chong Yidong

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).