From 4dd4d5b7591f96fdf1c8a2ae1c9caaffa11d9a33 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 13 Jul 2022 17:18:29 +0200 Subject: [PATCH 1/3] glib-aux: don't use GPtrArray in nm_utils_strsplit_quoted() GPtrArray requires an additional heap allocation for the GPtrArray. Utterly useless in the majority of cases. Anyway. Allocating (and exponentially grown) a buffer is not too hard, just slightly more cumbersome. Since nm_utils_strsplit_quoted() is heavily unit tested and entirely self-contained, let's opt for the more complicated implementation and avoid the extra allocation. --- src/libnm-glib-aux/nm-shared-utils.c | 33 ++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/src/libnm-glib-aux/nm-shared-utils.c b/src/libnm-glib-aux/nm-shared-utils.c index 67a4d58852..6128b0cb8a 100644 --- a/src/libnm-glib-aux/nm-shared-utils.c +++ b/src/libnm-glib-aux/nm-shared-utils.c @@ -2288,9 +2288,11 @@ nm_utils_escaped_tokens_options_split(char *str, const char **out_key, const cha char ** nm_utils_strsplit_quoted(const char *str) { - gs_unref_ptrarray GPtrArray *arr = NULL; - gs_free char *str_out = NULL; - CharLookupTable ch_lookup; + char **arr = NULL; + gsize arr_len = 0; + gsize arr_alloc = 0; + gs_free char *str_out = NULL; + CharLookupTable ch_lookup; nm_assert(str); @@ -2347,19 +2349,32 @@ nm_utils_strsplit_quoted(const char *str) str++; } - if (!arr) - arr = g_ptr_array_new(); - g_ptr_array_add(arr, g_strndup(str_out, j)); + if (arr_len >= arr_alloc) { + if (arr_alloc == 0) + arr_alloc = 4; + else + arr_alloc *= 2; + arr = g_realloc(arr, sizeof(char *) * arr_alloc); + } + + arr[arr_len++] = g_strndup(str_out, j); } if (!arr) return g_new0(char *, 1); - g_ptr_array_add(arr, NULL); - /* We want to return an optimally sized strv array, with no excess * memory allocated. Hence, clone once more. */ - return nm_memdup(arr->pdata, sizeof(char *) * arr->len); + + if (arr_len + 1u != arr_alloc) { + gs_free char **arr_old = arr; + + arr = g_new(char *, arr_len + 1u); + memcpy(arr, arr_old, sizeof(char *) * arr_len); + } + + arr[arr_len] = NULL; + return arr; } /*****************************************************************************/ From d4b7934997035f8a793d4a7c624aefa86f05ba85 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 13 Jul 2022 17:39:36 +0200 Subject: [PATCH 2/3] core: support "/run/NetworkManager/proc-cmdline" to overwrite /proc/cmdline We read /proc/cmdline for "match.kernel-command-line". But next we will also honor "nm.debug" on the kernel command line, to enable debug logging. For "nm.debug" it makes sense that it overwrites the debug options from the command line and from "NetworkManager.conf". That means, if you set "nm.debug", then verbose logging will be enabled. It can only be turned off again at runtime (via D-Bus), otherwise, it's hard to avoid. It still can make sense to overrule this setting once again. Support that, by honoring a file "/run/NetworkManager/proc-cmdline" to be used instead of "/proc/cmdline". This option is mainly for debugging and testing, but it might be useful in production too, if you had "nm.debug" enabled during boot, but later want to disable it until next reboot. Then you could do: sed 's/ *\ */ /g' /proc/cmdline > /run/NetworkManager/proc-cmdline nmcli general logging level DEFAULT domains DEFAULT --- src/core/nm-core-utils.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/core/nm-core-utils.c b/src/core/nm-core-utils.c index b2ebe30c25..66f8204502 100644 --- a/src/core/nm-core-utils.c +++ b/src/core/nm-core-utils.c @@ -3090,7 +3090,10 @@ again: if (G_UNLIKELY(!proc_cmdline)) { gs_free char *str = NULL; - g_file_get_contents("/proc/cmdline", &str, NULL, NULL); + /* /run/NetworkManager/proc-cmdline can be used to overrule /proc/cmdline. */ + if (!g_file_get_contents(NMRUNDIR "/proc-cmdline", &str, NULL, NULL)) + g_file_get_contents("/proc/cmdline", &str, NULL, NULL); + str = nm_str_realloc(str); proc_cmdline = str ?: ""; From 5c84fe0db5894701805c50534642ff9b6fd606ba Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 13 Jul 2022 17:32:37 +0200 Subject: [PATCH 3/3] core: support "nm.debug" kernel command line to enable verbose logging When NetworkManager runs in initrd, it can be cumbersome to enable debug logging. Granted, when using dracut, the NetworkManager dracut module will honor "rd.debug". However, a user may use NetworkManager in initrd without dracut. Then, the only way to enable debug logging would be by changing "NetworkManager.conf" and rebuild the initrd (or having some script in place, that allows to more conveniently enable debug logging for NetworkManager). To make it easier for debugging, honor "nm.debug" on the kernel command line. Note that if "nm.debug" is set on the kernel command line, it always overrides both the command line arguments and the configuration from NetworkManager.conf. That is intentional. The only way to override that is by overriding the kernel command line with a file "/run/NetworkManager/proc-cmdline". https://bugzilla.redhat.com/show_bug.cgi?id=2102313 --- man/NetworkManager.conf.xml | 7 +++++++ src/core/main.c | 15 ++++++++++----- src/core/nm-config-data.c | 7 +++++++ src/core/nm-config.c | 9 +++++++++ src/core/nm-config.h | 6 ++++++ 5 files changed, 39 insertions(+), 5 deletions(-) diff --git a/man/NetworkManager.conf.xml b/man/NetworkManager.conf.xml index 39cf895cb0..9be7a69e91 100644 --- a/man/NetworkManager.conf.xml +++ b/man/NetworkManager.conf.xml @@ -634,6 +634,13 @@ unmanaged-devices=mac:00:22:68:1c:59:b1;mac:00:1E:65:30:D1:C4;interface-name:eth early on. + + By setting on the kernel command line (either from + &nmrundir;/proc-cmdline or /proc/cmdline), + debug logging is enabled. This overrides both the command-line options and the settings + from NetworkManager.conf. + + NetworkManager's logging aims not to contain private sensitive data and you should be fine sharing the debug logs. Still, there will diff --git a/src/core/main.c b/src/core/main.c index 1cd3788074..2eb230d9ca 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -293,6 +293,7 @@ main(int argc, char *argv[]) GError *error_invalid_logging_config = NULL; const char *const *warnings; int errsv; + gboolean has_logging = FALSE; _nm_utils_is_manager_process = TRUE; @@ -354,10 +355,14 @@ main(int argc, char *argv[]) g_free(path); } - if (!nm_logging_setup(global_opt.opt_log_level, - global_opt.opt_log_domains, - &bad_domains, - &error)) { + if (nm_config_kernel_command_line_nm_debug()) { + /* we honor kernel command line. If "nm.debug" is set, we always enable trace logging. */ + nm_logging_setup("TRACE", "ALL", NULL, NULL); + has_logging = TRUE; + } else if (!nm_logging_setup(global_opt.opt_log_level, + global_opt.opt_log_domains, + &bad_domains, + &error)) { fprintf(stderr, _("%s. Please use --help to see a list of valid options.\n"), error->message); @@ -379,7 +384,7 @@ main(int argc, char *argv[]) /* Initialize logging from config file *only* if not explicitly * specified by commandline. */ - if (global_opt.opt_log_level == NULL && global_opt.opt_log_domains == NULL) { + if (!has_logging && !global_opt.opt_log_level && !global_opt.opt_log_domains) { if (!nm_logging_setup(nm_config_get_log_level(config), nm_config_get_log_domains(config), &bad_domains, diff --git a/src/core/nm-config-data.c b/src/core/nm-config-data.c index ddb7787fef..7e9364b6b5 100644 --- a/src/core/nm-config-data.c +++ b/src/core/nm-config-data.c @@ -839,6 +839,13 @@ nm_config_data_log(const NMConfigData *self, _LOG(stream, prefix, "# no-auto-default specs \"%s\"", msg); } + if (nm_config_kernel_command_line_nm_debug()) { + _LOG(stream, + prefix, + "# /proc/cmdline contains \"" NM_CONFIG_KERNEL_CMDLINE_NM_DEBUG + "\". Debug log enabled"); + } + #undef _LOG } diff --git a/src/core/nm-config.c b/src/core/nm-config.c index 701baae686..3d23d4f6cd 100644 --- a/src/core/nm-config.c +++ b/src/core/nm-config.c @@ -3014,6 +3014,15 @@ set_property(GObject *object, guint prop_id, const GValue *value, GParamSpec *ps /*****************************************************************************/ +gboolean +nm_config_kernel_command_line_nm_debug(void) +{ + return (nm_strv_find_first(nm_utils_proc_cmdline_split(), -1, NM_CONFIG_KERNEL_CMDLINE_NM_DEBUG) + >= 0); +} + +/*****************************************************************************/ + static gboolean init_sync(GInitable *initable, GCancellable *cancellable, GError **error) { diff --git a/src/core/nm-config.h b/src/core/nm-config.h index d7498f92be..0dd159f47e 100644 --- a/src/core/nm-config.h +++ b/src/core/nm-config.h @@ -18,6 +18,8 @@ #define NM_IS_CONFIG_CLASS(klass) (G_TYPE_CHECK_CLASS_TYPE((klass), NM_TYPE_CONFIG)) #define NM_CONFIG_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS((obj), NM_TYPE_CONFIG, NMConfigClass)) +#define NM_CONFIG_KERNEL_CMDLINE_NM_DEBUG "nm.debug" + /* Properties */ #define NM_CONFIG_CMD_LINE_OPTIONS "cmd-line-options" #define NM_CONFIG_ATOMIC_SECTION_PREFIXES "atomic-section-prefixes" @@ -197,4 +199,8 @@ void nm_config_clear_warnings(NMConfig *config); /*****************************************************************************/ +gboolean nm_config_kernel_command_line_nm_debug(void); + +/*****************************************************************************/ + #endif /* __NETWORKMANAGER_CONFIG_H__ */