all: merge branch 'th/strtoll-workaround'

https://bugzilla.redhat.com/show_bug.cgi?id=1797915

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/456

9 files changed, 186 insertions, 35 deletions

diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c
index 2e4b0ceb44..24d9dfcb43 100644
--- a/libnm-core/nm-utils.c
+++ b/libnm-core/nm-utils.c
@@ -2299,7 +2299,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;
 
@@ -2308,7 +2308,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 74abfb933f..f1371801ca 100644
--- a/shared/nm-glib-aux/nm-shared-utils.c
+++ b/shared/nm-glib-aux/nm-shared-utils.c
@@ -955,6 +955,158 @@ nm_utils_ipaddr_is_normalized (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;
+}
+
+/* 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,
+                   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
@@ -982,26 +1134,10 @@ _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) {
-#if NM_MORE_ASSERTS
-		int errsv = errno;
-
-		/* the caller must not pass an invalid @base. Hence, we expect the only failure that
-		 * can happen here is ERANGE, because invalid @str is not signaled via an errno according
-		 * to documentation. */
-		if (   errsv != ERANGE
-		    || !NM_IN_SET (v, G_MININT64, G_MAXINT64)) {
-			g_error ("g_ascii_strtoll() for \"%s\" failed with errno=%d (%s) and v=%"G_GINT64_FORMAT,
-			         str,
-			         errsv,
-			         nm_strerror_native (errsv),
-			         v);
-		}
-#endif
+	if (errno != 0)
 		return fallback;
-	}
 
 	if (s[0] != '\0') {
 		s = nm_str_skip_leading_spaces (s);
@@ -1034,7 +1170,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;
@@ -1053,8 +1189,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;
 	}
diff --git a/shared/nm-glib-aux/nm-shared-utils.h b/shared/nm-glib-aux/nm-shared-utils.h
index bba60954b1..b68e658c38 100644
--- a/shared/nm-glib-aux/nm-shared-utils.h
+++ b/shared/nm-glib-aux/nm-shared-utils.h
@@ -657,6 +657,17 @@ 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);
+
+guint64 nm_g_ascii_strtoull (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);
 
diff --git a/src/devices/bluetooth/nm-bluez-manager.c b/src/devices/bluetooth/nm-bluez-manager.c
index c24f1306f8..6ff96c323b 100644
--- a/src/devices/bluetooth/nm-bluez-manager.c
+++ b/src/devices/bluetooth/nm-bluez-manager.c
@@ -221,7 +221,7 @@ convert_uuids_to_capabilities (const char *const*strv)
 				continue;
 
 			s_part1 = g_strndup (str, s - str);
-			switch (g_ascii_strtoull (s_part1, NULL, 16)) {
+			switch (_nm_utils_ascii_str_to_int64 (s_part1, 16, 0, G_MAXINT, -1)) {
 			case 0x1103:
 				capabilities |= NM_BT_CAPABILITY_DUN;
 				break;
diff --git a/src/initrd/nmi-cmdline-reader.c b/src/initrd/nmi-cmdline-reader.c
index 41b063132e..2eb6d7e5be 100644
--- a/src/initrd/nmi-cmdline-reader.c
+++ b/src/initrd/nmi-cmdline-reader.c
@@ -719,7 +719,7 @@ reader_parse_vlan (Reader *reader, char *argument)
 	s_vlan = nm_connection_get_setting_vlan (connection);
 	g_object_set (s_vlan,
 	              NM_SETTING_VLAN_PARENT, phy,
-	              NM_SETTING_VLAN_ID, g_ascii_strtoull (vlanid, NULL, 10),
+	              NM_SETTING_VLAN_ID, (guint) _nm_utils_ascii_str_to_int64 (vlanid, 10, 0, G_MAXUINT, G_MAXUINT),
 	              NULL);
 
 	if (argument && *argument)
diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
index 0dc8894080..60bc504717 100644
--- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
+++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
@@ -550,16 +550,18 @@ make_connection_setting (const char *file,
 	g_object_set (s_con, NM_SETTING_CONNECTION_AUTH_RETRIES, (int) vint64, NULL);
 
 	nm_clear_g_free (&value);
-	v = svGetValueStr (ifcfg, "DEVTIMEOUT", &value);
+	v = svGetValue (ifcfg, "DEVTIMEOUT", &value);
 	if (v) {
+		v = nm_str_skip_leading_spaces (v);
 		vint64 = _nm_utils_ascii_str_to_int64 (v, 10, 0, ((gint64) G_MAXINT32) / 1000, -1);
 		if (vint64 != -1)
 			vint64 *= 1000;
-		else {
+		else if (v[0] != '\0') {
 			char *endptr;
 			double d;
 
-			d = g_ascii_strtod (v, &endptr);
+			d = nm_g_ascii_strtod (v, &endptr);
+			endptr = nm_str_skip_leading_spaces (endptr);
 			if (   errno == 0
 			    && endptr[0] == '\0'
 			    && d >= 0.0) {
diff --git a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wired-autoip b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wired-autoip
index dc47126ceb..e683db3c7b 100644
--- a/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wired-autoip
+++ b/src/settings/plugins/ifcfg-rh/tests/network-scripts/ifcfg-test-wired-autoip
@@ -3,3 +3,4 @@ DEVICE=eth0
 BOOTPROTO=autoip
 IPV4_FAILURE_FATAL=yes
 PEERDNS=no
+DEVTIMEOUT=2.6
diff --git a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
index 6db5f64a6b..efa9ea9b82 100644
--- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
+++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
@@ -1738,6 +1738,7 @@ static void
 test_read_wired_autoip (void)
 {
 	gs_unref_object NMConnection *connection = NULL;
+	NMSettingConnection *s_con;
 	NMSettingIPConfig *s_ip4;
 	char *unmanaged = NULL;
 
@@ -1751,6 +1752,9 @@ test_read_wired_autoip (void)
 	g_assert_cmpstr (nm_setting_ip_config_get_method (s_ip4), ==, NM_SETTING_IP4_CONFIG_METHOD_LINK_LOCAL);
 	g_assert (!nm_setting_ip_config_get_may_fail (s_ip4));
 	g_assert (nm_setting_ip_config_get_ignore_auto_dns (s_ip4));
+
+	s_con = nm_connection_get_setting_connection (connection);
+	g_assert_cmpint (nm_setting_connection_get_wait_device_timeout (s_con), ==, 2600);
 }
 
 static void
diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-parser.c b/src/settings/plugins/ifupdown/nms-ifupdown-parser.c
index 64cd431d5d..ac3ed1ce0f 100644
--- a/src/settings/plugins/ifupdown/nms-ifupdown-parser.c
+++ b/src/settings/plugins/ifupdown/nms-ifupdown-parser.c
@@ -565,9 +565,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,
@@ -575,12 +574,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;
@@ -593,7 +592,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_is_valid (AF_INET6, gateway_v)) {
@@ -614,7 +612,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;