Re: perror bug

2011-05-19 Thread Bruno Haible
    1) The strerror_r replacement, when EXTEND_STRERROR_R is defined,
       clobbers the strerror function's buffer, which it shouldn't.

Here's a followup that tries harder to clobber the strerror function's
buffer. It still is not possible on Solaris = 9 in 64-bit mode, because
that libc does not export 'sys_nerr'.


2011-05-19  Bruno Haible  br...@clisp.org

strerror_r: Avoid clobbering the strerror buffer when possible.
* lib/strerror.c: Define _NETBSD_SOURCE. Include nl_types.h.
(sys_nerr, sys_errlist): New declarations.
(strerror_r): Be careful not to clobber the strerror buffer on NetBSD,
HP-UX, native Win32, IRIX, and 32-bit Solaris.
* m4/strerror_r.m4 (gl_PREREQ_STRERROR_R): Test whether catgets exists.

--- lib/strerror_r.c.orig   Thu May 19 20:50:20 2011
+++ lib/strerror_r.cThu May 19 20:49:54 2011
@@ -19,6 +19,9 @@
 
 #include config.h
 
+/* Enable declaration of sys_nerr and sys_errlist in errno.h on NetBSD.  */
+#define _NETBSD_SOURCE 1
+
 /* Specification.  */
 #include string.h
 
@@ -46,17 +49,45 @@
 
 #else /* (__GLIBC__ = 2 || defined __UCLIBC__ ? !HAVE___XPG_STRERROR_R : 
!HAVE_DECL_STRERROR_R) */
 
-# include glthread/lock.h
-
-/* Use strerror(), with locking.  */
+/* Use the system's strerror().  */
 # undef strerror
 
 # define USE_SYSTEM_STRERROR 1
 
+# if defined __NetBSD__ || defined __hpux || ((defined _WIN32 || defined 
__WIN32__)  !defined __CYGWIN__) || defined __sgi || (defined __sun  
!defined _LP64)
+
+/* No locking needed.  */
+
+/* Get catgets internationalization functions.  */
+#  if HAVE_CATGETS
+#   include nl_types.h
+#  endif
+
+/* Get sys_nerr, sys_errlist on HP-UX (otherwise only declared in C++ mode).
+   Get sys_nerr, sys_errlist on IRIX (otherwise only declared with _SGIAPI).  
*/
+#  if defined __hpux || defined __sgi
+extern int sys_nerr;
+extern char *sys_errlist[];
+#  endif
+
+/* Get sys_nerr on Solaris.  */
+#  if defined __sun  !defined _LP64
+extern int sys_nerr;
+#  endif
+
+/* Get sys_nerr, sys_errlist on native Windows.  */
+#  include stdlib.h
+
+# else
+
+#  include glthread/lock.h
+
 /* This lock protects the buffer returned by strerror().  We assume that
no other uses of strerror() exist in the program.  */
 gl_lock_define_initialized(static, strerror_lock)
 
+# endif
+
 #endif
 
 
@@ -476,6 +507,87 @@
 
 #else /* USE_SYSTEM_STRERROR */
 
+/* Try to do what strerror (errnum) does, but without clobbering the
+   buffer used by strerror().  */
+
+# if defined __NetBSD__ || defined __hpux || ((defined _WIN32 || defined 
__WIN32__)  !defined __CYGWIN__) /* NetBSD, HP-UX, native Win32 */
+
+/* NetBSD:sys_nerr, sys_errlist are declared through _NETBSD_SOURCE
+  and errno.h above.
+   HP-UX: sys_nerr, sys_errlist are declared explicitly above.
+   native Win32:  sys_nerr, sys_errlist are declared in stdlib.h.  */
+if (errnum = 0  errnum  sys_nerr)
+  {
+#  if HAVE_CATGETS  (defined __NetBSD__ || defined __hpux)
+int saved_errno = errno;
+#   if defined __NetBSD__
+nl_catd catd = catopen (libc, NL_CAT_LOCALE);
+const char *errmsg =
+  (catd != (nl_catd)-1
+   ? catgets (catd, 1, errnum, sys_errlist[errnum])
+   : sys_errlist[errnum]);
+#   endif
+#   if defined __hpux
+nl_catd catd = catopen (perror, NL_CAT_LOCALE);
+const char *errmsg =
+  (catd != (nl_catd)-1
+   ? catgets (catd, 1, 1 + errnum, sys_errlist[errnum])
+   : sys_errlist[errnum]);
+#   endif
+#  else
+const char *errmsg = sys_errlist[errnum];
+#  endif
+if (errmsg == NULL || *errmsg == '\0')
+  ret = EINVAL;
+else
+  {
+size_t len = strlen (errmsg);
+
+if (len  buflen)
+  {
+memcpy (buf, errmsg, len + 1);
+ret = 0;
+  }
+else
+  ret = ERANGE;
+  }
+#  if HAVE_CATGETS  (defined __NetBSD__ || defined __hpux)
+if (catd != (nl_catd)-1)
+  catclose (catd);
+errno = saved_errno;
+#  endif
+  }
+else
+  ret = EINVAL;
+
+# elif defined __sgi || (defined __sun  !defined _LP64) /* IRIX, Solaris = 
9 32-bit */
+
+/* For a valid error number, the system's strerror() function returns
+   a pointer to a not copied string, not to a buffer.  */
+if (errnum = 0  errnum  sys_nerr)
+  {
+char *errmsg = strerror (errnum);
+
+if (errmsg == NULL || *errmsg == '\0')
+  ret = EINVAL;
+else
+  {
+size_t len = strlen (errmsg);
+
+if (len  buflen)
+  {
+memcpy (buf, errmsg, len + 1);
+ret = 0;
+  }
+else
+  ret = ERANGE;
+  }
+  }
+else
+  ret = EINVAL;
+
+# else
+
 gl_lock_lock (strerror_lock);
 
 {
@@ -502,6 +614,8 @@
 
 

Re: perror bug

2011-05-19 Thread Bruno Haible
 +# elif defined __sgi || (defined __sun  !defined _LP64) /* IRIX, Solaris 
 = 9 32-bit */
 +
 +    /* For a valid error number, the system's strerror() function returns
 +       a pointer to a not copied string, not to a buffer.  */

Just for reference: How to find out whether a closed-source strerror()
function uses a buffer or not. Run this program and observe whether
all three addresses are the same or not.

===
#include errno.h
#include stdio.h
#include string.h

int main ()
{
  const char *msg1;
  const char *msg2;
  const char *msg3;

  msg1 = strerror (ENOENT);
  printf (msg1 before: %p %s\n, msg1, msg1);
  msg2 = strerror (ENOTDIR);
  printf (msg2 before: %p %s\n, msg2, msg2);
  msg3 = strerror (0);
  printf (msg3 before: %p %s\n, msg3, msg3);

  strerror (ENOENT);
  strerror (ENOTDIR);
  strerror (0);

  printf (msg1 after:  %s\n, msg1);
  printf (msg2 after:  %s\n, msg2);
  printf (msg3 after:  %s\n, msg3);

  return 0;
}
===
-- 
In memoriam Anne Boleyn http://en.wikipedia.org/wiki/Anne_Boleyn



Re: perror bug

2011-05-19 Thread Bruno Haible
On 2011-14-03 I wrote in
http://lists.gnu.org/archive/html/bug-gnulib/2011-03/msg00117.html:
   2) The perror replacement uses strerror, thus clobbering the strerror
  buffer.
   3) On Cygwin, perror clobbers the strerror buffer.
 
 The fix for 2) should be to change lib/perror.c to call strerror_r.
 The fix for 3) should be to change m4/perror.m4 to enable the replacement
 on Cygwin.

Here comes part 2. Eric, I leave part 3 to you.


2011-05-19  Bruno Haible  br...@clisp.org

perror: Avoid clobbering the strerror buffer when possible.
* lib/strerror-impl.h: New file, extracted from lib/strerror.c.
* lib/strerror.c: Include it.
* modules/strerror (Files): Add lib/strerror-impl.h.
* lib/perror.c: Include stdlib.h, intprops.h, verify.h.
(my_strerror): New function, defined through lib/strerror-impl.h.
(perror): Use it instead of strerror.
* modules/perror (Files): Add lib/strerror-impl.h.
(Depends-on): Remove strerror. Add intprops, verify, strerror_r-posix.

= lib/strerror-impl.h =
/* strerror-impl.h --- Implementation of POSIX compatible strerror() function.

   Copyright (C) 2007-2011 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/.  */

#ifdef STATIC
STATIC
#endif
char *
strerror (int n)
{
  static char buf[256];

  int ret = strerror_r (n, buf, sizeof (buf));

  if (ret == 0)
return buf;

  if (ret == ERANGE)
/* If this happens, increase the size of buf.  */
abort ();

  {
static char const fmt[] = Unknown error (%d);
verify (sizeof (buf) = sizeof (fmt) + INT_STRLEN_BOUND (n));
sprintf (buf, fmt, n);
return buf;
  }
}
===
--- lib/perror.c.orig   Thu May 19 21:44:53 2011
+++ lib/perror.cThu May 19 21:40:43 2011
@@ -21,12 +21,26 @@
 #include stdio.h
 
 #include errno.h
+#include stdlib.h
 #include string.h
 
+#include intprops.h
+#include verify.h
+
+/* Use the system functions, not the gnulib overrides in this file.  */
+#undef sprintf
+
+/* my_strerror (errnum) is equivalent to strerror (errnum).
+   But it uses its own buffer, not the one from strerror().  */
+#define STATIC static
+#undef strerror
+#define strerror my_strerror
+#include strerror-impl.h
+
 void
 perror (const char *string)
 {
-  const char *errno_description = strerror (errno);
+  const char *errno_description = my_strerror (errno);
 
   if (string != NULL  *string != '\0')
 fprintf (stderr, %s: %s\n, string, errno_description);
--- lib/strerror.c.orig Thu May 19 21:44:53 2011
+++ lib/strerror.c  Thu May 19 21:22:27 2011
@@ -32,26 +32,6 @@
 /* Use the system functions, not the gnulib overrides in this file.  */
 # undef sprintf
 
-char *
-strerror (int n)
-{
-  static char buf[256];
-
-  int ret = strerror_r (n, buf, sizeof (buf));
-
-  if (ret == 0)
-return buf;
-
-  if (ret == ERANGE)
-/* If this happens, increase the size of buf.  */
-abort ();
-
-  {
-static char const fmt[] = Unknown error (%d);
-verify (sizeof (buf) = sizeof (fmt) + INT_STRLEN_BOUND (n));
-sprintf (buf, fmt, n);
-return buf;
-  }
-}
+# include strerror-impl.h
 
 #endif
--- modules/perror.orig Thu May 19 21:44:53 2011
+++ modules/perror  Thu May 19 21:23:39 2011
@@ -3,12 +3,15 @@
 
 Files:
 lib/perror.c
+lib/strerror-impl.h
 m4/perror.m4
 
 Depends-on:
 stdio
-errno   [test $REPLACE_PERROR = 1]
-strerror[test $REPLACE_PERROR = 1]
+errno[test $REPLACE_PERROR = 1]
+intprops [test $REPLACE_PERROR = 1]
+verify   [test $REPLACE_PERROR = 1]
+strerror_r-posix [test $REPLACE_PERROR = 1]
 
 configure.ac:
 gl_FUNC_PERROR
--- modules/strerror.orig   Thu May 19 21:44:53 2011
+++ modules/strerrorThu May 19 21:19:22 2011
@@ -3,6 +3,7 @@
 
 Files:
 lib/strerror.c
+lib/strerror-impl.h
 m4/strerror.m4
 
 Depends-on:
-- 
In memoriam Anne Boleyn http://en.wikipedia.org/wiki/Anne_Boleyn



Re: perror bug

2011-05-18 Thread Bruno Haible
On 2011-14-03 I wrote in
http://lists.gnu.org/archive/html/bug-gnulib/2011-03/msg00117.html:
 With gnulib, there are three problems
 in toto:
   1) The strerror_r replacement, when EXTEND_STRERROR_R is defined,
      clobbers the strerror function's buffer, which it shouldn't.
   2) The perror replacement uses strerror, thus clobbering the strerror
      buffer.
   3) On Cygwin, perror clobbers the strerror buffer.
 
 The fix for 1) should be to move most of lib/strerror.c to lib/strerror_r.c.
 The fix for 2) should be to change lib/perror.c to call strerror_r.
 The fix for 3) should be to change m4/perror.m4 to enable the replacement
 on Cygwin.

Here comes the fix for 1). Tested on

  glibc 2.8
  glibc 2.8 with -D_POSIX_C_SOURCE=200112L
  MacOS X 10.5
  OpenBSD 4.4
  AIX 5.1
  AIX 6.1
  HP-UX 11.23
  HP-UX 11.31
  IRIX 6.5
  OSF/1 5.1
  Solaris 9
  Solaris 10
  mingw
  Cygwin 1.5.x


2011-05-18  Bruno Haible  br...@clisp.org

strerror_r: Avoid clobbering the strerror buffer when possible.
* lib/strerror_r.c (strerror_r): Merge the three implementations.
Handle gnulib defined errno values here. When strerror() returns NULL
or an empty string, return EINVAL.
* lib/strerror.c (strerror): Always call strerror_r. Don't handle
gnulib defined errno values here.
* modules/strerror (Depends-on): Add verify, strerror_r-posix.

*** lib/strerror.c.orig Thu May 19 05:17:36 2011
--- lib/strerror.c  Thu May 19 04:47:44 2011
***
*** 17,355 
  
  #include config.h
  
  #include string.h
  
  #if REPLACE_STRERROR
  
  # include errno.h
  # include stdio.h
! 
! # if GNULIB_defined_ESOCK /* native Windows platforms */
! #  if HAVE_WINSOCK2_H
! #   include winsock2.h
! #  endif
! # endif
  
  # include intprops.h
  
  /* Use the system functions, not the gnulib overrides in this file.  */
  # undef sprintf
  
- # undef strerror
- # if ! HAVE_DECL_STRERROR
- #  define strerror(n) NULL
- # endif
- 
  char *
! rpl_strerror (int n)
  {
!   char const *msg = NULL;
!   /* These error messages are taken from glibc/sysdeps/gnu/errlist.c.  */
!   switch (n)
! {
! # if GNULIB_defined_ETXTBSY
! case ETXTBSY:
!   msg = Text file busy;
!   break;
! # endif
! 
! # if GNULIB_defined_ESOCK /* native Windows platforms */
! /* EWOULDBLOCK is the same as EAGAIN.  */
! case EINPROGRESS:
!   msg = Operation now in progress;
!   break;
! case EALREADY:
!   msg = Operation already in progress;
!   break;
! case ENOTSOCK:
!   msg = Socket operation on non-socket;
!   break;
! case EDESTADDRREQ:
!   msg = Destination address required;
!   break;
! case EMSGSIZE:
!   msg = Message too long;
!   break;
! case EPROTOTYPE:
!   msg = Protocol wrong type for socket;
!   break;
! case ENOPROTOOPT:
!   msg = Protocol not available;
!   break;
! case EPROTONOSUPPORT:
!   msg = Protocol not supported;
!   break;
! case ESOCKTNOSUPPORT:
!   msg = Socket type not supported;
!   break;
! case EOPNOTSUPP:
!   msg = Operation not supported;
!   break;
! case EPFNOSUPPORT:
!   msg = Protocol family not supported;
!   break;
! case EAFNOSUPPORT:
!   msg = Address family not supported by protocol;
!   break;
! case EADDRINUSE:
!   msg = Address already in use;
!   break;
! case EADDRNOTAVAIL:
!   msg = Cannot assign requested address;
!   break;
! case ENETDOWN:
!   msg = Network is down;
!   break;
! case ENETUNREACH:
!   msg = Network is unreachable;
!   break;
! case ENETRESET:
!   msg = Network dropped connection on reset;
!   break;
! case ECONNABORTED:
!   msg = Software caused connection abort;
!   break;
! case ECONNRESET:
!   msg = Connection reset by peer;
!   break;
! case ENOBUFS:
!   msg = No buffer space available;
!   break;
! case EISCONN:
!   msg = Transport endpoint is already connected;
!   break;
! case ENOTCONN:
!   msg = Transport endpoint is not connected;
!   break;
! case ESHUTDOWN:
!   msg = Cannot send after transport endpoint shutdown;
!   break;
! case ETOOMANYREFS:
!   msg = Too many references: cannot splice;
!   break;
! case ETIMEDOUT:
!   msg = Connection timed out;
!   break;
! case ECONNREFUSED:
!   msg = Connection refused;
!   break;
! case ELOOP:
!   msg = Too many levels of symbolic links;
!   break;
! case EHOSTDOWN:
!   msg = Host is down;
!   break;
! case EHOSTUNREACH:
!   msg = No route to host;
!   break;
! case EPROCLIM:
!   msg = Too many processes;
!   break;
! case EUSERS:
!   msg = Too many users;
!   break;
! case EDQUOT:
!   msg = Disk quota exceeded;
!   break;
! case ESTALE:
!   msg = Stale NFS file handle;
!   break;
! case 

Re: perror bug

2011-03-14 Thread Bruno Haible
Eric Blake wrote on 2011-02-10:
 POSIX requires that this program have an identical first and last line:
 
 #include stdio.h
 #include string.h
 #include errno.h
 int main (void) {
   char *err = strerror(1000);
   printf (%s\n, err);
   errno = 2000;
   perror (hi);
   printf (%s\n, err);
   return 0;
 }
 
 but on cygwin 1.7.7, the perror() corrupts the buffer returned by
 strerror().  We should probably fix that in gnulib as part of our perror
 module.

Yes, but first let's see which other problems there are.

The program below tests whether strerror's buffer is read-write (i.e.
whether it is overwritten by subsequent strerror calls) and, when compiled
with -DPERROR, whether perror calls clobber the strerror buffer.

The result is:

   read-write buffer?perror reuses strerror buffer?
glibc   yes   no
OpenBSD yes   no
OSF/1   yes   no
Cygwin 1.5  yes   yes
Cygwin 1.7  yes   yes
mingw   yes   no

That's the situation without gnulib. With gnulib, there are three problems
in toto:
  1) The strerror_r replacement, when EXTEND_STRERROR_R is defined,
 clobbers the strerror function's buffer, which it shouldn't.
  2) The perror replacement uses strerror, thus clobbering the strerror
 buffer.
  3) On Cygwin, perror clobbers the strerror buffer.

The fix for 1) should be to move most of lib/strerror.c to lib/strerror_r.c.
The fix for 2) should be to change lib/perror.c to call strerror_r.
The fix for 3) should be to change m4/perror.m4 to enable the replacement
on Cygwin.

I think the three fixes should be applied in this order, bottom-up.

Bruno


 foo.c 
#include errno.h
#include stdio.h
#include string.h

int main ()
{
  const char *msg1;
  const char *msg2;
  const char *msg3;

  msg1 = strerror (ENOENT);
  printf (msg1 before: %s\n, msg1);
  msg2 = strerror (-4);
  printf (msg2 before: %s\n, msg2);
  msg3 = strerror (1729576);
  printf (msg3 before: %s\n, msg3);

  freopen (/dev/null, w, stderr);

#ifdef PERROR
  errno = EACCES; perror ();
  errno = -5; perror ();
  errno = 153272; perror ();
#else
  strerror (EACCES);
  strerror (-5);
  strerror (153272);
#endif

  printf (msg1 after:  %s\n, msg1);
  printf (msg2 after:  %s\n, msg2);
  printf (msg3 after:  %s\n, msg3);

  return 0;
}
===



Re: perror bug

2011-02-09 Thread Eric Blake
On 02/09/2011 05:48 PM, Eric Blake wrote:
 POSIX requires that this program have an identical first and last line:
 
 #include stdio.h
 #include string.h
 #include errno.h
 int main (void) {
   char *err = strerror(1000);
   printf (%s\n, err);
   errno = 2000;
   perror (hi);
   printf (%s\n, err);
   return 0;
 }
 
 but on cygwin 1.7.7, the perror() corrupts the buffer returned by
 strerror().  We should probably fix that in gnulib as part of our perror
 module.

Actually, maybe the fix to this is to make sure that we always replace
strerror(), and strcpy() the output into a secondary buffer (other libc
functions that incorrectly call strerror() will not overwrite our
secondary buffer), and make sure that the rest of gnulib uses only
strerror_r() rather than strerror() (although that means making more
modules dependent on strerror_r-posix, such as perror).

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature