util: Replace non-threadsafe strerror #24933

pull laanwj wants to merge 5 commits into bitcoin:master from laanwj:2022-04-strerror-threadsafe changing 9 files +69 −20
  1. laanwj commented at 2:26 PM on April 20, 2022: member

    Some uses of non-threadsafe strerror have snuck into the code since they were removed in #4152. Add a wrapper SysErrorString for thread-safe strerror alternatives (with code from NetworkErrorString) and replace all uses of strerror with this.

    Edit: I've also added a commit that refactors the code so that buf[] is never read at all if the function fails, making some fragile-looking code unnecessary.

    Edit2: from the linux manpage:

    ATTRIBUTES
       For an explanation of the terms used in this section, see attributes(7).

       ┌───────────────────┬───────────────┬─────────────────────────┐
       │Interface          │ Attribute     │ Value                   │
       ├───────────────────┼───────────────┼─────────────────────────┤
       │strerror()         │ Thread safety │ MT-Unsafe race:strerror │
       ├───────────────────┼───────────────┼─────────────────────────┤
…
       ├───────────────────┼───────────────┼─────────────────────────┤
       │strerror_r(),      │ Thread safety │ MT-Safe                 │
       │strerror_l()       │               │                         │
       └───────────────────┴───────────────┴─────────────────────────┘

    As the function can be called from any thread at any time, using a non-thread-safe function is unacceptable.

  2. laanwj added the label Utils/log/libs on Apr 20, 2022
  3. in src/util/syserror.cpp:23 in b24cd432f4 outdated 
      16 | +    char buf[256];
  17 | +    buf[0] = 0;
  18 | +    /* Too bad there are two incompatible implementations of the
  19 | +     * thread-safe strerror. */
  20 | +    const char *s;
  21 | +#ifdef STRERROR_R_CHAR_P /* GNU variant can return a pointer outside the passed buffer */
    laanwj commented at 2:35 PM on April 20, 2022:

    Thinking of it, shouldn't we be using errno_t strerror_s( char *buf, rsize_t bufsz, errno_t errnum ); from C11/C++17 here?

    Edit: oh, is that windows-only? Gah…

  4. laanwj force-pushed on Apr 20, 2022
  5. laanwj force-pushed on Apr 20, 2022
  6. laanwj force-pushed on Apr 20, 2022
  7. laanwj force-pushed on Apr 20, 2022
  8. laanwj force-pushed on Apr 20, 2022
  9. laanwj commented at 11:43 AM on April 21, 2022: member

    To test on your favorite platform:

    $ c++ error-test.cpp util/syserror.cpp -o error-test -DHAVE_CONFIG_H -I.

    (make sure to define -DHAVE_CONFIG_H, it 's important that the right implementation is chosen for your platform)

    error-test.cpp:

    #include <util/syserror.h>

#include <iostream>

int main()
{
    for (int i = -200; i < 200; ++i) {
        std::cout << SysErrorString(i) << std::endl;
    }
}
  10. jonatack commented at 12:04 PM on April 21, 2022: member

    Concept ACK on removing std::strerror, according to https://en.cppreference.com/w/cpp/string/byte/strerror it "is not required to be thread-safe. Implementations may be returning different pointers to static read-only string literals or may be returning the same pointer over and over, pointing at a static buffer in which strerror places the string" and this pull removes the remaining ones apart from three lines in src/leveldb/util/.

  11. jonatack commented at 12:13 PM on April 21, 2022: member

    <details><summary>build options</summary><p>

    Build Options:
  with external callbacks = no
  with benchmarks         = no
  with tests              = yes
  with coverage           = no
  with examples           = no
  module ecdh             = no
  module recovery         = yes
  module extrakeys        = yes
  module schnorrsig       = yes

  asm                     = x86_64
  ecmult window size      = 15
  ecmult gen prec. bits   = 4

  valgrind                = yes
  CC                      = clang
  CPPFLAGS                =  -DDEBUG_LOCKORDER -DDEBUG_LOCKCONTENTION -DHAVE_CONFIG_H
  SECP_CFLAGS             = -O2  -std=c89 -pedantic -Wno-long-long -Wnested-externs -Wshadow -Wstrict-prototypes -Wundef -Wno-overlength-strings -Wall -Wno-unused-function -Wextra -Wcast-align -Wconditional-uninitialized -fvisibility=hidden 
  CFLAGS                  = -g -O2
  LDFLAGS                 = 

Options used to compile and link:
  external signer = yes
  multiprocess    = no
  with experimental syscall sandbox support = yes
  with libs       = yes
  with wallet     = yes
    with sqlite   = yes
    with bdb      = yes
  with gui / qt   = yes
    with qr       = yes
  with zmq        = yes
  with test       = yes
  with fuzz binary = yes
  with bench      = yes
  with upnp       = yes
  with natpmp     = yes
  use asm         = yes
  USDT tracing    = yes
  sanitizers      = 
  debug enabled   = yes
  gprof enabled   = no
  werror          = yes
  LTO             = no

  target os       = linux-gnu
  build os        = linux-gnu

  CC              = /usr/bin/ccache clang
  CFLAGS          = -pthread -g -O2
  CPPFLAGS        =  -DDEBUG -DDEBUG_LOCKORDER -DABORT_ON_FAILED_ASSUME -fmacro-prefix-map=$(abs_top_srcdir)=.   -DHAVE_BUILD_INFO -DPROVIDE_FUZZ_MAIN_FUNCTION -DDEBUG_LOCKORDER -DDEBUG_LOCKCONTENTION -DHAVE_CONFIG_H
  CXX             = /usr/bin/ccache clang++ -std=c++20
  CXXFLAGS        =   -O0 -g3 -ftrapv -fdebug-prefix-map=$(abs_top_srcdir)=.  -Wstack-protector -fstack-protector-all -fcf-protection=full -fstack-clash-protection  -Wall -Wextra -Wgnu -Wformat -Wformat-security -Wvla -Wshadow-field -Wthread-safety -Wrange-loop-analysis -Wredundant-decls -Wunused-member-function -Wdate-time -Wconditional-uninitialized -Woverloaded-virtual -Wsuggest-override -Wunreachable-code-loop-increment -Wimplicit-fallthrough -Wdocumentation  -Wno-unused-parameter -Wno-self-assign -Werror   
  LDFLAGS         =  -lpthread  -Wl,-z,relro -Wl,-z,now -Wl,-z,separate-code -pie   
  ARFLAGS         = cr

    </p></details>

    <details><summary>error_test output (debian testing)</summary><p>

    Unknown error -200 (-200)
Unknown error -199 (-199)
Unknown error -198 (-198)
Unknown error -197 (-197)
Unknown error -196 (-196)
Unknown error -195 (-195)
Unknown error -194 (-194)
Unknown error -193 (-193)
Unknown error -192 (-192)
Unknown error -191 (-191)
Unknown error -190 (-190)
Unknown error -189 (-189)
Unknown error -188 (-188)
Unknown error -187 (-187)
Unknown error -186 (-186)
Unknown error -185 (-185)
Unknown error -184 (-184)
Unknown error -183 (-183)
Unknown error -182 (-182)
Unknown error -181 (-181)
Unknown error -180 (-180)
Unknown error -179 (-179)
Unknown error -178 (-178)
Unknown error -177 (-177)
Unknown error -176 (-176)
Unknown error -175 (-175)
Unknown error -174 (-174)
Unknown error -173 (-173)
Unknown error -172 (-172)
Unknown error -171 (-171)
Unknown error -170 (-170)
Unknown error -169 (-169)
Unknown error -168 (-168)
Unknown error -167 (-167)
Unknown error -166 (-166)
Unknown error -165 (-165)
Unknown error -164 (-164)
Unknown error -163 (-163)
Unknown error -162 (-162)
Unknown error -161 (-161)
Unknown error -160 (-160)
Unknown error -159 (-159)
Unknown error -158 (-158)
Unknown error -157 (-157)
Unknown error -156 (-156)
Unknown error -155 (-155)
Unknown error -154 (-154)
Unknown error -153 (-153)
Unknown error -152 (-152)
Unknown error -151 (-151)
Unknown error -150 (-150)
Unknown error -149 (-149)
Unknown error -148 (-148)
Unknown error -147 (-147)
Unknown error -146 (-146)
Unknown error -145 (-145)
Unknown error -144 (-144)
Unknown error -143 (-143)
Unknown error -142 (-142)
Unknown error -141 (-141)
Unknown error -140 (-140)
Unknown error -139 (-139)
Unknown error -138 (-138)
Unknown error -137 (-137)
Unknown error -136 (-136)
Unknown error -135 (-135)
Unknown error -134 (-134)
Unknown error -133 (-133)
Unknown error -132 (-132)
Unknown error -131 (-131)
Unknown error -130 (-130)
Unknown error -129 (-129)
Unknown error -128 (-128)
Unknown error -127 (-127)
Unknown error -126 (-126)
Unknown error -125 (-125)
Unknown error -124 (-124)
Unknown error -123 (-123)
Unknown error -122 (-122)
Unknown error -121 (-121)
Unknown error -120 (-120)
Unknown error -119 (-119)
Unknown error -118 (-118)
Unknown error -117 (-117)
Unknown error -116 (-116)
Unknown error -115 (-115)
Unknown error -114 (-114)
Unknown error -113 (-113)
Unknown error -112 (-112)
Unknown error -111 (-111)
Unknown error -110 (-110)
Unknown error -109 (-109)
Unknown error -108 (-108)
Unknown error -107 (-107)
Unknown error -106 (-106)
Unknown error -105 (-105)
Unknown error -104 (-104)
Unknown error -103 (-103)
Unknown error -102 (-102)
Unknown error -101 (-101)
Unknown error -100 (-100)
Unknown error -99 (-99)
Unknown error -98 (-98)
Unknown error -97 (-97)
Unknown error -96 (-96)
Unknown error -95 (-95)
Unknown error -94 (-94)
Unknown error -93 (-93)
Unknown error -92 (-92)
Unknown error -91 (-91)
Unknown error -90 (-90)
Unknown error -89 (-89)
Unknown error -88 (-88)
Unknown error -87 (-87)
Unknown error -86 (-86)
Unknown error -85 (-85)
Unknown error -84 (-84)
Unknown error -83 (-83)
Unknown error -82 (-82)
Unknown error -81 (-81)
Unknown error -80 (-80)
Unknown error -79 (-79)
Unknown error -78 (-78)
Unknown error -77 (-77)
Unknown error -76 (-76)
Unknown error -75 (-75)
Unknown error -74 (-74)
Unknown error -73 (-73)
Unknown error -72 (-72)
Unknown error -71 (-71)
Unknown error -70 (-70)
Unknown error -69 (-69)
Unknown error -68 (-68)
Unknown error -67 (-67)
Unknown error -66 (-66)
Unknown error -65 (-65)
Unknown error -64 (-64)
Unknown error -63 (-63)
Unknown error -62 (-62)
Unknown error -61 (-61)
Unknown error -60 (-60)
Unknown error -59 (-59)
Unknown error -58 (-58)
Unknown error -57 (-57)
Unknown error -56 (-56)
Unknown error -55 (-55)
Unknown error -54 (-54)
Unknown error -53 (-53)
Unknown error -52 (-52)
Unknown error -51 (-51)
Unknown error -50 (-50)
Unknown error -49 (-49)
Unknown error -48 (-48)
Unknown error -47 (-47)
Unknown error -46 (-46)
Unknown error -45 (-45)
Unknown error -44 (-44)
Unknown error -43 (-43)
Unknown error -42 (-42)
Unknown error -41 (-41)
Unknown error -40 (-40)
Unknown error -39 (-39)
Unknown error -38 (-38)
Unknown error -37 (-37)
Unknown error -36 (-36)
Unknown error -35 (-35)
Unknown error -34 (-34)
Unknown error -33 (-33)
Unknown error -32 (-32)
Unknown error -31 (-31)
Unknown error -30 (-30)
Unknown error -29 (-29)
Unknown error -28 (-28)
Unknown error -27 (-27)
Unknown error -26 (-26)
Unknown error -25 (-25)
Unknown error -24 (-24)
Unknown error -23 (-23)
Unknown error -22 (-22)
Unknown error -21 (-21)
Unknown error -20 (-20)
Unknown error -19 (-19)
Unknown error -18 (-18)
Unknown error -17 (-17)
Unknown error -16 (-16)
Unknown error -15 (-15)
Unknown error -14 (-14)
Unknown error -13 (-13)
Unknown error -12 (-12)
Unknown error -11 (-11)
Unknown error -10 (-10)
Unknown error -9 (-9)
Unknown error -8 (-8)
Unknown error -7 (-7)
Unknown error -6 (-6)
Unknown error -5 (-5)
Unknown error -4 (-4)
Unknown error -3 (-3)
Unknown error -2 (-2)
Unknown error -1 (-1)
Success (0)
Operation not permitted (1)
No such file or directory (2)
No such process (3)
Interrupted system call (4)
Input/output error (5)
No such device or address (6)
Argument list too long (7)
Exec format error (8)
Bad file descriptor (9)
No child processes (10)
Resource temporarily unavailable (11)
Cannot allocate memory (12)
Permission denied (13)
Bad address (14)
Block device required (15)
Device or resource busy (16)
File exists (17)
Invalid cross-device link (18)
No such device (19)
Not a directory (20)
Is a directory (21)
Invalid argument (22)
Too many open files in system (23)
Too many open files (24)
Inappropriate ioctl for device (25)
Text file busy (26)
File too large (27)
No space left on device (28)
Illegal seek (29)
Read-only file system (30)
Too many links (31)
Broken pipe (32)
Numerical argument out of domain (33)
Numerical result out of range (34)
Resource deadlock avoided (35)
File name too long (36)
No locks available (37)
Function not implemented (38)
Directory not empty (39)
Too many levels of symbolic links (40)
Unknown error 41 (41)
No message of desired type (42)
Identifier removed (43)
Channel number out of range (44)
Level 2 not synchronized (45)
Level 3 halted (46)
Level 3 reset (47)
Link number out of range (48)
Protocol driver not attached (49)
No CSI structure available (50)
Level 2 halted (51)
Invalid exchange (52)
Invalid request descriptor (53)
Exchange full (54)
No anode (55)
Invalid request code (56)
Invalid slot (57)
Unknown error 58 (58)
Bad font file format (59)
Device not a stream (60)
No data available (61)
Timer expired (62)
Out of streams resources (63)
Machine is not on the network (64)
Package not installed (65)
Object is remote (66)
Link has been severed (67)
Advertise error (68)
Srmount error (69)
Communication error on send (70)
Protocol error (71)
Multihop attempted (72)
RFS specific error (73)
Bad message (74)
Value too large for defined data type (75)
Name not unique on network (76)
File descriptor in bad state (77)
Remote address changed (78)
Can not access a needed shared library (79)
Accessing a corrupted shared library (80)
.lib section in a.out corrupted (81)
Attempting to link in too many shared libraries (82)
Cannot exec a shared library directly (83)
Invalid or incomplete multibyte or wide character (84)
Interrupted system call should be restarted (85)
Streams pipe error (86)
Too many users (87)
Socket operation on non-socket (88)
Destination address required (89)
Message too long (90)
Protocol wrong type for socket (91)
Protocol not available (92)
Protocol not supported (93)
Socket type not supported (94)
Operation not supported (95)
Protocol family not supported (96)
Address family not supported by protocol (97)
Address already in use (98)
Cannot assign requested address (99)
Network is down (100)
Network is unreachable (101)
Network dropped connection on reset (102)
Software caused connection abort (103)
Connection reset by peer (104)
No buffer space available (105)
Transport endpoint is already connected (106)
Transport endpoint is not connected (107)
Cannot send after transport endpoint shutdown (108)
Too many references: cannot splice (109)
Connection timed out (110)
Connection refused (111)
Host is down (112)
No route to host (113)
Operation already in progress (114)
Operation now in progress (115)
Stale file handle (116)
Structure needs cleaning (117)
Not a XENIX named type file (118)
No XENIX semaphores available (119)
Is a named type file (120)
Remote I/O error (121)
Disk quota exceeded (122)
No medium found (123)
Wrong medium type (124)
Operation canceled (125)
Required key not available (126)
Key has expired (127)
Key has been revoked (128)
Key was rejected by service (129)
Owner died (130)
State not recoverable (131)
Operation not possible due to RF-kill (132)
Memory page has hardware error (133)
Unknown error 134 (134)
Unknown error 135 (135)
Unknown error 136 (136)
Unknown error 137 (137)
Unknown error 138 (138)
Unknown error 139 (139)
Unknown error 140 (140)
Unknown error 141 (141)
Unknown error 142 (142)
Unknown error 143 (143)
Unknown error 144 (144)
Unknown error 145 (145)
Unknown error 146 (146)
Unknown error 147 (147)
Unknown error 148 (148)
Unknown error 149 (149)
Unknown error 150 (150)
Unknown error 151 (151)
Unknown error 152 (152)
Unknown error 153 (153)
Unknown error 154 (154)
Unknown error 155 (155)
Unknown error 156 (156)
Unknown error 157 (157)
Unknown error 158 (158)
Unknown error 159 (159)
Unknown error 160 (160)
Unknown error 161 (161)
Unknown error 162 (162)
Unknown error 163 (163)
Unknown error 164 (164)
Unknown error 165 (165)
Unknown error 166 (166)
Unknown error 167 (167)
Unknown error 168 (168)
Unknown error 169 (169)
Unknown error 170 (170)
Unknown error 171 (171)
Unknown error 172 (172)
Unknown error 173 (173)
Unknown error 174 (174)
Unknown error 175 (175)
Unknown error 176 (176)
Unknown error 177 (177)
Unknown error 178 (178)
Unknown error 179 (179)
Unknown error 180 (180)
Unknown error 181 (181)
Unknown error 182 (182)
Unknown error 183 (183)
Unknown error 184 (184)
Unknown error 185 (185)
Unknown error 186 (186)
Unknown error 187 (187)
Unknown error 188 (188)
Unknown error 189 (189)
Unknown error 190 (190)
Unknown error 191 (191)
Unknown error 192 (192)
Unknown error 193 (193)
Unknown error 194 (194)
Unknown error 195 (195)
Unknown error 196 (196)
Unknown error 197 (197)
Unknown error 198 (198)
Unknown error 199 (199)

    </p></details>

  12. in src/init.cpp:68 in 9d5fa88167 outdated 
      64 | @@ -65,6 +65,7 @@
  65 |  #include <util/string.h>
  66 |  #include <util/syscall_sandbox.h>
  67 |  #include <util/system.h>
  68 | +#include <util/syserror.h>
    jonatack commented at 12:23 PM on April 21, 2022:

    eb68c15a21856f1edf nit, sort if you retouch

    -#include <util/system.h>
 #include <util/syserror.h>
+#include <util/system.h>
    laanwj commented at 4:41 PM on April 21, 2022:

    Thanks, fixed

  13. in src/util/syserror.cpp:16 in 9d5fa88167 outdated 
      11 | +
  12 | +#include <cstring>
  13 | +
  14 | +std::string SysErrorString(int err)
  15 | +{
  16 | +    char buf[256];
    jonatack commented at 12:59 PM on April 21, 2022:

    Unsure, but just mentioning that per man strerror: "The GNU C Library uses a buffer of 1024 characters for strerror(). This buffer size therefore should be sufficient to avoid an ERANGE error when calling strerror_r()."

    laanwj commented at 4:27 PM on April 21, 2022:

    Huh. Yeah, would be better to bump it. Though personally I'd say 256 characters should be sufficient for an error message. But maybe some languages with UTF-8 (after all, it's a locale specific function).

    Edit: Also FWIW we're robust against errors in strerror. It will just return Unknown error (code) in that case.

    laanwj commented at 4:41 PM on April 21, 2022:

    Bumped in a separate commit.

    jonatack commented at 5:05 PM on April 21, 2022:

    Yes, 1024 seems like a lot. That was how much RAM a Sinclair ZX80 had 😄.

  14. jonatack commented at 1:09 PM on April 21, 2022: member

    ACK 9d5fa881678a code review, at each commit built and tested running error_test on debian testing 5.16.18-1 (2022-03-29) with -DHAVE_CONFIG_H defined, verified in man strerror that the strerror_r() function used here is similar to strerror(), but is thread safe. Tested that both the gnu and posix paths in SysErrorString() work for me locally. Looked up strerror_s in https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/strerror-s-strerror-s-wcserror-s-wcserror-s and that it is thread-safe as well.

  15. laanwj force-pushed on Apr 21, 2022
  16. laanwj force-pushed on Apr 21, 2022
  17. jonatack commented at 5:45 PM on April 21, 2022: member

    ACK e3915676b0ac7da4ee58602231d6616a2e0b01ae

  18. prusnak commented at 9:22 PM on April 21, 2022: contributor

    Approach ACK

  19. MarcoFalke commented at 11:20 AM on April 26, 2022: member

    What about:

    diff --git a/test/lint/lint-locale-dependence.py b/test/lint/lint-locale-dependence.py
index 2abf1be6b3..837224a1c4 100755
--- a/test/lint/lint-locale-dependence.py
+++ b/test/lint/lint-locale-dependence.py
@@ -144,7 +144,7 @@ LOCALE_DEPENDENT_FUNCTIONS = [
     "strcasecmp",
     "strcasestr",
     "strcoll",      # LC_COLLATE
-    #"strerror",
+    "strerror",
     "strfmon",
     "strftime",     # LC_TIME
     "strncasecmp",
  20. laanwj force-pushed on Apr 28, 2022
  21. laanwj commented at 7:58 AM on April 28, 2022: member

    Rebased (to get the Python port of the linter) and added a commit that updates the linter.

    • Added doc comment re NetworkErrorString
    • Updated the copyright year in syserror.* to 2022
  22. laanwj force-pushed on Apr 28, 2022
  23. laanwj force-pushed on Apr 28, 2022
  24. util: Replace non-threadsafe strerror
    Some uses of non-threadsafe `strerror` have snuck into the code since
they were removed in #4152. Add a wrapper `SysErrorString` for
thread-safe strerror alternatives and replace all uses of `strerror`
with this.
    46971c6dbf
  25. util: Use strerror_s for SysErrorString on Windows e7f2f77756
  26. util: Refactor SysErrorString logic
    Deduplicate code and error checks by making sure `s` stays `nullptr`
in case of error. Return "Unknown error" instead of an empty string in
this case.
    718da302c7
  27. util: Increase buffer size to 1024 in SysErrorString
    Increase the error message buffer to 1024 as recommended in the manual
page (Thanks Jon Atack)
    f00fb1265a
  28. test: Add `strerror` to locale-dependence linter
    Add `strerror` to the locale-dependence linter to catch its use. Add
exemptions for bdb interface code (false positive) and strerror.cpp
(the only allowed use).

Also fix a bug in the regexp so that `_r` and `_s` variants are detected
again.
    e3a06a3c6c
  29. laanwj force-pushed on Apr 28, 2022
  30. jonatack commented at 10:34 AM on April 28, 2022: member

    ACK e3a06a3c6cbb288ac89a2725cf71ae8adaebf35c

  31. laanwj merged this on May 4, 2022
  32. laanwj closed this on May 4, 2022
  33. sidhujag referenced this in commit c86f6f9120 on May 4, 2022
  34. luke-jr referenced this in commit 74025fa311 on May 21, 2022
  35. luke-jr referenced this in commit c3bcedb66a on May 21, 2022
  36. luke-jr referenced this in commit d0e42a4c00 on May 21, 2022
  37. luke-jr referenced this in commit e80c91e034 on May 21, 2022
  38. luke-jr referenced this in commit ce5dc7c1a3 on May 21, 2022
  39. luke-jr referenced this in commit 3c651702c6 on May 22, 2022
  40. DrahtBot locked this on May 4, 2023
Contributors
laanwjjonatackprusnakMarcoFalke
Labels
Utils/log/libs
Linked (view graph)
#4152 Replace non-threadsafe strerror gmtime setlocale
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-13 15:14 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me