bug#48940: 27.2; regression: "emacs --script /dev/stdin" parses the script incorrectly when /dev/stdin is a pipe

["Bryan C. Mills" via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

I did a little digging and found that I can reproduce the issue under
a debugger by constructing a named pipe with `mkfifo pipe.el` and
feeding the script into it using `cat script.el >>pipe.el`. (Then I
can execute `run --script pipe.el` under gdb to capture a backtrace at
the problematic lseek call.)

I identified one bug in the safe_to_load_version function.
I think I've got a fix for it, but I'm not very familiar with the
emacs codebase, especially when it comes to regression testing.
(My patch against the emacs-27 branch is below.)

I tested the fix locally with a small dummy script as input and it
seems to work, but I can't be confident that there won't be deeper
bugs. You're welcome to use it as a starting point for a more robust
fix.


From c97e4b97af86023177417e49c46b19812b2f4caa Mon Sep 17 00:00:00 2001
From: "Bryan C. Mills" <bcmills@google.com>
Date: Mon, 14 Jun 2021 17:30:11 -0400
Subject: [PATCH] Fix invocation with '--script /dev/stdin'

* src/lread.c (safe_to_load_version): Check lseek errors.  (Bug#48940)
---
 src/lread.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/src/lread.c b/src/lread.c
index 47116ad5ae..96b6695b5c 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -990,12 +990,21 @@ #define UPDATE_BEG_END_STATE(ch) \
    because of an incompatible change in the byte compiler.  */

 static int
-safe_to_load_version (int fd)
+safe_to_load_version (Lisp_Object file, int fd)
 {
+  struct stat st;
   char buf[512];
   int nbytes, i;
   int version = 1;

+  /* If the file is not regular, then we cannot safely seek it.
+     Assume that it is not safe to load as a compiled file.  */
+  if (fstat(fd, &st) == 0)
+    {
+      if (!S_ISREG (st.st_mode))
+        return 0;
+    }
+
   /* Read the first few bytes from the file, and look for a line
      specifying the byte compiler version used.  */
   nbytes = emacs_read_quit (fd, buf, sizeof buf);
@@ -1013,7 +1022,9 @@ safe_to_load_version (int fd)
  version = 0;
     }

-  lseek (fd, 0, SEEK_SET);
+  if (lseek (fd, 0, SEEK_SET) < 0)
+    report_file_error ("Seeking to start of file", file);
+
   return version;
 }

@@ -1316,7 +1327,7 @@ DEFUN ("load", Fload, Sload, 1, 5, 0,
   if (is_elc
       /* version = 1 means the file is empty, in which case we can
  treat it as not byte-compiled.  */
-      || (fd >= 0 && (version = safe_to_load_version (fd)) > 1))
+      || (fd >= 0 && (version = safe_to_load_version (file, fd)) > 1))
     /* Load .elc files directly, but not when they are
        remote and have no handler!  */
     {
@@ -1326,7 +1337,7 @@ DEFUN ("load", Fload, Sload, 1, 5, 0,
    int result;

    if (version < 0
-       && ! (version = safe_to_load_version (fd)))
+       && ! (version = safe_to_load_version (file, fd)))
      {
        safe_p = 0;
        if (!load_dangerous_libraries)
-- 
2.32.0.272.g935e593368-goog

On Sun, Jun 13, 2021 at 6:26 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > Date: Wed, 9 Jun 2021 16:31:36 -0400
> > From:  "Bryan C. Mills" via "Bug reports for GNU Emacs,
> >  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> >
> > `emacs --script /dev/stdin` worked reliably for me on Emacs 24 through 26.
> >
> > As of Emacs 27, it no longer works reliably — depending on the script contents,
> > it either fails with spurious errors or silently exits without
> > finishing the script.
> >
> >
> > In the following bash interaction, all three emacs invocations
> > *should* run the same script.
> > The third invocation demonstrates the bug: when /dev/stdin is a pipe from `cat`
> > (instead of the script.el file directly), the script is no longer interpreted.
> > With other scripts, I get various errors, seemingly due to `;` or `"`
> > characters being
> > dropped or ignored from the input.
> >
> > ```
> > $ cat script.el | cat /dev/stdin
> > (print "Hello emacs!")
> >
> > $ emacs --no-init-file --no-site-file --script script.el
> >
> > "Hello emacs!"
> >
> > $ emacs --no-init-file --no-site-file --script /dev/stdin <script.el
> >
> > "Hello emacs!"
> >
> > $ cat script.el | emacs --no-init-file --no-site-file --script /dev/stdin
> >
> > $ echo $?
> > 0
> >
> > $
> > ```
> >
> > This reproduces with the GNU Emacs 27.1 currently packaged on Debian Testing,
> > as well as the Google build of GNU Emacs 27.2 from which I produced
> > the report below.
>
> I see the problem, but it happens for me in Emacs 26 and Emacs 25 as
> well, so I'm not sure this is new, or why it works for you in older
> versions.  Maybe Debian included some local patches in those older
> versions?  (My old Emacs versions were built from the unmodified
> upstream sources.)
>
> AFAICS, the problem seems to be that load-with-code-conversion calls
> insert-file-contents, and the latter comes up with an empty buffer in
> the problematic case.
>
> It is notoriously hard to debug a program whose standard input was
> redirected from a pipe of another program, so I couldn't see why the
> above happens.  If someone could step in a debugger through
> insert-file-contents in this case and describe what's going on there,
> or tell how to do that when Emacs is invoked like this, maybe we
> could make some progress.
>
> Thanks.