bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: undefined behavior in closeout, aggravated by libsigsegv


From: Eric Blake
Subject: Re: undefined behavior in closeout, aggravated by libsigsegv
Date: Sat, 18 Jul 2009 06:21:30 -0600
User-agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.22) Gecko/20090605 Thunderbird/2.0.0.22 Mnenhy/0.7.6.666

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Jim Meyering on 7/18/2009 2:42 AM:
> How about fixing gnulib's lib/error.c to do the right thing?
> Then if you teach m4/error.m4 to detect when glibc's error function
> is defective (currently always) it can use the improved replacement.

It is not defective on Linux (that is, glibc's fflush behaves reliably
even after fclose(stdout), and glibc can exploit this fact in its error()
implementation).  And according to doc/glibc-functions/error.texi, no
other platforms provide error(), so we already provide it everywhere else.
 Since the gnulib error module already has changes not currently in glibc,
we can go ahead and change error() without worrying about glibc adopting
it (although with #if _LIBC guards to make upstream adoption easier).

How about this patch?  It was enough to fix the behavior of m4 on cygwin
(it still doesn't fix the fact that libsigsegv needs to learn to play
nicer with cygwin 1.7, nor that I'm working on a newlib patch to teach
cygwin's fflush(stdout) to always be safe like it is on Linux, but those
are independent threads).  And it's lighter-weight than calling freopen in
the closeout module (not to mention that since it was error() and not
close_stdout() that was making the problematic fflush call, so fixing the
problem from within closeout only attacks a symptom, not the cause).  I'll
commit this in another day if I don't hear any comments first.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkphvkoACgkQ84KuGfSFAYBvXACcCmKvHHAFnbE86fKAOXZLzpbb
nJYAoID4HgXAYMZ7SeUzEzWpnBsCLVjN
=KoIT
-----END PGP SIGNATURE-----
>From d19ab0454ad81cd702f17864f082aefcbea6687a Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Sat, 18 Jul 2009 06:18:21 -0600
Subject: [PATCH] error: avoid undefined use of stdout

* lib/error.c (error, error_at_line): Check that fd 1 is open
before flushing stdout.  Avoids a crash on cygwin when libsigsegv
is handling faults and the close_stdout module wants to report the
detection of closed stdout as an error.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog   |    8 ++++++++
 lib/error.c |   16 +++++++++++++++-
 2 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 81e6d83..e5ba82e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2009-07-18  Eric Blake  <address@hidden>
+
+       error: avoid undefined use of stdout
+       * lib/error.c (error, error_at_line): Check that fd 1 is open
+       before flushing stdout.  Avoids a crash on cygwin when libsigsegv
+       is handling faults and the close_stdout module wants to report the
+       detection of closed stdout as an error.
+
 2009-07-17  Eric Blake  <address@hidden>

        pipe: be robust in face of closed fds
diff --git a/lib/error.c b/lib/error.c
index 3177bd5..f2d45d9 100644
--- a/lib/error.c
+++ b/lib/error.c
@@ -1,5 +1,5 @@
 /* Error handler for noninteractive utilities
-   Copyright (C) 1990-1998, 2000-2007 Free Software Foundation, Inc.
+   Copyright (C) 1990-1998, 2000-2007, 2009 Free Software Foundation, Inc.
    This file is part of the GNU C Library.

    This program is free software: you can redistribute it and/or modify
@@ -85,6 +85,8 @@ extern void __error_at_line (int status, int errnum, const 
char *file_name,

 #else /* not _LIBC */

+# include <fcntl.h>
+
 # if !HAVE_DECL_STRERROR_R && STRERROR_R_CHAR_P
 #  ifndef HAVE_DECL_STRERROR_R
 "this configure-time declaration test was not run"
@@ -236,6 +238,12 @@ error (int status, int errnum, const char *message, ...)
                   0);
 #endif

+#if !_LIBC
+  /* POSIX states that fflush (stdout) after fclose is unspecified; it
+     is safe in glibc, but not on all other platforms.  fflush (NULL)
+     is always defined, but too draconian.  */
+  if (0 <= fcntl (1, F_GETFL))
+#endif
   fflush (stdout);
 #ifdef _LIBC
   _IO_flockfile (stderr);
@@ -295,6 +303,12 @@ error_at_line (int status, int errnum, const char 
*file_name,
                   0);
 #endif

+#if !_LIBC
+  /* POSIX states that fflush (stdout) after fclose is unspecified; it
+     is safe in glibc, but not on all other platforms.  fflush (NULL)
+     is always defined, but too draconian.  */
+  if (0 <= fcntl (1, F_GETFL))
+#endif
   fflush (stdout);
 #ifdef _LIBC
   _IO_flockfile (stderr);
-- 
1.6.3.3.334.g916e1


reply via email to

[Prev in Thread] Current Thread [Next in Thread]