GNU bug report logs - #33524
27.0.50; Suspicious code in flymake-proc around temporary directories

Previous Next

Full log

View this message in rfc822 format

From: João Távora <joaotavora <at> gmail.com>
To: Michael Albinus <michael.albinus <at> gmx.de>
Cc: Philipp Stephani <p.stephani2 <at> gmail.com>, 33524 <at> debbugs.gnu.org
Subject: bug#33524: 27.0.50; Suspicious code in flymake-proc around temporary directories
Date: Fri, 30 Nov 2018 14:00:19 +0000

[Message part 1 (text/plain, inline)]

No, it looks great, for all I know... Which is very little :-)

Flymake-proc is the "legacy" backend that I packed up in a file. It
probably has many such bugs.

Thanks very much Michael and Philipp,
João



On Fri, Nov 30, 2018 at 12:59 PM Michael Albinus <michael.albinus <at> gmx.de>
wrote:

> Philipp Stephani <p.stephani2 <at> gmail.com> writes:
>
> Hi Philipp,
>
> > Sorry for being imprecise. Yes, I mean quoted filenames. (It's
> > possible that the issue also arises for true remote filenames, but I
> > haven't checked.)
>
> "True remote filename" is still a wrong phrase. Quoted file names and
> remote file names play different games.
>
> > You can easily reproduce user-facing problems. Assuming you have some
> > binary installed that would check Java files using the legacy backend:
> >
> > $ touch /tmp/{a,b}.java
> > $ emacs -Q -eval '(progn (add-hook (quote prog-mode-hook) (quote
> > flymake-mode)) (ediff "/:/tmp/a.java" "/:/tmp/b.java"))'
> >
> > And Emacs immediately hangs because it runs into the endless loop.
> > Stacktrace in this case is
> >
> > Debugger entered--Lisp error: (quit)
> >   display-warning((flymake flymake-proc) "Failed to delete dir /,
> > error ignored" :error "*Flymake log*")
> >   flymake--log-1(1 flymake-proc "Failed to delete dir %s, error
> > ignored" "/")
> >   flymake-proc--safe-delete-directory("/")
> >   flymake-proc--delete-temp-directory("/tmp/:/tmp/")
> >   flymake-proc-simple-java-cleanup()
> >   #f(compiled-function () #<bytecode 0x11a4ea9>)()
> >   flymake-proc-legacy-flymake(#f(compiled-function (&rest args)
> > #<bytecode 0x11a4e51>))
> >   flymake--run-backend(flymake-proc-legacy-flymake)
> >   #f(compiled-function (backend) #<bytecode 0x117b195>)
> > (flymake-proc-legacy-flymake)
> >   run-hook-wrapped(#f(compiled-function (backend) #<bytecode
> > 0x117b195>) flymake-proc-legacy-flymake)
> >   flymake-start((post-command) nil)
> >   #f(compiled-function () #<bytecode 0x1180771>)()
> >   set-window-buffer(nil #<buffer a.java>)
> >   switch-to-buffer(#<buffer a.java>)
> >   ediff-setup-windows-multiframe-compare(#<buffer a.java> #<buffer
> > b.java> nil #<buffer *Ediff Control Panel*>)
> >   ediff-setup-windows-multiframe(#<buffer a.java> #<buffer b.java> nil
> > #<buffer *Ediff Control Panel*>)
> >   ediff-setup-windows-default(#<buffer a.java> #<buffer b.java> nil
> > #<buffer *Ediff Control Panel*>)
> >   ediff-setup-windows(#<buffer a.java> #<buffer b.java> nil #<buffer
> > *Ediff Control Panel*>)
> >   ediff-setup(#<buffer a.java> "/:/tmp/a.java" #<buffer b.java>
> > "/:/tmp/b.java" nil nil nil ((ediff-job-name . ediff-files)) nil)
> >   ediff-files-internal("/:/tmp/a.java" "/:/tmp/b.java" nil nil
> > ediff-files)
> >   ediff("/:/tmp/a.java" "/:/tmp/b.java")
> >   (progn (add-hook (quote prog-mode-hook) (quote flymake-mode)) (ediff
> > "/:/tmp/a.java" "/:/tmp/b.java"))
> >   eval((progn (add-hook (quote prog-mode-hook) (quote flymake-mode))
> > (ediff "/:/tmp/a.java" "/:/tmp/b.java")))
> >   command-line-1(("-f" "toggle-debug-on-quit" "-eval" "(progn
> > (add-hook (quote prog-mode-hook) (quote flymake-mode)) (ediff
> > \"/:/tmp/a.java\" \"/:/tmp/b.java\"))"))
> >   command-line()
> >   normal-top-level()
>
> The following patch fixes this problem:
>
> diff --git a/lisp/progmodes/flymake-proc.el
> b/lisp/progmodes/flymake-proc.el
> index 8600be9b97..e969c5d992 100644
> --- a/lisp/progmodes/flymake-proc.el
> +++ b/lisp/progmodes/flymake-proc.el
> @@ -874,7 +874,7 @@ flymake-proc-create-temp-with-folder-structure
>    (unless (stringp file-name)
>      (error "Invalid file-name"))
>
> -  (let* ((dir       (file-name-directory file-name))
> +  (let* ((dir       (file-name-directory (file-name-unquote file-name)))
>           ;; Not sure what this slash-pos is all about, but I guess it's
> just
>           ;; trying to remove the leading / of absolute file names.
>          (slash-pos (string-match "/" dir))
>
> However, I don't know whether this is sufficient, because I don't know
> why you use quoted file names in your example. Is this something which
> shall be preserved in the temporary directory, created by flymake?
>
> João, do you have further remarks?
>
> Best regards, Michael.
>


-- 
João Távora

[Message part 2 (text/html, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.