Re: mktemp write failure

[Eric Blake]

Eric Blake <ebb9 <at> byu.net> writes:

> > mktemp -q >&-
> 
> I'll be committing this to expose the bug, once I get freopen_safer 
implemented 
> in gnulib to fix the bug.

This patch looks deceptively simple, since all the magic is in gnulib!  I also 
audited all other uses of freopen; changing stdin is unaffected, but changing 
any other stream requires this.  I didn't try to figure out if I could trigger 
inconsistencies with the other affected apps (cat, head, ptx, shuf, tac, tail, 
tee, tr, and uniq).  And we may want to use xfreopen in more places (dircolors, 
du, mktemp, ptx, shuf, tsort, uniq), but I did not do that here.


>From ccb6e85fa2cc21d132a116b5cf8e9f56bf747fe4 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Thu, 5 Nov 2009 12:19:45 -0700
Subject: [PATCH 1/2] mktemp: fix bug with -q and closed stdout

If stdin or stdout is closed, then freopen(,stderr) can violate
the premise that STDERR_FILENO==fileno(stderr), which in turn
breaks mktemp -q.

* gnulib: Update, to pick up freopen_safer.
* bootstrap.conf (gnulib_modules): Add freopen-safer.
* tests/misc/close-stdout: Enhance test to catch bug.
* src/mktemp.c (includes): Use stdio--.h.
---
 bootstrap.conf          |    1 +
 gnulib                  |    2 +-
 src/mktemp.c            |    2 +-
 tests/misc/close-stdout |    6 ++++--
 4 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/bootstrap.conf b/bootstrap.conf
index b3a82e0..8d59ae4 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -91,6 +91,7 @@ gnulib_modules="
   fopen-safer
   fprintftime
   freopen
+  freopen-safer
   fseeko
   fsusage
   fsync
diff --git a/gnulib b/gnulib
index 757089b..1f40750 160000
--- a/gnulib
+++ b/gnulib
@@ -1 +1 @@
-Subproject commit 757089b71ada13e18169e2be3509c1b07bd1c63d
+Subproject commit 1f40750a44348b393e188acbccd0e0e441e3ce93
diff --git a/src/mktemp.c b/src/mktemp.c
index 6ce40f1..303b9ce 100644
--- a/src/mktemp.c
+++ b/src/mktemp.c
@@ -17,7 +17,6 @@
 /* Written by Jim Meyering and Eric Blake.  */

 #include <config.h>
-#include <stdio.h>
 #include <sys/types.h>
 #include <getopt.h>

@@ -27,6 +26,7 @@
 #include "error.h"
 #include "filenamecat.h"
 #include "quote.h"
+#include "stdio--.h"
 #include "tempname.h"

 /* The official name of this program (e.g., no `g' prefix).  */
diff --git a/tests/misc/close-stdout b/tests/misc/close-stdout
index 852c3c8..ae2350d 100755
--- a/tests/misc/close-stdout
+++ b/tests/misc/close-stdout
@@ -52,7 +52,8 @@ if "$p/src/test" -w /dev/stdout >/dev/null &&
   cp --verbose a b >&- 2>/dev/null && fail=1
   rm -Rf tmpfile-?????? || fail=1
   mktemp tmpfile-XXXXXX >&- 2>/dev/null && fail=1
-  test -e tmpfile-?????? && fail=1
+  mktemp tmpfile-XXXXXX -q >&- 2>/dev/null && fail=1
+  case `echo tmpfile-??????` in 'tmpfile-??????') ;; *) fail=1 ;; esac
 fi

 # Likewise for /dev/full, if /dev/full works.
@@ -61,7 +62,8 @@ if test -w /dev/full && test -c /dev/full; then
   cp --verbose a b >/dev/full 2>/dev/null && fail=1
   rm -Rf tmpdir-?????? || fail=1
   mktemp -d tmpdir-XXXXXX >/dev/full 2>/dev/null && fail=1
-  test -e tmpdir-?????? && fail=1
+  mktemp -d -q tmpdir-XXXXXX >/dev/full 2>/dev/null && fail=1
+  case `echo tmpfile-??????` in 'tmpfile-??????') ;; *) fail=1 ;; esac
 fi

 Exit $fail
-- 
1.6.4.2


>From 6b55bc3aeef3e08f1eeee6b85153f95969840891 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Thu, 5 Nov 2009 16:48:09 -0700
Subject: [PATCH 2/2] build: consistently use freopen-safer

cat, head, ptx, shuf, tac, tail, tee, tr, and uniq were affected

* gl/modules/xfreopen (Depends-on): Add freopen-safer.
* gl/lib/xfreopen.c (includes): Use stdio--.h.
* src/ptx.c (includes): Likewise.
* src/shuf.c (includes): Likewise.
* src/uniq.c (includes): Likewise.
---
 gl/lib/xfreopen.c   |    1 +
 gl/modules/xfreopen |    1 +
 src/ptx.c           |    2 +-
 src/shuf.c          |    2 +-
 src/uniq.c          |    2 +-
 5 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/gl/lib/xfreopen.c b/gl/lib/xfreopen.c
index 6109169..32e68fa 100644
--- a/gl/lib/xfreopen.c
+++ b/gl/lib/xfreopen.c
@@ -21,6 +21,7 @@
 #include "error.h"
 #include "exitfail.h"
 #include "quote.h"
+#include "stdio--.h"

 #include "gettext.h"
 #define _(msgid) gettext (msgid)
diff --git a/gl/modules/xfreopen b/gl/modules/xfreopen
index 411f80b..ed4ede7 100644
--- a/gl/modules/xfreopen
+++ b/gl/modules/xfreopen
@@ -8,6 +8,7 @@ lib/xfreopen.h
 Depends-on:
 error
 exitfail
+freopen-safer
 quote

 configure.ac:
diff --git a/src/ptx.c b/src/ptx.c
index 4947a0f..701fcb3 100644
--- a/src/ptx.c
+++ b/src/ptx.c
@@ -19,7 +19,6 @@

 #include <config.h>

-#include <stdio.h>
 #include <getopt.h>
 #include <sys/types.h>
 #include "system.h"
@@ -29,6 +28,7 @@
 #include "quote.h"
 #include "quotearg.h"
 #include "regex.h"
+#include "stdio--.h"
 #include "xstrtol.h"

 /* The official name of this program (e.g., no `g' prefix).  */
diff --git a/src/shuf.c b/src/shuf.c
index 0bb11ab..71411a4 100644
--- a/src/shuf.c
+++ b/src/shuf.c
@@ -19,7 +19,6 @@

 #include <config.h>

-#include <stdio.h>
 #include <sys/types.h>
 #include "system.h"

@@ -29,6 +28,7 @@
 #include "quotearg.h"
 #include "randint.h"
 #include "randperm.h"
+#include "stdio--.h"
 #include "xstrtol.h"

 /* The official name of this program (e.g., no `g' prefix).  */
diff --git a/src/uniq.c b/src/uniq.c
index 7509bfc..ac7ecac 100644
--- a/src/uniq.c
+++ b/src/uniq.c
@@ -18,7 +18,6 @@
 
 #include <config.h>

-#include <stdio.h>
 #include <getopt.h>
 #include <sys/types.h>

@@ -29,6 +28,7 @@
 #include "hard-locale.h"
 #include "posixver.h"
 #include "quote.h"
+#include "stdio--.h"
 #include "xmemcoll.h"
 #include "xstrtol.h"
 #include "memcasecmp.h"
-- 
1.6.4.2