Subject: bug#33965: handling of closed file descriptors

From: Paul Eggert <[email-masked]>
Date: Sat, 5 Jan 2019 20:06:51 -0800

Bruno Haible wrote:

>> The feature is not portable to POSIX
>> platforms. Besides, I doubt whether anybody is using the feature anyway.
> 
> OK, if you question the "feature" as a whole,

Sorry, I was referring only to the never-used feature of diffutils, where "-" in 
a command-line argument stands for a missing file if stdin is closed (only in 
some cases, e.g., -N). I think nobody uses this feature or will miss it if it's 
removed, and anyway it can't work on some POSIX-conforming platforms. I put the 
feature into diffutils in many years ago, but it's now clear that the test case 
that is exercising it is a mistake and that the feature itself was mistaken.

But to broaden this to the more-general issue of whether kernels should force 
stdin/stdout/stderr to be open after an exec:

>    I predict that this "simple change to the kernel" will not make it into
>    the majority of the operating systems in the next 10 years. (But you
>    could start lobbying for it among the OpenBSD people. They would be the
>    most likely to adopt it, I guess.)

OpenBSD has already adopted it for setuid and setgid executables, as has 
FreeBSD, NetBSD, and SELinux. I imagine this feature is reasonably common 
elsewhere, at least for setuid (I haven't checked carefully). As far as I know 
only HP-UX does it for non-setuid/gid executables.

>    In the mean time, the remaining options I can see are:
> 
>      (a) Keep using the *-safer Gnulib modules.
> 
>      (b) Introduce a sanity_check_file_descriptors() function
 > ...
>          void
>          sanity_check_file_descriptors (void)
>          {
>            int fd;
>            for (fd = 0; fd <= 2; fd++)
>              if (fcntl (fd, F_GETFD, NULL) < 0 && errno == EBADF)
>                exit (125);
>          }
> ... 
>    (b) surely is simpler than (a).

I suggest instead a third option (c) that was formerly used in Coreutils, which 
is that the program makes sure that stdin/stdout/stderr are open before doing 
anything that creates a file descriptor. As I understand it Coreutils moved away 
from that approach primarily because it was redundant if the (earlier) *-safer 
modules are also used. However, in hindsight perhaps Coreutils should have 
stopped using the *-safer modules and just used method (c); it's a lot simpler 
and less intrusive.

I resurrected and refreshed that old Coreutils code and put it into a new Gnulib 
module stdopen, adjusted Gnulib's xfreopen module in a couple of minor ways 
(only diffutils uses xfreopen as far as I know) so that it no longer requires 
freopen-safer, and installed the first four attached patches into Gnulib. I then 
propagated this into diffutils and removed the unportable feature by installing 
the last three attached patches into diffutils.

As a result of these changes, diffutils no longer uses the *-safer modules (it 
uses the new stdopen module instead), it no longer supports the unportable 
feature in question, and it no longer mishandles some obscure situations where 
stdin, stdout, or stderr are closed.
>From 14e1f1c39766bcb3fe4d6ec975ecac4058d11d98 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Sat, 5 Jan 2019 11:02:51 -0800
Subject: [PATCH 1/4] stdopen: copy from last use in coreutils

* lib/stdopen.c, lib/stdopen.h, m4/stdopen.m4:
New files, taken from their last commit in coreutils
2007-07-23T12:35:[email-masked]
71aa3ea88084d17bcb4fc1031ad7b66f8647115e.
---
 ChangeLog     |  8 ++++++
 lib/stdopen.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/stdopen.h | 16 +++++++++++
 m4/stdopen.m4 | 13 +++++++++
 4 files changed, 111 insertions(+)
 create mode 100644 lib/stdopen.c
 create mode 100644 lib/stdopen.h
 create mode 100644 m4/stdopen.m4

diff --git a/ChangeLog b/ChangeLog
index d05cd8453..72d75ef26 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2019-01-05  Paul Eggert  <[email-masked]>
+
+	stdopen: copy from last use in coreutils
+	* lib/stdopen.c, lib/stdopen.h, m4/stdopen.m4:
+	New files, taken from their last commit in coreutils
+	2007-07-23T12:35:[email-masked]
+	71aa3ea88084d17bcb4fc1031ad7b66f8647115e.
+
 2019-01-05  Bruno Haible  <[email-masked]>
 
 	argp: Don't pass an invalid argument to dgettext().
diff --git a/lib/stdopen.c b/lib/stdopen.c
new file mode 100644
index 000000000..e9b554485
--- /dev/null
+++ b/lib/stdopen.c
@@ -0,0 +1,74 @@
+/* stdopen.c - ensure that the three standard file descriptors are in use
+
+   Copyright (C) 2005, 2006 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/>.  */
+
+/* Written by Paul Eggert and Jim Meyering.  */
+
+#include <config.h>
+
+#include "stdopen.h"
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <errno.h>
+
+/* Try to ensure that all of the standard file numbers (0, 1, 2)
+   are in use.  Without this, each application would have to guard
+   every call to open, dup, fopen, etc. with tests to ensure they
+   don't use one of the special file numbers when opening a file.
+   Return false if at least one of the file descriptors is initially
+   closed and an attempt to reopen it fails.  Otherwise, return true.  */
+bool
+stdopen (void)
+{
+  int fd;
+  bool ok = true;
+
+  for (fd = 0; fd <= 2; fd++)
+    {
+      if (fcntl (fd, F_GETFD) < 0)
+	{
+	  if (errno != EBADF)
+	    ok = false;
+	  else
+	    {
+	      static const int contrary_mode[]
+		= { O_WRONLY, O_RDONLY, O_RDONLY };
+	      int mode = contrary_mode[fd];
+	      int new_fd;
+	      /* Open /dev/null with the contrary mode so that the typical
+		 read (stdin) or write (stdout, stderr) operation will fail.
+		 With descriptor 0, we can do even better on systems that
+		 have /dev/full, by opening that write-only instead of
+		 /dev/null.  The only drawback is that a write-provoked
+		 failure comes with a misleading errno value, ENOSPC.  */
+	      if (mode == O_RDONLY
+		  || (new_fd = open ("/dev/full", mode) != fd))
+		new_fd = open ("/dev/null", mode);
+	      if (new_fd != fd)
+		{
+		  if (0 <= new_fd)
+		    close (new_fd);
+		  ok = false;
+		}
+	    }
+	}
+    }
+
+  return ok;
+}
diff --git a/lib/stdopen.h b/lib/stdopen.h
new file mode 100644
index 000000000..d54e5f135
--- /dev/null
+++ b/lib/stdopen.h
@@ -0,0 +1,16 @@
+#ifndef STDOPEN_H
+# define STDOPEN_H 1
+
+# include <stdbool.h>
+
+# ifdef __cplusplus
+extern "C" {
+# endif
+
+bool stdopen (void);
+
+# ifdef __cplusplus
+}
+# endif
+
+#endif
diff --git a/m4/stdopen.m4 b/m4/stdopen.m4
new file mode 100644
index 000000000..b18c22ddf
--- /dev/null
+++ b/m4/stdopen.m4
@@ -0,0 +1,13 @@
+#serial 1
+dnl Copyright (C) 2005 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_STDOPEN],
+[
+  AC_LIBSOURCES([stdopen.c, stdopen.h])
+  AC_LIBOBJ([stdopen])
+
+  dnl Prerequisites.
+])
-- 
2.20.1

>From a4015f0453aeb60cfa70dcdfbd78db62b3a79b62 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Sat, 5 Jan 2019 18:02:32 -0800
Subject: [PATCH 2/4] stdopen: modernize and simplify

* lib/stdopen.c: Update copyright date
Do not include sys/types.h; no longer needed these days.
(stdopen): Use C99-style decl in loop.  Return int errno
value, rather than just a bool.  Do not worry about fd mismatches,
since the caller cares only if 0, 1, 2 are occupied.
* lib/stdopen.h: No need to include <stdbool.h>.
* m4/stdopen.m4: Remove.
* modules/stdopen: New file.
---
 ChangeLog       | 10 ++++++++
 lib/stdopen.c   | 63 +++++++++++++++++++++----------------------------
 lib/stdopen.h   |  4 +---
 m4/stdopen.m4   | 13 ----------
 modules/stdopen | 24 +++++++++++++++++++
 5 files changed, 62 insertions(+), 52 deletions(-)
 delete mode 100644 m4/stdopen.m4
 create mode 100644 modules/stdopen

diff --git a/ChangeLog b/ChangeLog
index 72d75ef26..16ad6f169 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,15 @@
 2019-01-05  Paul Eggert  <[email-masked]>
 
+	stdopen: modernize and simplify
+	* lib/stdopen.c: Update copyright date
+	Do not include sys/types.h; no longer needed these days.
+	(stdopen): Use C99-style decl in loop.  Return int errno
+	value, rather than just a bool.  Do not worry about fd mismatches,
+	since the caller cares only if 0, 1, 2 are occupied.
+	* lib/stdopen.h: No need to include <stdbool.h>.
+	* m4/stdopen.m4: Remove.
+	* modules/stdopen: New file.
+
 	stdopen: copy from last use in coreutils
 	* lib/stdopen.c, lib/stdopen.h, m4/stdopen.m4:
 	New files, taken from their last commit in coreutils
diff --git a/lib/stdopen.c b/lib/stdopen.c
index e9b554485..3903224e6 100644
--- a/lib/stdopen.c
+++ b/lib/stdopen.c
@@ -1,6 +1,6 @@
 /* stdopen.c - ensure that the three standard file descriptors are in use
 
-   Copyright (C) 2005, 2006 Free Software Foundation, Inc.
+   Copyright (C) 2005-2006, 2019 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
@@ -21,7 +21,6 @@
 
 #include "stdopen.h"
 
-#include <sys/types.h>
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <unistd.h>
@@ -31,44 +30,36 @@
    are in use.  Without this, each application would have to guard
    every call to open, dup, fopen, etc. with tests to ensure they
    don't use one of the special file numbers when opening a file.
-   Return false if at least one of the file descriptors is initially
-   closed and an attempt to reopen it fails.  Otherwise, return true.  */
-bool
+   Return zero if successful, an errno value if at least one of
+   the file descriptors is initially closed and could not be opened.  */
+
+int
 stdopen (void)
 {
-  int fd;
-  bool ok = true;
-
-  for (fd = 0; fd <= 2; fd++)
+  for (int fd = STDIN_FILENO; fd <= STDERR_FILENO; fd++)
     {
       if (fcntl (fd, F_GETFD) < 0)
-	{
-	  if (errno != EBADF)
-	    ok = false;
-	  else
-	    {
-	      static const int contrary_mode[]
-		= { O_WRONLY, O_RDONLY, O_RDONLY };
-	      int mode = contrary_mode[fd];
-	      int new_fd;
-	      /* Open /dev/null with the contrary mode so that the typical
-		 read (stdin) or write (stdout, stderr) operation will fail.
-		 With descriptor 0, we can do even better on systems that
-		 have /dev/full, by opening that write-only instead of
-		 /dev/null.  The only drawback is that a write-provoked
-		 failure comes with a misleading errno value, ENOSPC.  */
-	      if (mode == O_RDONLY
-		  || (new_fd = open ("/dev/full", mode) != fd))
-		new_fd = open ("/dev/null", mode);
-	      if (new_fd != fd)
-		{
-		  if (0 <= new_fd)
-		    close (new_fd);
-		  ok = false;
-		}
-	    }
-	}
+        {
+          /* Open /dev/null with the contrary mode so that the typical
+             read (stdin) or write (stdout, stderr) operation will fail.
+             With descriptor 0, we can do even better on systems that
+             have /dev/full, by opening that write-only instead of
+             /dev/null.  The only drawback is that a write-provoked
+             failure comes with a misleading errno value, ENOSPC.  */
+          int mode = fd == STDIN_FILENO ? O_WRONLY : O_RDONLY;
+          int full_fd = fd == STDIN_FILENO ? open ("/dev/full", mode) : -1;
+          int new_fd = full_fd < 0 ? open ("/dev/null", mode) : full_fd;
+          if (new_fd < 0)
+            return errno;
+          if (STDERR_FILENO < new_fd)
+            {
+              /* 0, 1, and 2 are already open somehow.
+                 Our is not to reason why.  */
+              close (new_fd);
+              return 0;
+            }
+        }
     }
 
-  return ok;
+  return 0;
 }
diff --git a/lib/stdopen.h b/lib/stdopen.h
index d54e5f135..27901d397 100644
--- a/lib/stdopen.h
+++ b/lib/stdopen.h
@@ -1,13 +1,11 @@
 #ifndef STDOPEN_H
 # define STDOPEN_H 1
 
-# include <stdbool.h>
-
 # ifdef __cplusplus
 extern "C" {
 # endif
 
-bool stdopen (void);
+int stdopen (void);
 
 # ifdef __cplusplus
 }
diff --git a/m4/stdopen.m4 b/m4/stdopen.m4
deleted file mode 100644
index b18c22ddf..000000000
--- a/m4/stdopen.m4
+++ /dev/null
@@ -1,13 +0,0 @@
-#serial 1
-dnl Copyright (C) 2005 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_STDOPEN],
-[
-  AC_LIBSOURCES([stdopen.c, stdopen.h])
-  AC_LIBOBJ([stdopen])
-
-  dnl Prerequisites.
-])
diff --git a/modules/stdopen b/modules/stdopen
new file mode 100644
index 000000000..81de5765b
--- /dev/null
+++ b/modules/stdopen
@@ -0,0 +1,24 @@
+Description:
+Arrange for stdin/stdout/stderr to be open.
+
+Files:
+lib/stdopen.c
+lib/stdopen.h
+
+Depends-on:
+fcntl
+unistd
+
+configure.ac:
+
+Makefile.am:
+lib_SOURCES += stdopen.c
+
+Include:
+"stdopen.h"
+
+License:
+GPL
+
+Maintainer:
+all
-- 
2.20.1

>From f9d6fc71e4ebc142410352253fb1db337f530bdf Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Sat, 5 Jan 2019 18:03:10 -0800
Subject: [PATCH 3/4] xfreopen need not depend on freopen-safer

* modules/xfreopen (Depends-on):
Depend on freopen, not freopen-safer.
---
 ChangeLog        | 4 ++++
 modules/xfreopen | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index 16ad6f169..a88a52083 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
 2019-01-05  Paul Eggert  <[email-masked]>
 
+	xfreopen need not depend on freopen-safer
+	* modules/xfreopen (Depends-on):
+	Depend on freopen, not freopen-safer.
+
 	stdopen: modernize and simplify
 	* lib/stdopen.c: Update copyright date
 	Do not include sys/types.h; no longer needed these days.
diff --git a/modules/xfreopen b/modules/xfreopen
index dcc48e0ac..850446d7a 100644
--- a/modules/xfreopen
+++ b/modules/xfreopen
@@ -8,7 +8,7 @@ lib/xfreopen.h
 Depends-on:
 error
 exitfail
-freopen-safer
+freopen
 quote
 
 configure.ac:
-- 
2.20.1

>From 10652b56fe5b76370187a06296168cff9cffb730 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Sat, 5 Jan 2019 18:55:19 -0800
Subject: [PATCH 4/4] xfreopen need not include stdio--.h

* lib/xfreopen.c: Do not include stdio--.h.
---
 ChangeLog      | 3 +++
 lib/xfreopen.c | 1 -
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index a88a52083..73a86f29a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,8 @@
 2019-01-05  Paul Eggert  <[email-masked]>
 
+	xfreopen need not include stdio--.h
+	* lib/xfreopen.c: Do not include stdio--.h.
+
 	xfreopen need not depend on freopen-safer
 	* modules/xfreopen (Depends-on):
 	Depend on freopen, not freopen-safer.
diff --git a/lib/xfreopen.c b/lib/xfreopen.c
index 32a38d444..46a7c92a5 100644
--- a/lib/xfreopen.c
+++ b/lib/xfreopen.c
@@ -21,7 +21,6 @@
 #include "error.h"
 #include "exitfail.h"
 #include "quote.h"
-#include "stdio--.h"
 
 #include "gettext.h"
 #define _(msgid) gettext (msgid)
-- 
2.20.1

>From 1104d026514014808629f28f2e6c787bf3c520c4 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Sat, 5 Jan 2019 18:05:33 -0800
Subject: [PATCH 1/3] build: update gnulib submodule to latest

---
 gnulib | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gnulib b/gnulib
index 4652c7b..10652b5 160000
--- a/gnulib
+++ b/gnulib
@@ -1 +1 @@
-Subproject commit 4652c7bafa60332145f1e05a7de5f48e1bc56226
+Subproject commit 10652b56fe5b76370187a06296168cff9cffb730
-- 
2.17.1

>From 2c9d956aacdfddbb87334c985cd995d75102f789 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Sat, 5 Jan 2019 19:20:11 -0800
Subject: [PATCH 2/3] =?UTF-8?q?diff:=20remove=20unportable=20=E2=80=98diff?=
 =?UTF-8?q?=20-N=20-=20f=20<&-=E2=80=99=20feature?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* NEWS: Mention this.
* bootstrap.conf (gnulib_modules): Add stdopen.
* doc/diffutils.texi (Comparing Directories):
Do not document behavior if stdin is closed.
* src/diff.c: Include stdopen.h.
(main): Call stdopen early.
(compare_files) [__hpux]: Remove recently-introduced
special case for HP-UX exec with stdin closed.
* tests/new-file: Remove tests of the removed feature.
---
 NEWS               |  9 +++++++++
 bootstrap.conf     |  1 +
 doc/diffutils.texi |  3 +--
 src/diff.c         | 11 ++++-------
 tests/new-file     | 14 --------------
 5 files changed, 15 insertions(+), 23 deletions(-)

diff --git a/NEWS b/NEWS
index d45d0fc..af675da 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,15 @@ GNU diffutils NEWS                                    -*- outline -*-
 
 * Noteworthy changes in release ?.? (????-??-??) [?]
 
+** Incompatible changes
+
+  diff no longer treats a closed stdin as representing an absent file
+  in usage like 'diff --new-file - foo <&-'.  This feature was rarely
+  if ever used and was not portable to POSIX platforms that reopen
+  stdin on exec, such as SELinux if the process underwent an AT_SECURE
+  transition, or HP-UX even if not setuid.
+  [bug#33965 introduced in 2.8]
+
 
 * Noteworthy changes in release 3.7 (2018-12-31) [stable]
 
diff --git a/bootstrap.conf b/bootstrap.conf
index 48cadd4..0c9fbfe 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -67,6 +67,7 @@ stat
 stat-macros
 stat-time
 stdint
+stdopen
 strcase
 strftime
 strptime
diff --git a/doc/diffutils.texi b/doc/diffutils.texi
index 42d3abd..3a780db 100644
--- a/doc/diffutils.texi
+++ b/doc/diffutils.texi
@@ -1796,8 +1796,7 @@ second position.)  To do this, use the @option{--new-file}
 (@option{-N}) option.  This option affects command-line arguments as
 well as files found via directory traversal; for example, @samp{diff
 -N a b} treats @file{a} as empty if @file{a} does not exist but
-@file{b} does, and similarly @samp{diff -N - b} treats standard input
-as empty if it is closed but @file{b} exists.
+@file{b} does.
 
 If the older directory contains large files that are not in
 the newer directory, you can make the patch smaller by using the
diff --git a/src/diff.c b/src/diff.c
index 2ed3ae5..a5ec7f4 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -37,6 +37,7 @@
 #include <progname.h>
 #include <sh-quote.h>
 #include <stat-time.h>
+#include <stdopen.h>
 #include <timespec.h>
 #include <version-etc.h>
 #include <xalloc.h>
@@ -297,6 +298,9 @@ main (int argc, char **argv)
   re_set_syntax (RE_SYNTAX_GREP | RE_NO_POSIX_BACKTRACKING);
   excluded = new_exclude ();
   presume_output_tty = false;
+  int stdopen_errno = stdopen ();
+  if (stdopen_errno != 0)
+    error (EXIT_TROUBLE, stdopen_errno, "standard file descriptors");
 
   /* Decode the options.  */
 
@@ -1172,13 +1176,6 @@ compare_files (struct comparison const *parent,
 	      cmp.file[f].desc = STDIN_FILENO;
 	      if (binary && ! isatty (STDIN_FILENO))
 		set_binary_mode (STDIN_FILENO, O_BINARY);
-#ifdef __hpux
-	      /* Recognize file descriptors closed by the parent on HP-UX.  */
-	      int flags = fcntl (STDIN_FILENO, F_GETFL, NULL);
-	      if (flags >= 0 && (flags & FD_CLOEXEC) != 0)
-		cmp.file[f].desc = ERRNO_ENCODE (EBADF);
-	      else
-#endif
 	      if (fstat (STDIN_FILENO, &cmp.file[f].stat) != 0)
 		cmp.file[f].desc = ERRNO_ENCODE (errno);
 	      else
diff --git a/tests/new-file b/tests/new-file
index 4400051..e3a61da 100755
--- a/tests/new-file
+++ b/tests/new-file
@@ -10,12 +10,6 @@ echo a > a || fail=1
 echo '0a1
 > a' > exp || fail=1
 
-returns_ 1 diff -N - a <&- > out || fail=1
-compare exp out || fail=1
-
-returns_ 1 diff --unidirectional-new-file - a <&- > out || fail=1
-compare exp out || fail=1
-
 returns_ 1 diff -N b - < a > out || fail=1
 compare exp out || fail=1
 
@@ -25,14 +19,6 @@ compare exp out || fail=1
 echo '1d0
 < a' > exp || fail=1
 
-returns_ 1 diff -N a - <&- > out || fail=1
-compare exp out || fail=1
-
-# With closed standard input, require an exit status of 2
-# and empty stdout.
-returns_ 2 diff --unidirectional-new-file a - <&- > out || fail=1
-compare /dev/null out || fail=1
-
 returns_ 1 diff -N - b < a > out || fail=1
 compare exp out || fail=1
 
-- 
2.17.1

>From 601eceb57cfb87ded594701acc401277c53ff837 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Sat, 5 Jan 2019 19:55:08 -0800
Subject: [PATCH 3/3] diff: fix cmp, diff3, sdiff with stdin closed

* NEWS: Mention this.
* src/cmp.c, src/diff3.c, src/sdiff.c: Include stdopen.h.
(main): Call stdopen early.
* src/cmp.c (main): Simplify now that we need not worry about
stdin being closed.
* src/diff.c (main): Translate stdopen diagnostic.
---
 NEWS        |  8 ++++++++
 src/cmp.c   | 28 ++++++++++++++--------------
 src/diff.c  |  2 +-
 src/diff3.c |  4 ++++
 src/sdiff.c |  4 ++++
 5 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/NEWS b/NEWS
index af675da..9ab0748 100644
--- a/NEWS
+++ b/NEWS
@@ -11,6 +11,14 @@ GNU diffutils NEWS                                    -*- outline -*-
   transition, or HP-UX even if not setuid.
   [bug#33965 introduced in 2.8]
 
+** Bug fixes
+
+  diff and related programs no longer get confused if stdin, stdout,
+  or stderr are closed.  Previously, they sometimes opened files into
+  file descriptors 0, 1, or 2 and then mistakenly did I/O with them
+  that was intended for stdin, stdout, or stderr.
+  [bug#33965 present since "the beginning"]
+
 
 * Noteworthy changes in release 3.7 (2018-12-31) [stable]
 
diff --git a/src/cmp.c b/src/cmp.c
index 53ff210..7bad6dd 100644
--- a/src/cmp.c
+++ b/src/cmp.c
@@ -31,6 +31,7 @@
 #include <hard-locale.h>
 #include <inttostr.h>
 #include <progname.h>
+#include <stdopen.h>
 #include <unlocked-io.h>
 #include <version-etc.h>
 #include <xalloc.h>
@@ -199,7 +200,7 @@ GB 1,000,000,000, G 1,073,741,824, and so on for T, P, E, Z, Y."),
 int
 main (int argc, char **argv)
 {
-  int c, f, exit_status;
+  int c, exit_status;
   size_t words_per_buffer;
 
   exit_failure = EXIT_TROUBLE;
@@ -209,6 +210,9 @@ main (int argc, char **argv)
   bindtextdomain (PACKAGE, LOCALEDIR);
   textdomain (PACKAGE);
   c_stack_action (0);
+  int stdopen_errno = stdopen ();
+  if (stdopen_errno != 0)
+    error (EXIT_TROUBLE, stdopen_errno, _("standard file descriptors"));
 
   /* Parse command line options.  */
 
@@ -268,7 +272,7 @@ main (int argc, char **argv)
   file[0] = argv[optind++];
   file[1] = optind < argc ? argv[optind++] : "-";
 
-  for (f = 0; f < 2 && optind < argc; f++)
+  for (int f = 0; f < 2 && optind < argc; f++)
     {
       char *arg = argv[optind++];
       specify_ignore_initial (f, &arg, 0);
@@ -277,33 +281,29 @@ main (int argc, char **argv)
   if (optind < argc)
     try_help ("extra operand '%s'", argv[optind]);
 
-  for (f = 0; f < 2; f++)
+  for (int f = 0; f < 2; f++)
     {
-      /* If file[1] is "-", treat it first; this avoids a misdiagnostic if
-	 stdin is closed and opening file[0] yields file descriptor 0.  */
-      int f1 = f ^ (STREQ (file[1], "-"));
-
       /* Two files with the same name and offset are identical.
 	 But wait until we open the file once, for proper diagnostics.  */
       if (f && ignore_initial[0] == ignore_initial[1]
 	  && file_name_cmp (file[0], file[1]) == 0)
 	return EXIT_SUCCESS;
 
-      if (STREQ (file[f1], "-"))
+      if (STREQ (file[f], "-"))
 	{
-	  file_desc[f1] = STDIN_FILENO;
+	  file_desc[f] = STDIN_FILENO;
 	  if (O_BINARY && ! isatty (STDIN_FILENO))
 	    set_binary_mode (STDIN_FILENO, O_BINARY);
 	}
       else
-	file_desc[f1] = open (file[f1], O_RDONLY | O_BINARY, 0);
+	file_desc[f] = open (file[f], O_RDONLY | O_BINARY, 0);
 
-      if (file_desc[f1] < 0 || fstat (file_desc[f1], stat_buf + f1) != 0)
+      if (file_desc[f] < 0 || fstat (file_desc[f], stat_buf + f) != 0)
 	{
-	  if (file_desc[f1] < 0 && comparison_type == type_status)
+	  if (file_desc[f] < 0 && comparison_type == type_status)
 	    exit (EXIT_TROUBLE);
 	  else
-	    die (EXIT_TROUBLE, errno, "%s", file[f1]);
+	    die (EXIT_TROUBLE, errno, "%s", file[f]);
 	}
     }
 
@@ -361,7 +361,7 @@ main (int argc, char **argv)
 
   exit_status = cmp ();
 
-  for (f = 0; f < 2; f++)
+  for (int f = 0; f < 2; f++)
     if (close (file_desc[f]) != 0)
       die (EXIT_TROUBLE, errno, "%s", file[f]);
   if (exit_status != EXIT_SUCCESS && comparison_type < type_no_stdout)
diff --git a/src/diff.c b/src/diff.c
index a5ec7f4..d54782c 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -300,7 +300,7 @@ main (int argc, char **argv)
   presume_output_tty = false;
   int stdopen_errno = stdopen ();
   if (stdopen_errno != 0)
-    error (EXIT_TROUBLE, stdopen_errno, "standard file descriptors");
+    error (EXIT_TROUBLE, stdopen_errno, _("standard file descriptors"));
 
   /* Decode the options.  */
 
diff --git a/src/diff3.c b/src/diff3.c
index c7b2840..36bf4a6 100644
--- a/src/diff3.c
+++ b/src/diff3.c
@@ -30,6 +30,7 @@
 #include <file-type.h>
 #include <getopt.h>
 #include <progname.h>
+#include <stdopen.h>
 #include <system-quote.h>
 #include <version-etc.h>
 #include <xalloc.h>
@@ -272,6 +273,9 @@ main (int argc, char **argv)
   bindtextdomain (PACKAGE, LOCALEDIR);
   textdomain (PACKAGE);
   c_stack_action (0);
+  int stdopen_errno = stdopen ();
+  if (stdopen_errno != 0)
+    error (EXIT_TROUBLE, stdopen_errno, _("standard file descriptors"));
 
   while ((c = getopt_long (argc, argv, "aeimvx3AEL:TX", longopts, 0)) != -1)
     {
diff --git a/src/sdiff.c b/src/sdiff.c
index 25ce3fe..bbbe16c 100644
--- a/src/sdiff.c
+++ b/src/sdiff.c
@@ -32,6 +32,7 @@
 #include <file-type.h>
 #include <getopt.h>
 #include <progname.h>
+#include <stdopen.h>
 #include <system-quote.h>
 #include <version-etc.h>
 #include <xalloc.h>
@@ -455,6 +456,9 @@ main (int argc, char *argv[])
   bindtextdomain (PACKAGE, LOCALEDIR);
   textdomain (PACKAGE);
   c_stack_action (cleanup);
+  int stdopen_errno = stdopen ();
+  if (stdopen_errno != 0)
+    error (EXIT_TROUBLE, stdopen_errno, _("standard file descriptors"));
 
   prog = getenv ("EDITOR");
   if (prog)
-- 
2.17.1