From e1ca3bf7ed4068ba3522398d20a1acc04256f43c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 31 Jan 2019 13:17:26 +0100 Subject: [PATCH] shared: add nm_strerror_native() to replace strerror() and g_strerror() We have various options for strerror(), with ups and downsides: - strerror() - returns pointer that is overwritten on next call. It's convenient to use, but dangerous. - not thread-safe. - not guaranteed to be UTF-8. - strerror_r() - takes input buffer and is less convenient to use. At least, we are in control of when the buffer gets overwritten. - there is a Posix/XSI and a glibc variant, making it sligthly inconvenient to used. This could be solved by a wrapper we implement. - thread-safe. - not guaranteed to be UTF-8. - g_strerror() - convenient and safe to use. Also the buffer is never released for the remainder of the program. - passing untrusted error numbers to g_strerror() can result in a denial of service, as the internal buffer grows until out-of-memory. - thread-safe. - guaranteed to be UTF-8 (depending on locale). Add our own wrapper nm_strerror_native(). It is: - convenient to use (returning a buffer that does not require management). - slightly dangerous as the buffer gets overwritten on the next call (like strerror()). - thread-safe. - guaranteed to be UTF-8 (depending on locale). - doesn't keep an unlimited cache of strings, unlike g_strerror(). You can't have it all. g_strerror() is leaking all generated error messages. I think that is unacceptable, because it would mean we need to keep track where our error numbers come from (and trust libraries we use to only set a restricted set of known error numbers). --- shared/nm-utils/nm-errno.c | 123 ++++++++++++++++++++++++++++++++++++- shared/nm-utils/nm-errno.h | 7 +++ 2 files changed, 129 insertions(+), 1 deletion(-) diff --git a/shared/nm-utils/nm-errno.c b/shared/nm-utils/nm-errno.c index ddf280766c..30eb9a8e78 100644 --- a/shared/nm-utils/nm-errno.c +++ b/shared/nm-utils/nm-errno.c @@ -22,6 +22,8 @@ #include "nm-errno.h" +#include + /*****************************************************************************/ NM_UTILS_LOOKUP_STR_DEFINE_STATIC (_geterror, @@ -61,6 +63,20 @@ NM_UTILS_LOOKUP_STR_DEFINE_STATIC (_geterror, NM_UTILS_LOOKUP_ITEM_IGNORE (_NM_ERRNO_RESERVED_LAST_PLUS_1), ); +/** + * nm_strerror(): + * @nmerr: the NetworkManager specific errno to be converted + * to string. + * + * NetworkManager specific error numbers reserve a range in "errno.h" with + * our own defines. For numbers that don't fall into this range, the numbers + * are identical to the common error numbers. + * + * Idential to strerror(), g_strerror(), nm_strerror_native() for error numbers + * that are not in the reserved range of NetworkManager specific errors. + * + * Returns: (transfer none): the string representation of the error number. + */ const char * nm_strerror (int nmerr) { @@ -73,5 +89,110 @@ nm_strerror (int nmerr) if (s) return s; } - return g_strerror (nmerr); + return nm_strerror_native (nmerr); +} + +/*****************************************************************************/ + +/** + * nm_strerror_native_r: + * @errsv: the errno to convert to string. + * @buf: the output buffer where to write the string to. + * @buf_size: the length of buffer. + * + * This is like strerror_r(), with one difference: depending on the + * locale, the returned string is guaranteed to be valid UTF-8. + * Also, there is some confusion as to whether to use glibc's + * strerror_r() or the POXIX/XSI variant. This is abstracted + * by the function. + * + * Note that the returned buffer may also be a statically allocated + * buffer, and not the input buffer @buf. Consequently, the returned + * string may be longer than @buf_size. + * + * Returns: (transfer none): a NUL terminated error message. This is either a static + * string (that is never freed), or the provided @buf argumnt. + */ +const char * +nm_strerror_native_r (int errsv, char *buf, gsize buf_size) +{ + char *buf2; + + nm_assert (buf); + nm_assert (buf_size > 0); + +#if (_POSIX_C_SOURCE >= 200112L) && ! _GNU_SOURCE + /* XSI-compliant */ + { + int errno_saved = errno; + + if (strerror_r (errsv, buf, buf_size) != 0) { + g_snprintf (buf, buf_size, "Unspecified errno %d", errsv); + errno = errno_saved; + } + buf2 = buf; + } +#else + /* GNU-specific */ + buf2 = strerror_r (errsv, buf, buf_size); +#endif + + /* like g_strerror(), ensure that the error message is UTF-8. */ + if ( !g_get_charset (NULL) + && !g_utf8_validate (buf2, -1, NULL)) { + gs_free char *msg = NULL; + + msg = g_locale_to_utf8 (buf2, -1, NULL, NULL, NULL); + if (msg) { + g_strlcpy (buf, msg, buf_size); + buf2 = buf; + } + } + + return buf2; +} + +/** + * nm_strerror_native: + * @errsv: the errno integer from + * + * Like strerror(), but strerror() is not thread-safe and not guaranteed + * to be UTF-8. + * + * g_strerror() is a thread-safe variant of strerror(), however it caches + * all returned strings in a dictionary. That means, using this on untrusted + * error numbers can result in this cache to grow without limits. + * + * Instead, return a tread-local buffer. This way, it's thread-safe. + * + * There is a downside to this: subsequent calls of nm_strerror_native() + * overwrite the error message. + * + * Returns: (transfer none): the text representation of the error number. + */ +const char * +nm_strerror_native (int errsv) +{ + static _nm_thread_local char *buf_static = NULL; + char *buf; + + buf = buf_static; + if (G_UNLIKELY (!buf)) { + int errno_saved = errno; + pthread_key_t key; + + buf = g_malloc (NM_STRERROR_BUFSIZE); + buf_static = buf; + + if ( pthread_key_create (&key, g_free) != 0 + || pthread_setspecific (key, buf) != 0) { + /* Failure. We will leak the buffer when the thread exits. + * + * Nothing we can do about it really. For Debug builds we fail with an assertion. */ + nm_assert_not_reached (); + } + errno = errno_saved; + } + + return nm_strerror_native_r (errsv, buf, NM_STRERROR_BUFSIZE); } diff --git a/shared/nm-utils/nm-errno.h b/shared/nm-utils/nm-errno.h index 68cf49a0c1..4b16b216ec 100644 --- a/shared/nm-utils/nm-errno.h +++ b/shared/nm-utils/nm-errno.h @@ -173,4 +173,11 @@ const char *nm_strerror (int nmerr); /*****************************************************************************/ +#define NM_STRERROR_BUFSIZE 1024 + +const char *nm_strerror_native_r (int errsv, char *buf, gsize buf_size); +const char *nm_strerror_native (int errsv); + +/*****************************************************************************/ + #endif /* __NM_ERRNO_H__ */