From 5b552d2946b65bec91f4c4473034130e0982e548 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 1 Apr 2020 12:30:12 +0200 Subject: [PATCH 1/6] ifupdown: use _nm_utils_ascii_str_to_int64() for converting netmask to string (cherry picked from commit 3930ef194ec37d51eb74aed6e7e2ac403539bbfd) (cherry picked from commit 1a80179c60ea02af9da1ce454afbdc3cae023426) (cherry picked from commit 1a54909bb4161341272c761759ac8c35fa87a46c) --- src/settings/plugins/ifupdown/nms-ifupdown-parser.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-parser.c b/src/settings/plugins/ifupdown/nms-ifupdown-parser.c index 239f641596..266ab578d7 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-parser.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-parser.c @@ -582,9 +582,8 @@ update_ip6_setting_from_if_block (NMConnection *connection, const char *nameserver_v; const char *nameservers_v; const char *search_v; - int prefix_int = 128; + guint prefix_int; - /* Address */ address_v = ifparser_getkey (block, "address"); if (!address_v) { g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION, @@ -592,12 +591,12 @@ update_ip6_setting_from_if_block (NMConnection *connection, return FALSE; } - /* Prefix */ prefix_v = ifparser_getkey (block, "netmask"); if (prefix_v) - prefix_int = g_ascii_strtoll (prefix_v, NULL, 10); + prefix_int = _nm_utils_ascii_str_to_int64 (prefix_v, 10, 0, 128, G_MAXINT); + else + prefix_int = 128; - /* Add the new address to the setting */ addr = nm_ip_address_new (AF_INET6, address_v, prefix_int, error); if (!addr) return FALSE; @@ -610,7 +609,6 @@ update_ip6_setting_from_if_block (NMConnection *connection, } nm_ip_address_unref (addr); - /* gateway */ gateway_v = ifparser_getkey (block, "gateway"); if (gateway_v) { if (!nm_utils_ipaddr_valid (AF_INET6, gateway_v)) { @@ -631,7 +629,6 @@ update_ip6_setting_from_if_block (NMConnection *connection, if (!nm_setting_ip_config_get_num_dns (s_ip6)) _LOGI ("No dns-nameserver configured in /etc/network/interfaces"); - /* DNS searches */ search_v = ifparser_getkey (block, "dns-search"); if (search_v) { gs_free const char **list = NULL; From 2511c64ea4ed144672e80a0ed6f05da3f622c575 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 1 Apr 2020 12:47:55 +0200 Subject: [PATCH 2/6] device/bluetooth: avoid g_ascii_strtoull() to parse capabilities Avoid g_ascii_strtoull() calling directly. It has subtle issues, which is why we have a wrapper for it. (cherry picked from commit 659ac9cc1299399b34b4677a492c9943458e1b52) (cherry picked from commit 62469c1401dcbd155da5e03bf8cfd37986d6541f) (cherry picked from commit 386ea3ff26fe68105c4f72d1706b06a3a2dc4341) --- src/devices/bluetooth/nm-bluez-device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/devices/bluetooth/nm-bluez-device.c b/src/devices/bluetooth/nm-bluez-device.c index 377ee478a0..dce9db2061 100644 --- a/src/devices/bluetooth/nm-bluez-device.c +++ b/src/devices/bluetooth/nm-bluez-device.c @@ -680,7 +680,7 @@ convert_uuids_to_capabilities (const char **strings) parts = g_strsplit (*iter, "-", -1); if (parts && parts[0]) { - switch (g_ascii_strtoull (parts[0], NULL, 16)) { + switch (_nm_utils_ascii_str_to_int64 (parts[0], 16, 0, G_MAXINT, -1)) { case 0x1103: capabilities |= NM_BT_CAPABILITY_DUN; break; From 4633d6a8b5adb12137bcd5ef61cb917bc826a3c3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 1 Apr 2020 12:26:20 +0200 Subject: [PATCH 3/6] shared: add nm_g_ascii_strtoll() to workaround bug (cherry picked from commit f4446e34c689d6bd81deb8bbab01387716db801f) (cherry picked from commit 6836679878889f882acd243b0c914865971edd4e) (cherry picked from commit 49c523cf1e4da6a14fb09d0877db44646df1c688) --- shared/nm-glib-aux/nm-shared-utils.c | 66 ++++++++++++++++++++++++++++ shared/nm-glib-aux/nm-shared-utils.h | 4 ++ 2 files changed, 70 insertions(+) diff --git a/shared/nm-glib-aux/nm-shared-utils.c b/shared/nm-glib-aux/nm-shared-utils.c index fb945ef9fb..83913e6673 100644 --- a/shared/nm-glib-aux/nm-shared-utils.c +++ b/shared/nm-glib-aux/nm-shared-utils.c @@ -714,6 +714,72 @@ nm_utils_parse_inaddr_prefix (int addr_family, /*****************************************************************************/ +/** + * nm_g_ascii_strtoll() + * @nptr: the string to parse + * @endptr: the pointer on the first invalid chars + * @base: the base. + * + * This wraps g_ascii_strtoll() and should in almost all cases behave identical + * to it. + * + * However, it seems there are situations where g_ascii_strtoll() might set + * errno to some unexpected value EAGAIN. Possibly this is related to creating + * the C locale during + * + * #ifdef USE_XLOCALE + * return strtoll_l (nptr, endptr, base, get_C_locale ()); + * + * This wrapper tries to workaround that condition. + */ +gint64 +nm_g_ascii_strtoll (const char *nptr, + char **endptr, + guint base) +{ + int try_count = 2; + gint64 v; + const int errsv_orig = errno; + int errsv; + + nm_assert (nptr); + nm_assert (base == 0u || (base >= 2u && base <= 36u)); + +again: + errno = 0; + v = g_ascii_strtoll (nptr, endptr, base); + errsv = errno; + + if (errsv == 0) { + if (errsv_orig != 0) + errno = errsv_orig; + return v; + } + + if ( errsv == ERANGE + && NM_IN_SET (v, G_MININT64, G_MAXINT64)) + return v; + + if ( errsv == EINVAL + && v == 0 + && nptr + && nptr[0] == '\0') + return v; + + if (try_count-- > 0) + goto again; + +#if NM_MORE_ASSERTS + g_critical ("g_ascii_strtoll() for \"%s\" failed with errno=%d (%s) and v=%"G_GINT64_FORMAT, + nptr, + errsv, + nm_strerror_native (errsv), + v); +#endif + + return v; +} + /* _nm_utils_ascii_str_to_int64: * * A wrapper for g_ascii_strtoll, that checks whether the whole string diff --git a/shared/nm-glib-aux/nm-shared-utils.h b/shared/nm-glib-aux/nm-shared-utils.h index 19d6184aa3..76cd8d368c 100644 --- a/shared/nm-glib-aux/nm-shared-utils.h +++ b/shared/nm-glib-aux/nm-shared-utils.h @@ -539,6 +539,10 @@ gboolean nm_utils_parse_inaddr_prefix (int addr_family, char **out_addr, int *out_prefix); +gint64 nm_g_ascii_strtoll (const char *nptr, + char **endptr, + guint base); + gint64 _nm_utils_ascii_str_to_int64 (const char *str, guint base, gint64 min, gint64 max, gint64 fallback); guint64 _nm_utils_ascii_str_to_uint64 (const char *str, guint base, guint64 min, guint64 max, guint64 fallback); From e2bfbd9c81aa30154c31a5e77217484c6a7c7ece Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 1 Apr 2020 12:26:20 +0200 Subject: [PATCH 4/6] shared: add nm_g_ascii_strtod() to workaround bug (cherry picked from commit 35a9f632a81df40209e3d71f2328ccdfcf175aee) (cherry picked from commit f8cae1ed18ad5d29c246e9d9036f670badd28126) (cherry picked from commit 0de1c3a53a6196694cbe4ecb2dade727a7469e65) --- shared/nm-glib-aux/nm-shared-utils.c | 37 ++++++++++++++++++++++++++++ shared/nm-glib-aux/nm-shared-utils.h | 3 +++ 2 files changed, 40 insertions(+) diff --git a/shared/nm-glib-aux/nm-shared-utils.c b/shared/nm-glib-aux/nm-shared-utils.c index 83913e6673..f8be16ba18 100644 --- a/shared/nm-glib-aux/nm-shared-utils.c +++ b/shared/nm-glib-aux/nm-shared-utils.c @@ -780,6 +780,43 @@ again: return v; } +/* see nm_g_ascii_strtoll(). */ +double +nm_g_ascii_strtod (const char *nptr, + char **endptr) +{ + int try_count = 2; + double v; + int errsv; + + nm_assert (nptr); + +again: + v = g_ascii_strtod (nptr, endptr); + errsv = errno; + + if (errsv == 0) + return v; + + if (errsv == ERANGE) + return v; + + if (try_count-- > 0) + goto again; + +#if NM_MORE_ASSERTS + g_critical ("g_ascii_strtod() for \"%s\" failed with errno=%d (%s) and v=%f", + nptr, + errsv, + nm_strerror_native (errsv), + v); +#endif + + /* Not really much else to do. Return the parsed value and leave errno set + * to the unexpected value. */ + return v; +} + /* _nm_utils_ascii_str_to_int64: * * A wrapper for g_ascii_strtoll, that checks whether the whole string diff --git a/shared/nm-glib-aux/nm-shared-utils.h b/shared/nm-glib-aux/nm-shared-utils.h index 76cd8d368c..0860515562 100644 --- a/shared/nm-glib-aux/nm-shared-utils.h +++ b/shared/nm-glib-aux/nm-shared-utils.h @@ -543,6 +543,9 @@ gint64 nm_g_ascii_strtoll (const char *nptr, char **endptr, guint base); +double nm_g_ascii_strtod (const char *nptr, + char **endptr); + gint64 _nm_utils_ascii_str_to_int64 (const char *str, guint base, gint64 min, gint64 max, gint64 fallback); guint64 _nm_utils_ascii_str_to_uint64 (const char *str, guint base, guint64 min, guint64 max, guint64 fallback); From 12ab561ee9f52e11074d0736b27e72b95e0da6c5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 1 Apr 2020 13:50:16 +0200 Subject: [PATCH 5/6] shared: add nm_g_ascii_strtoull() to workaround bug (cherry picked from commit 3b58c5fef490cfdae632f5007c77267e47971734) (cherry picked from commit 95565bef77ddfb36dd61b764a3d8c0b6cec21333) (cherry picked from commit d629db4a0e80752e031ca0f5c410ad4b59e9090a) --- shared/nm-glib-aux/nm-shared-utils.c | 49 ++++++++++++++++++++++++++++ shared/nm-glib-aux/nm-shared-utils.h | 4 +++ 2 files changed, 53 insertions(+) diff --git a/shared/nm-glib-aux/nm-shared-utils.c b/shared/nm-glib-aux/nm-shared-utils.c index f8be16ba18..b8bbeeb561 100644 --- a/shared/nm-glib-aux/nm-shared-utils.c +++ b/shared/nm-glib-aux/nm-shared-utils.c @@ -780,6 +780,55 @@ again: return v; } +/* See nm_g_ascii_strtoll() */ +guint64 +nm_g_ascii_strtoull (const char *nptr, + char **endptr, + guint base) +{ + int try_count = 2; + guint64 v; + const int errsv_orig = errno; + int errsv; + + nm_assert (nptr); + nm_assert (base == 0u || (base >= 2u && base <= 36u)); + +again: + errno = 0; + v = g_ascii_strtoull (nptr, endptr, base); + errsv = errno; + + if (errsv == 0) { + if (errsv_orig != 0) + errno = errsv_orig; + return v; + } + + if ( errsv == ERANGE + && NM_IN_SET (v, G_MAXUINT64)) + return v; + + if ( errsv == EINVAL + && v == 0 + && nptr + && nptr[0] == '\0') + return v; + + if (try_count-- > 0) + goto again; + +#if NM_MORE_ASSERTS + g_critical ("g_ascii_strtoull() for \"%s\" failed with errno=%d (%s) and v=%"G_GUINT64_FORMAT, + nptr, + errsv, + nm_strerror_native (errsv), + v); +#endif + + return v; +} + /* see nm_g_ascii_strtoll(). */ double nm_g_ascii_strtod (const char *nptr, diff --git a/shared/nm-glib-aux/nm-shared-utils.h b/shared/nm-glib-aux/nm-shared-utils.h index 0860515562..e069366afc 100644 --- a/shared/nm-glib-aux/nm-shared-utils.h +++ b/shared/nm-glib-aux/nm-shared-utils.h @@ -543,6 +543,10 @@ gint64 nm_g_ascii_strtoll (const char *nptr, char **endptr, guint base); +guint64 nm_g_ascii_strtoull (const char *nptr, + char **endptr, + guint base); + double nm_g_ascii_strtod (const char *nptr, char **endptr); From 2792b6619ba5b193ab27c5818cba0e6c24967a5b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 1 Apr 2020 12:30:20 +0200 Subject: [PATCH 6/6] all: use wrappers for g_ascii_strtoll(), g_ascii_strtoull(), g_ascii_strtod() Sometimes these function may set errno to unexpected values like EAGAIN. This causes confusion. Avoid that by using our own wrappers that retry in that case. For example, in rhbz#1797915 we have failures like: errno = 0; v = g_ascii_strtoll ("10", 0, &end); if (errno != 0) g_assert_not_reached (); as g_ascii_strtoll() would return 10, but also set errno to EAGAIN. Work around that by using wrapper functions that retry. This certainly should be fixed in glib (or glibc), but the issues are severe enough to warrant a workaround. Note that our workarounds are very defensive. We only retry 2 times, if we get an unexpected errno value. This is in the hope to recover from a spurious EAGAIN. It won't recover from other errors. https://bugzilla.redhat.com/show_bug.cgi?id=1797915 (cherry picked from commit 7e49f4a199beb9b8012ec554c4a9ad1c851f7ff2) (cherry picked from commit eec2740d718db4c73bf4c31b97852ee83bb3dfa7) (cherry picked from commit 500f0b96ae93cc0dbcb63124c46956cf532031c3) --- libnm-core/nm-utils.c | 4 ++-- shared/nm-glib-aux/nm-shared-utils.c | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index 6d8a0fe327..c2f038dc80 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -2286,7 +2286,7 @@ _nm_utils_parse_tc_handle (const char *str, GError **error) nm_assert (str); - maj = g_ascii_strtoll (str, (char **) &sep, 0x10); + maj = nm_g_ascii_strtoll (str, (char **) &sep, 0x10); if (sep == str) goto fail; @@ -2295,7 +2295,7 @@ _nm_utils_parse_tc_handle (const char *str, GError **error) if (sep[0] == ':') { const char *str2 = &sep[1]; - min = g_ascii_strtoll (str2, (char **) &sep, 0x10); + min = nm_g_ascii_strtoll (str2, (char **) &sep, 0x10); sep = nm_str_skip_leading_spaces (sep); if (sep[0] != '\0') goto fail; diff --git a/shared/nm-glib-aux/nm-shared-utils.c b/shared/nm-glib-aux/nm-shared-utils.c index b8bbeeb561..62753dc06e 100644 --- a/shared/nm-glib-aux/nm-shared-utils.c +++ b/shared/nm-glib-aux/nm-shared-utils.c @@ -893,7 +893,7 @@ _nm_utils_ascii_str_to_int64 (const char *str, guint base, gint64 min, gint64 ma } errno = 0; - v = g_ascii_strtoll (str, (char **) &s, base); + v = nm_g_ascii_strtoll (str, (char **) &s, base); if (errno != 0) return fallback; @@ -929,7 +929,7 @@ _nm_utils_ascii_str_to_uint64 (const char *str, guint base, guint64 min, guint64 } errno = 0; - v = g_ascii_strtoull (str, (char **) &s, base); + v = nm_g_ascii_strtoull (str, (char **) &s, base); if (errno != 0) return fallback; @@ -948,8 +948,8 @@ _nm_utils_ascii_str_to_uint64 (const char *str, guint base, guint64 min, guint64 if ( v != 0 && str[0] == '-') { - /* I don't know why, but g_ascii_strtoull() accepts minus signs ("-2" gives 18446744073709551614). - * For "-0" that is OK, but otherwise not. */ + /* As documented, g_ascii_strtoull() accepts negative values, and returns their + * absolute value. We don't. */ errno = ERANGE; return fallback; }