From 4b5ee5901caa4cecefd333462660956d67ed2975 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 May 2020 19:17:44 +0200 Subject: [PATCH 1/3] proc-cmdline: fix return value clobbering in proc_cmdline_get_key() Let's make sure proc_cmdline_get_key() follows our coding style: never clobber return values on failure, and always initialize on success. --- src/basic/proc-cmdline.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/basic/proc-cmdline.c b/src/basic/proc-cmdline.c index d8d30b0f1d..1bc884c20d 100644 --- a/src/basic/proc-cmdline.c +++ b/src/basic/proc-cmdline.c @@ -218,7 +218,7 @@ static int cmdline_get_key(const char *line, const char *key, ProcCmdlineFlags f } int proc_cmdline_get_key(const char *key, ProcCmdlineFlags flags, char **ret_value) { - _cleanup_free_ char *line = NULL; + _cleanup_free_ char *line = NULL, *v = NULL; int r; /* Looks for a specific key on the kernel command line and (with lower priority) the EFI variable. @@ -245,14 +245,24 @@ int proc_cmdline_get_key(const char *key, ProcCmdlineFlags flags, char **ret_val if (r < 0) return r; - r = cmdline_get_key(line, key, flags, ret_value); - if (r != 0) /* Either error or true if found. */ + r = cmdline_get_key(line, key, flags, ret_value ? &v : NULL); + if (r < 0) return r; + if (r > 0) { + if (ret_value) + *ret_value = TAKE_PTR(v); + + return r; + } line = mfree(line); r = systemd_efi_options_variable(&line); - if (r == -ENODATA) + if (r == -ENODATA) { + if (ret_value) + *ret_value = NULL; + return false; /* Not found */ + } if (r < 0) return r; From 05a7827e8782527f6c28d5f3f1fbec327b141754 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 May 2020 19:18:45 +0200 Subject: [PATCH 2/3] proc-cmdline: add checking of EFI options variable in proc_cmdline_get_key_many() too --- src/basic/proc-cmdline.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/basic/proc-cmdline.c b/src/basic/proc-cmdline.c index 1bc884c20d..5545543fbd 100644 --- a/src/basic/proc-cmdline.c +++ b/src/basic/proc-cmdline.c @@ -296,6 +296,7 @@ int proc_cmdline_get_bool(const char *key, bool *ret) { int proc_cmdline_get_key_many_internal(ProcCmdlineFlags flags, ...) { _cleanup_free_ char *line = NULL; + bool processing_efi = true; const char *p; va_list ap; int r, ret = 0; @@ -306,9 +307,9 @@ int proc_cmdline_get_key_many_internal(ProcCmdlineFlags flags, ...) { /* This call may clobber arguments on failure! */ - r = proc_cmdline(&line); - if (r < 0) - return r; + r = systemd_efi_options_variable(&line); + if (r < 0 && r != -ENODATA) + log_debug_errno(r, "Failed to get SystemdOptions EFI variable, ignoring: %m"); p = line; for (;;) { @@ -317,8 +318,22 @@ int proc_cmdline_get_key_many_internal(ProcCmdlineFlags flags, ...) { r = proc_cmdline_extract_first(&p, &word, flags); if (r < 0) return r; - if (r == 0) + if (r == 0) { + /* We finished with this command line. If this was the EFI one, then let's proceed with the regular one */ + if (processing_efi) { + processing_efi = false; + + line = mfree(line); + r = proc_cmdline(&line); + if (r < 0) + return r; + + p = line; + continue; + } + break; + } va_start(ap, flags); From 09835de3ccfee80703d00cd16978de2c296706ed Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 May 2020 19:22:09 +0200 Subject: [PATCH 3/3] proc-cmdline: make checking of EFI options variable optional And do not use it in the IMPORT{cmdline} udev code. Wherever we expose direct interfaces to check the kernel cmdline, let's not consult our systemd-specific EFI variable, but strictly use the actual kernel variable, because that's what we claim we do. i.e. it's fine to use the EFI variable for our own settings, but for the generic APIs to the kernel cmdline we should not use it. Specifically, this applies to IMPORT{cmdline} and ConditionKernelCommandLine=. In the latter case we weren#t checking the EFI variable anyway, hence let's do the same for the udev case, too. Fixes: #15739 --- src/basic/proc-cmdline.c | 28 ++++++++++++++++++---------- src/basic/proc-cmdline.h | 7 ++++--- src/udev/udev-rules.c | 2 +- 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/basic/proc-cmdline.c b/src/basic/proc-cmdline.c index 5545543fbd..eb45682f3a 100644 --- a/src/basic/proc-cmdline.c +++ b/src/basic/proc-cmdline.c @@ -119,15 +119,18 @@ int proc_cmdline_parse(proc_cmdline_parse_t parse_item, void *data, ProcCmdlineF /* We parse the EFI variable first, because later settings have higher priority. */ - r = systemd_efi_options_variable(&line); - if (r < 0 && r != -ENODATA) - log_debug_errno(r, "Failed to get SystemdOptions EFI variable, ignoring: %m"); + if (!FLAGS_SET(flags, PROC_CMDLINE_IGNORE_EFI_OPTIONS)) { + r = systemd_efi_options_variable(&line); + if (r < 0 && r != -ENODATA) + log_debug_errno(r, "Failed to get SystemdOptions EFI variable, ignoring: %m"); - r = proc_cmdline_parse_given(line, parse_item, data, flags); - if (r < 0) - return r; + r = proc_cmdline_parse_given(line, parse_item, data, flags); + if (r < 0) + return r; + + line = mfree(line); + } - line = mfree(line); r = proc_cmdline(&line); if (r < 0) return r; @@ -245,6 +248,9 @@ int proc_cmdline_get_key(const char *key, ProcCmdlineFlags flags, char **ret_val if (r < 0) return r; + if (FLAGS_SET(flags, PROC_CMDLINE_IGNORE_EFI_OPTIONS)) /* Shortcut */ + return cmdline_get_key(line, key, flags, ret_value); + r = cmdline_get_key(line, key, flags, ret_value ? &v : NULL); if (r < 0) return r; @@ -307,9 +313,11 @@ int proc_cmdline_get_key_many_internal(ProcCmdlineFlags flags, ...) { /* This call may clobber arguments on failure! */ - r = systemd_efi_options_variable(&line); - if (r < 0 && r != -ENODATA) - log_debug_errno(r, "Failed to get SystemdOptions EFI variable, ignoring: %m"); + if (!FLAGS_SET(flags, PROC_CMDLINE_IGNORE_EFI_OPTIONS)) { + r = systemd_efi_options_variable(&line); + if (r < 0 && r != -ENODATA) + log_debug_errno(r, "Failed to get SystemdOptions EFI variable, ignoring: %m"); + } p = line; for (;;) { diff --git a/src/basic/proc-cmdline.h b/src/basic/proc-cmdline.h index 275f46c89e..077d3a99fb 100644 --- a/src/basic/proc-cmdline.h +++ b/src/basic/proc-cmdline.h @@ -6,9 +6,10 @@ #include "log.h" typedef enum ProcCmdlineFlags { - PROC_CMDLINE_STRIP_RD_PREFIX = 1 << 0, /* automatically strip "rd." prefix if it is set (and we are in the initrd, since otherwise we'd not consider it anyway) */ - PROC_CMDLINE_VALUE_OPTIONAL = 1 << 1, /* the value is optional (for boolean switches that can omit the value) */ - PROC_CMDLINE_RD_STRICT = 1 << 2, /* ignore this in the initrd */ + PROC_CMDLINE_STRIP_RD_PREFIX = 1 << 0, /* automatically strip "rd." prefix if it is set (and we are in the initrd, since otherwise we'd not consider it anyway) */ + PROC_CMDLINE_VALUE_OPTIONAL = 1 << 1, /* the value is optional (for boolean switches that can omit the value) */ + PROC_CMDLINE_RD_STRICT = 1 << 2, /* ignore this in the initrd */ + PROC_CMDLINE_IGNORE_EFI_OPTIONS = 1 << 3, /* don't check systemd's private EFI variable */ } ProcCmdlineFlags; typedef int (*proc_cmdline_parse_t)(const char *key, const char *value, void *data); diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index 4c955b0c07..e9975030aa 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -1814,7 +1814,7 @@ static int udev_rule_apply_token_to_event( case TK_M_IMPORT_CMDLINE: { _cleanup_free_ char *value = NULL; - r = proc_cmdline_get_key(token->value, PROC_CMDLINE_VALUE_OPTIONAL, &value); + r = proc_cmdline_get_key(token->value, PROC_CMDLINE_VALUE_OPTIONAL|PROC_CMDLINE_IGNORE_EFI_OPTIONS, &value); if (r < 0) return log_rule_error_errno(dev, rules, r, "Failed to read '%s' option from /proc/cmdline: %m",