]> The Tcpdump Group git mirrors - libpcap/commitdiff
Make pcap_strerror() more thread-safe. 1290/head
authorDenis Ovsienko <[email protected]>
Tue, 5 Mar 2024 17:52:41 +0000 (17:52 +0000)
committerDenis Ovsienko <[email protected]>
Fri, 15 Mar 2024 16:43:36 +0000 (16:43 +0000)
Drop the checks for strerror() and the code block in pcap_strerror()
that resorts to sys_nerr and sys_errlist[] because HAVE_STRERROR is
always defined because every supported OS implements strerror(), which
is in POSIX.1-2001.

Move the GNU- and POSIX-specific strerror_r() code blocks from
pcapint_vfmt_errmsg_for_errno() to pcap_strerror() to make the latter
thread-safe when strerror_r() is available.  If POSIX strerror_r()
fails, try to fill the buffer with a useful string even if the failure
cannot be interpreted.

Update comments to explain the flow of conditionals with regard to
various OSes.

CHANGES
CMakeLists.txt
cmakeconfig.h.in
configure.ac
fmtutils.c
pcap.c
pcap_strerror.3pcap

diff --git a/CHANGES b/CHANGES
index 75c2d99dfb064ab7e4fca0f70a40495e36534741..b846c624b82f0a1f07973b363604b36d71f1a7b1 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -6,6 +6,7 @@ DayOfTheWeek, Month DD, YYYY / The Tcpdump Group
       Remove an always-false pointer test from snf_read().
       Clean up DECnet address handling.
       struct pcap: Update buffer type from "void *" to "u_char *".
+      Make pcap_strerror() more thread-safe.
     Link-layer types:
       Add LINKTYPE_ETW/DLT_ETW.
       Add LINKTYPE_NETANALYZER_NG/DLT_NETANALYZER_NG (pull request
index e2d7b88eb679a60122a9d2b850dd846a714d0d4b..708d987fd60854e99f6d97d4a59c2143c2889dd7 100644 (file)
@@ -710,7 +710,6 @@ main(void)
 #
 # Now check for various system functions.
 #
-check_function_exists(strerror HAVE_STRERROR)
 check_function_exists(strerror_r HAVE_STRERROR_R)
 if(HAVE_STRERROR_R)
     #
index f5b6012d712eb9aac6bbf835eb8ebc6e76de410d..32b1aa8087abfab52685df5dee55833f9c017cfa 100644 (file)
 /* Define to 1 if you have the <stdlib.h> header file. */
 #cmakedefine HAVE_STDLIB_H 1
 
-/* Define to 1 if you have the `strerror' function. */
-#cmakedefine HAVE_STRERROR 1
-
 /* Define to 1 if you have the <strings.h> header file. */
 #cmakedefine HAVE_STRINGS_H 1
 
index 6ac3b4d3561db856f382931382bd8dcf9401bee7..312fdf13a3676c620f591818e6c35beee417f33a 100644 (file)
@@ -161,7 +161,6 @@ haiku*)
        ;;
 esac
 
-AC_CHECK_FUNCS(strerror)
 AC_CHECK_FUNC(strerror_r,
     [
        #
index 8658bc3c1168ed49c435e7022b9530eeaf0eedcb..3a73be6ddbd2282eb94bd9ce3dc1296a74e45790 100644 (file)
@@ -334,43 +334,10 @@ pcapint_vfmt_errmsg_for_errno(char *errbuf, size_t errbuflen, int errnum,
         */
        if (!use_utf_8)
                utf_8_to_acp_truncated(errbuf);
-#elif defined(HAVE_GNU_STRERROR_R)
-       /*
-        * We have a GNU-style strerror_r(), which is *not* guaranteed to
-        * do anything to the buffer handed to it, and which returns a
-        * pointer to the error string, which may or may not be in
-        * the buffer.
-        *
-        * It is, however, guaranteed to succeed.
-        */
-       char strerror_buf[PCAP_ERRBUF_SIZE];
-       char *errstring = strerror_r(errnum, strerror_buf, PCAP_ERRBUF_SIZE);
-       snprintf(p, errbuflen_remaining, "%s", errstring);
-#elif defined(HAVE_POSIX_STRERROR_R)
-       /*
-        * We have a POSIX-style strerror_r(), which is guaranteed to fill
-        * in the buffer, but is not guaranteed to succeed.
-        */
-       int err = strerror_r(errnum, p, errbuflen_remaining);
-       if (err == EINVAL) {
-               /*
-                * UNIX 03 says this isn't guaranteed to produce a
-                * fallback error message.
-                */
-               snprintf(p, errbuflen_remaining, "Unknown error: %d",
-                   errnum);
-       } else if (err == ERANGE) {
-               /*
-                * UNIX 03 says this isn't guaranteed to produce a
-                * fallback error message.
-                */
-               snprintf(p, errbuflen_remaining,
-                   "Message for error %d is too long", errnum);
-       }
 #else
        /*
-        * We have neither _wcserror_s() nor strerror_r(), so we're
-        * stuck with using pcap_strerror().
+        * Either Windows without _wcserror_s() or not Windows.  Let pcap_strerror()
+        * solve the non-UTF-16 part of this problem space.
         */
        snprintf(p, errbuflen_remaining, "%s", pcap_strerror(errnum));
 #endif
diff --git a/pcap.c b/pcap.c
index 6fa39d433cb4e6d2e33b17462bb7cafdcd135ecc..4d8776cbd9bbcc652b2acd3f0fddba0767109d28 100644 (file)
--- a/pcap.c
+++ b/pcap.c
@@ -3745,12 +3745,16 @@ pcap_statustostr(int errnum)
 }
 
 /*
- * Not all systems have strerror().
+ * A long time ago the purpose of this function was to hide the difference
+ * between those Unix-like OSes that implemented strerror() and those that
+ * didn't.  All the currently supported OSes implement strerror(), which is in
+ * POSIX.1-2001, uniformly and that particular problem no longer exists.  But
+ * now they implement a few incompatible thread-safe variants of strerror(),
+ * and hiding that difference is the current purpose of this function.
  */
 const char *
 pcap_strerror(int errnum)
 {
-#ifdef HAVE_STRERROR
 #ifdef _WIN32
        static thread_local char errbuf[PCAP_ERRBUF_SIZE];
        errno_t err = strerror_s(errbuf, PCAP_ERRBUF_SIZE, errnum);
@@ -3758,19 +3762,76 @@ pcap_strerror(int errnum)
        if (err != 0) /* err = 0 if successful */
                pcapint_strlcpy(errbuf, "strerror_s() error", PCAP_ERRBUF_SIZE);
        return (errbuf);
+#elif defined(HAVE_GNU_STRERROR_R)
+       /*
+        * We have a GNU-style strerror_r(), which is *not* guaranteed to
+        * do anything to the buffer handed to it, and which returns a
+        * pointer to the error string, which may or may not be in
+        * the buffer.
+        *
+        * It is, however, guaranteed to succeed.
+        *
+        * At the time of this writing this applies to the following cases,
+        * each of which allows to use either the GNU implementation or the
+        * POSIX implementation, and this source tree defines _GNU_SOURCE to
+        * use the GNU implementation:
+        * - Hurd
+        * - Linux with GNU libc
+        * - Linux with uClibc-ng
+        */
+       static thread_local char errbuf[PCAP_ERRBUF_SIZE];
+       return strerror_r(errnum, errbuf, PCAP_ERRBUF_SIZE);
+#elif defined(HAVE_POSIX_STRERROR_R)
+       /*
+        * We have a POSIX-style strerror_r(), which is guaranteed to fill
+        * in the buffer, but is not guaranteed to succeed.
+        *
+        * At the time of this writing this applies to the following cases:
+        * - AIX 7
+        * - FreeBSD
+        * - Haiku
+        * - HP-UX 11
+        * - illumos
+        * - Linux with musl libc
+        * - macOS
+        * - NetBSD
+        * - OpenBSD
+        * - Solaris 10 & 11
+        */
+       static thread_local char errbuf[PCAP_ERRBUF_SIZE];
+       int err = strerror_r(errnum, errbuf, PCAP_ERRBUF_SIZE);
+       switch (err) {
+       case EINVAL:
+               /*
+                * UNIX 03 says this isn't guaranteed to produce a
+                * fallback error message.
+                */
+               snprintf(errbuf, PCAP_ERRBUF_SIZE,
+                        "Unknown error: %d", errnum);
+               break;
+       case ERANGE:
+               /*
+                * UNIX 03 says this isn't guaranteed to produce a
+                * fallback error message.
+                */
+               snprintf(errbuf, PCAP_ERRBUF_SIZE,
+                        "Message for error %d is too long", errnum);
+               break;
+       default:
+               snprintf(errbuf, PCAP_ERRBUF_SIZE,
+                        "strerror_r(%d, ...) unexpectedly returned %d",
+                        errnum, err);
+       }
+       return errbuf;
 #else
+       /*
+        * At the time of this writing every supported OS implements strerror()
+        * and at least one thread-safe variant thereof, so this is a very
+        * unlikely last-resort branch.  Particular implementations of strerror()
+        * may be thread-safe, but this is neither required nor guaranteed.
+        */
        return (strerror(errnum));
 #endif /* _WIN32 */
-#else
-       extern int sys_nerr;
-       extern const char *const sys_errlist[];
-       static thread_local char errbuf[PCAP_ERRBUF_SIZE];
-
-       if ((unsigned int)errnum < sys_nerr)
-               return ((char *)sys_errlist[errnum]);
-       (void)snprintf(errbuf, sizeof errbuf, "Unknown error: %d", errnum);
-       return (errbuf);
-#endif
 }
 
 int
index dedfa40697cb9715e0c182fda7be14b9faa18112..769f4e1347ca3a6c1a5f13c248ee8189f3ec5789 100644 (file)
@@ -17,7 +17,7 @@
 .\" WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED WARRANTIES OF
 .\" MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE.
 .\"
-.TH PCAP_STRERROR 3PCAP "3 January 2014"
+.TH PCAP_STRERROR 3PCAP "12 March 2024"
 .SH NAME
 pcap_strerror \- convert an errno value to a string
 .SH SYNOPSIS
@@ -31,10 +31,14 @@ const char *pcap_strerror(int error);
 .ft
 .fi
 .SH DESCRIPTION
-.BR pcap_strerror ()
-is provided in case
-.BR strerror (3)
-isn't available.  It returns an error message string corresponding to
+This function returns an error message string corresponding to
 .IR error .
+It uses either
+.BR strerror (3)
+or its thread-safe variant if one is available, which currently is the case in
+every supported OS.
+.SH BACKWARD COMPATIBILITY
+This function was not thread-safe in libpcap before 1.8.1 on Windows and
+in libpcap before 1.11.0 on all other OSes.
 .SH SEE ALSO
 .BR pcap (3PCAP)