From d15b8c6c561258eb0e0b92d6176a16ccc8c23be3 Mon Sep 17 00:00:00 2001 From: Antonio Cardace Date: Thu, 27 Aug 2020 18:18:31 +0200 Subject: [PATCH 1/4] core: add 'dhcp-vendor-class-identifier' validation function So that it can be reused. Signed-off-by: Antonio Cardace (cherry picked from commit 5cca669ff39c6909be906e8974e424ffd2ea42c2) (cherry picked from commit 847488cb2f9f0ba8017938e0876677180c0c91a0) --- .../nm-libnm-core-utils.c | 55 +++++++++++++++++++ .../nm-libnm-core-utils.h | 2 + libnm-core/nm-setting-ip4-config.c | 48 +--------------- po/POTFILES.in | 1 + src/devices/nm-device.c | 11 ++-- 5 files changed, 65 insertions(+), 52 deletions(-) diff --git a/libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.c b/libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.c index f2c85cc60..8be7d913f 100644 --- a/libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.c +++ b/libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.c @@ -6,6 +6,8 @@ #include "nm-common-macros.h" +#include "nm-errors.h" + #include /*****************************************************************************/ @@ -257,3 +259,56 @@ NM_UTILS_ENUM2STR_DEFINE (nm_utils_route_type2str, guint8, NM_UTILS_ENUM2STR (RTN_UNREACHABLE, "unreachable"), NM_UTILS_ENUM2STR (RTN_UNSPEC, "unspecified"), ); + +gboolean +nm_utils_validate_dhcp4_vendor_class_id (const char *vci, GError **error) +{ + const char * bin; + gsize unescaped_len; + gs_free char *to_free = NULL; + + g_return_val_if_fail (!error || !(*error), FALSE); + g_return_val_if_fail (vci, FALSE); + + if (vci[0] == '\0') { + g_set_error_literal (error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _ ("property cannot be an empty string")); + g_prefix_error (error, + "%s.%s: ", + NM_SETTING_IP4_CONFIG_SETTING_NAME, + NM_SETTING_IP4_CONFIG_DHCP_VENDOR_CLASS_IDENTIFIER); + return FALSE; + } + + bin = nm_utils_buf_utf8safe_unescape (vci, + NM_UTILS_STR_UTF8_SAFE_FLAG_NONE, + &unescaped_len, + (gpointer *) &to_free); + /* a DHCP option cannot be longer than 255 bytes */ + if (unescaped_len > 255) { + g_set_error_literal (error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _ ("property cannot be longer than 255 bytes")); + g_prefix_error (error, + "%s.%s: ", + NM_SETTING_IP4_CONFIG_SETTING_NAME, + NM_SETTING_IP4_CONFIG_DHCP_VENDOR_CLASS_IDENTIFIER); + return FALSE; + } + if (strlen (bin) != unescaped_len) { + g_set_error_literal (error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _ ("property cannot contain any nul bytes")); + g_prefix_error (error, + "%s.%s: ", + NM_SETTING_IP4_CONFIG_SETTING_NAME, + NM_SETTING_IP4_CONFIG_DHCP_VENDOR_CLASS_IDENTIFIER); + return FALSE; + } + + return TRUE; +} diff --git a/libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.h b/libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.h index bb3fa5fcf..6c1337d88 100644 --- a/libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.h +++ b/libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.h @@ -140,4 +140,6 @@ guint8 nm_utils_route_type_by_name (const char *name); const char *nm_utils_route_type2str (guint8 val, char *buf, gsize len); +gboolean nm_utils_validate_dhcp4_vendor_class_id (const char *vci, GError **error); + #endif /* __NM_LIBNM_SHARED_UTILS_H__ */ diff --git a/libnm-core/nm-setting-ip4-config.c b/libnm-core/nm-setting-ip4-config.c index 0b8dc89b3..7ffefc25b 100644 --- a/libnm-core/nm-setting-ip4-config.c +++ b/libnm-core/nm-setting-ip4-config.c @@ -227,51 +227,9 @@ verify (NMSetting *setting, NMConnection *connection, GError **error) return FALSE; } - if (priv->dhcp_vendor_class_identifier) { - const char * bin; - gsize unescaped_len; - gs_free char *to_free = NULL; - - if (priv->dhcp_vendor_class_identifier[0] == '\0') { - g_set_error_literal (error, - NM_CONNECTION_ERROR, - NM_CONNECTION_ERROR_INVALID_PROPERTY, - _ ("property cannot be an empty string")); - g_prefix_error (error, - "%s.%s: ", - NM_SETTING_IP4_CONFIG_SETTING_NAME, - NM_SETTING_IP4_CONFIG_DHCP_VENDOR_CLASS_IDENTIFIER); - return FALSE; - } - - bin = nm_utils_buf_utf8safe_unescape (priv->dhcp_vendor_class_identifier, - NM_UTILS_STR_UTF8_SAFE_FLAG_NONE, - &unescaped_len, - (gpointer *) &to_free); - /* a DHCP option cannot be longer than 255 bytes */ - if (unescaped_len > 255) { - g_set_error_literal (error, - NM_CONNECTION_ERROR, - NM_CONNECTION_ERROR_INVALID_PROPERTY, - _ ("property cannot be longer than 255 bytes")); - g_prefix_error (error, - "%s.%s: ", - NM_SETTING_IP4_CONFIG_SETTING_NAME, - NM_SETTING_IP4_CONFIG_DHCP_VENDOR_CLASS_IDENTIFIER); - return FALSE; - } - if (strlen (bin) != unescaped_len) { - g_set_error_literal (error, - NM_CONNECTION_ERROR, - NM_CONNECTION_ERROR_INVALID_PROPERTY, - _ ("property cannot contain any nul bytes")); - g_prefix_error (error, - "%s.%s: ", - NM_SETTING_IP4_CONFIG_SETTING_NAME, - NM_SETTING_IP4_CONFIG_DHCP_VENDOR_CLASS_IDENTIFIER); - return FALSE; - } - } + if ( priv->dhcp_vendor_class_identifier + && !nm_utils_validate_dhcp4_vendor_class_id (priv->dhcp_vendor_class_identifier, error)) + return FALSE; /* Failures from here on are NORMALIZABLE_ERROR... */ diff --git a/po/POTFILES.in b/po/POTFILES.in index 25cb5c4a6..ea2eafa3f 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -59,6 +59,7 @@ libnm-core/nm-dbus-utils.c libnm-core/nm-keyfile/nm-keyfile-utils.c libnm-core/nm-keyfile/nm-keyfile.c libnm-core/nm-libnm-core-aux/nm-libnm-core-aux.c +libnm-core/nm-libnm-core-intern/nm-libnm-core-utils.c libnm-core/nm-setting-6lowpan.c libnm-core/nm-setting-8021x.c libnm-core/nm-setting-adsl.c diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 72a2b1008..012af4d9a 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -8754,7 +8754,6 @@ dhcp4_get_vendor_class_identifier (NMDevice *self, NMSettingIP4Config *s_ip4) { gs_free char *config_data_prop = NULL; gs_free char *to_free = NULL; - gboolean validate = FALSE; const char *conn_prop; GBytes *bytes = NULL; const char *bin; @@ -8764,12 +8763,14 @@ dhcp4_get_vendor_class_identifier (NMDevice *self, NMSettingIP4Config *s_ip4) if (!conn_prop) { /* set in NetworkManager.conf ? */ - validate = TRUE; config_data_prop = nm_config_data_get_connection_default ( NM_CONFIG_GET_DATA, NM_CON_DEFAULT ("ipv4.dhcp-vendor-class-identifier"), self); - conn_prop = config_data_prop; + + if ( config_data_prop + && nm_utils_validate_dhcp4_vendor_class_id (config_data_prop, NULL)) + conn_prop = config_data_prop; } if (conn_prop) { @@ -8777,10 +8778,6 @@ dhcp4_get_vendor_class_identifier (NMDevice *self, NMSettingIP4Config *s_ip4) NM_UTILS_STR_UTF8_SAFE_FLAG_NONE, &len, (gpointer *) &to_free); - - if (validate && (bin[0] == '\0' || len > 255 || strlen (bin) != len)) - return NULL; - if (to_free) bytes = g_bytes_new_take (g_steal_pointer (&to_free), len); else -- 2.26.2 From a491bad06f7c29b257fd948f2149ef80b179da22 Mon Sep 17 00:00:00 2001 From: Antonio Cardace Date: Thu, 27 Aug 2020 17:43:54 +0200 Subject: [PATCH 2/4] initrd: parse 'rd.net.dhcp.vendor-class' kernel cmdline arg This arguments makes NM set the ipv4.dhcp-vendor-class-identifier property for all connections. https://bugzilla.redhat.com/show_bug.cgi?id=1872299 Signed-off-by: Antonio Cardace (cherry picked from commit c056cb9306be29a2c194a308b3b6cc639980abe2) (cherry picked from commit 15856a4fa20feaae6bd073fc2874180b2a1a335d) --- src/initrd/nmi-cmdline-reader.c | 6 ++++ src/initrd/tests/test-cmdline-reader.c | 47 ++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/src/initrd/nmi-cmdline-reader.c b/src/initrd/nmi-cmdline-reader.c index be39ef896..ba747b30a 100644 --- a/src/initrd/nmi-cmdline-reader.c +++ b/src/initrd/nmi-cmdline-reader.c @@ -28,6 +28,7 @@ typedef struct { /* Parameters to be set for all connections */ gboolean ignore_auto_dns; int dhcp_timeout; + char *dhcp4_vci; } Reader; static Reader * @@ -52,6 +53,7 @@ reader_destroy (Reader *reader, gboolean free_hash) g_ptr_array_unref (reader->array); hash = g_steal_pointer (&reader->hash); nm_clear_g_free (&reader->hostname); + nm_clear_g_free (&reader->dhcp4_vci); nm_g_slice_free (reader); if (!free_hash) return g_steal_pointer (&hash); @@ -95,6 +97,7 @@ reader_create_connection (Reader *reader, NM_SETTING_IP_CONFIG_MAY_FAIL, TRUE, NM_SETTING_IP_CONFIG_IGNORE_AUTO_DNS, reader->ignore_auto_dns, NM_SETTING_IP_CONFIG_DHCP_TIMEOUT, reader->dhcp_timeout, + NM_SETTING_IP4_CONFIG_DHCP_VENDOR_CLASS_IDENTIFIER, reader->dhcp4_vci, NULL); setting = nm_setting_ip6_config_new (); @@ -927,6 +930,9 @@ nmi_cmdline_reader_parse (const char *sysfs_dir, const char *const*argv, char ** else if (nm_streq (tag, "rd.net.timeout.dhcp")) { reader->dhcp_timeout = _nm_utils_ascii_str_to_int64 (argument, 10, 0, G_MAXINT32, 0); + } else if (nm_streq (tag, "rd.net.dhcp.vendor-class")) { + if (nm_utils_validate_dhcp4_vendor_class_id (argument, NULL)) + nm_utils_strdup_reset (&reader->dhcp4_vci, argument); } } diff --git a/src/initrd/tests/test-cmdline-reader.c b/src/initrd/tests/test-cmdline-reader.c index 7787cf5ea..a909bc380 100644 --- a/src/initrd/tests/test-cmdline-reader.c +++ b/src/initrd/tests/test-cmdline-reader.c @@ -1485,6 +1485,52 @@ test_bootif_off (void) g_assert_cmpstr (hostname, ==, NULL); } +static void +test_dhcp_vendor_class_id (void) +{ + gs_unref_hashtable GHashTable *connections = NULL; + const char *const*ARGV = NM_MAKE_STRV ("rd.net.dhcp.vendor-class=testvci", + "ip=eno1:dhcp"); + NMConnection *connection; + NMSettingIP4Config *s_ip4; + gs_free char *hostname = NULL; + gs_free char *vci_long = NULL; + char vci_arg_long[512] = {0}; + + connections = nmi_cmdline_reader_parse (TEST_INITRD_DIR "/sysfs", ARGV, &hostname); + g_assert (connections); + g_assert_cmpint (g_hash_table_size (connections), ==, 1); + g_assert_cmpstr (hostname, ==, NULL); + + connection = g_hash_table_lookup (connections, "eno1"); + g_assert (connection); + nmtst_assert_connection_verifies_without_normalization (connection); + s_ip4 = NM_SETTING_IP4_CONFIG (nm_connection_get_setting_ip4_config (connection)); + g_assert_cmpstr (nm_setting_ip4_config_get_dhcp_vendor_class_identifier (s_ip4), ==, "testvci"); + + ARGV = NM_MAKE_STRV ("rd.net.dhcp.vendor-class", + "ip=eno1:dhcp"); + connections = nmi_cmdline_reader_parse (TEST_INITRD_DIR "/sysfs", ARGV, &hostname); + connection = g_hash_table_lookup (connections, "eno1"); + g_assert (connection); + nmtst_assert_connection_verifies_without_normalization (connection); + s_ip4 = NM_SETTING_IP4_CONFIG (nm_connection_get_setting_ip4_config (connection)); + g_assert (nm_setting_ip4_config_get_dhcp_vendor_class_identifier (s_ip4) == NULL); + + + + memset (vci_arg_long, 'A', 400); + vci_long = g_strdup_printf ("rd.net.dhcp.vendor-class=%s", vci_arg_long); + ARGV = NM_MAKE_STRV (vci_long, + "ip=eno1:dhcp"); + connections = nmi_cmdline_reader_parse (TEST_INITRD_DIR "/sysfs", ARGV, &hostname); + connection = g_hash_table_lookup (connections, "eno1"); + g_assert (connection); + nmtst_assert_connection_verifies_without_normalization (connection); + s_ip4 = NM_SETTING_IP4_CONFIG (nm_connection_get_setting_ip4_config (connection)); + g_assert (nm_setting_ip4_config_get_dhcp_vendor_class_identifier (s_ip4) == NULL); +} + NMTST_DEFINE (); int main (int argc, char **argv) @@ -1521,6 +1567,7 @@ int main (int argc, char **argv) g_test_add_func ("/initrd/cmdline/bootif/hwtype", test_bootif_hwtype); g_test_add_func ("/initrd/cmdline/bootif/off", test_bootif_off); g_test_add_func ("/initrd/cmdline/neednet", test_neednet); + g_test_add_func ("/initrd/cmdline/dhcp/vendor_class_id", test_dhcp_vendor_class_id); return g_test_run (); } -- 2.26.2 From bbd77df8ae1cc2510b1ff2c1c27ddf3d907faec3 Mon Sep 17 00:00:00 2001 From: Antonio Cardace Date: Tue, 1 Sep 2020 18:38:45 +0200 Subject: [PATCH 3/4] initrd: fix memory leak Signed-off-by: Antonio Cardace Fixes: 9f9609555d1c ('initrd: add configuration generator') (cherry picked from commit d5c05d07c7aff317284d2d5197d75e0f605b4364) (cherry picked from commit bba54613eb4255166c921844e8b6d2a2bd0000a1) --- shared/nm-glib-aux/nm-macros-internal.h | 8 ++++++++ src/initrd/nm-initrd-generator.c | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/shared/nm-glib-aux/nm-macros-internal.h b/shared/nm-glib-aux/nm-macros-internal.h index 15bcd7e58..57ddee050 100644 --- a/shared/nm-glib-aux/nm-macros-internal.h +++ b/shared/nm-glib-aux/nm-macros-internal.h @@ -216,6 +216,14 @@ NM_AUTO_DEFINE_FCN0 (GError *, gs_local_free_error, g_error_free) #define gs_unref_keyfile nm_auto(gs_local_keyfile_unref) NM_AUTO_DEFINE_FCN0 (GKeyFile *, gs_local_keyfile_unref, g_key_file_unref) +/** + * gs_free_option_context: + * + * Call g_option_context_free() on a variable location when it goes out of scope. + */ +#define gs_free_option_context nm_auto(gs_local_option_context) +NM_AUTO_DEFINE_FCN0 (GOptionContext *, gs_local_option_context, g_option_context_free); + /*****************************************************************************/ #include "nm-glib.h" diff --git a/src/initrd/nm-initrd-generator.c b/src/initrd/nm-initrd-generator.c index f984ed739..5a93480bf 100644 --- a/src/initrd/nm-initrd-generator.c +++ b/src/initrd/nm-initrd-generator.c @@ -83,7 +83,7 @@ main (int argc, char *argv[]) { G_OPTION_REMAINING, '\0', 0, G_OPTION_ARG_STRING_ARRAY, &remaining, NULL, NULL }, { NULL } }; - GOptionContext *option_context; + gs_free_option_context GOptionContext *option_context = NULL; gs_free_error GError *error = NULL; gs_free char *hostname = NULL; int errsv; -- 2.26.2 From 3dec958f413a4566e97183a522afb27b47a9146e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 3 Sep 2020 11:35:40 +0200 Subject: [PATCH 4/4] initrd/tests: fix memleak in test_dhcp_vendor_class_id() Having leaks in the tests, breaks running the test under valgrind. There must be no leaks. Fixes: c056cb9306be ('initrd: parse 'rd.net.dhcp.vendor-class' kernel cmdline arg') (cherry picked from commit bff23d15d41a42c7b5f43cb3d18d66e7cd289823) (cherry picked from commit 5bea8db7ca8fd7520fe605b59e29b974e04b4721) Signed-off-by: Antonio Cardace --- src/initrd/tests/test-cmdline-reader.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/initrd/tests/test-cmdline-reader.c b/src/initrd/tests/test-cmdline-reader.c index a909bc380..a11b76e01 100644 --- a/src/initrd/tests/test-cmdline-reader.c +++ b/src/initrd/tests/test-cmdline-reader.c @@ -1508,6 +1508,8 @@ test_dhcp_vendor_class_id (void) s_ip4 = NM_SETTING_IP4_CONFIG (nm_connection_get_setting_ip4_config (connection)); g_assert_cmpstr (nm_setting_ip4_config_get_dhcp_vendor_class_identifier (s_ip4), ==, "testvci"); + nm_clear_pointer (&connections, g_hash_table_unref); + ARGV = NM_MAKE_STRV ("rd.net.dhcp.vendor-class", "ip=eno1:dhcp"); connections = nmi_cmdline_reader_parse (TEST_INITRD_DIR "/sysfs", ARGV, &hostname); @@ -1517,7 +1519,7 @@ test_dhcp_vendor_class_id (void) s_ip4 = NM_SETTING_IP4_CONFIG (nm_connection_get_setting_ip4_config (connection)); g_assert (nm_setting_ip4_config_get_dhcp_vendor_class_identifier (s_ip4) == NULL); - + nm_clear_pointer (&connections, g_hash_table_unref); memset (vci_arg_long, 'A', 400); vci_long = g_strdup_printf ("rd.net.dhcp.vendor-class=%s", vci_arg_long); -- 2.26.2