From bb60956b393c3534fb4960772c7fd023aa0f81d5 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 20 Apr 2022 01:40:36 +0900 Subject: [PATCH 1/7] path-util: make readlink_value() refuse O_DIRECTORY returned from path_extract_filename() The function is now only used by sd-device.c and pam_systemd.c, and they expects the result are not directory. Hence, it is safe to change the behavior. Addresses https://github.com/systemd/systemd/pull/23089#discussion_r853006017. --- src/basic/fs-util.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index 96b588a4b5..cc25222c32 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -155,7 +155,7 @@ int readlink_malloc(const char *p, char **ret) { } int readlink_value(const char *p, char **ret) { - _cleanup_free_ char *link = NULL; + _cleanup_free_ char *link = NULL, *name = NULL; int r; assert(p); @@ -165,7 +165,14 @@ int readlink_value(const char *p, char **ret) { if (r < 0) return r; - return path_extract_filename(link, ret); + r = path_extract_filename(link, &name); + if (r < 0) + return r; + if (r == O_DIRECTORY) + return -EINVAL; + + *ret = TAKE_PTR(name); + return 0; } int readlink_and_make_absolute(const char *p, char **r) { From e4e1353c251e253b7cdf48db41b27572b6ed9172 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 20 Apr 2022 01:43:11 +0900 Subject: [PATCH 2/7] sd-device: refuse O_DIRECTORY returned from path_extract_filename() In both cases, it is expected that the symlink targets do not end with '/'. Addresses https://github.com/systemd/systemd/pull/23089#discussion_r853007218. --- src/libsystemd/sd-device/sd-device.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index 0c5b2a67e3..bf9c753db5 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -1181,6 +1181,8 @@ static int device_set_sysname_and_sysnum(sd_device *device) { r = path_extract_filename(device->devpath, &sysname); if (r < 0) return r; + if (r == O_DIRECTORY) + return -EINVAL; /* some devices have '!' in their name, change that to '/' */ for (p = strchrnul(sysname, '!'); *p != '\0'; p = strchrnul(p, '!')) @@ -1457,6 +1459,8 @@ int device_get_device_id(sd_device *device, const char **ret) { r = path_extract_filename(device->devpath, &sysname); if (r < 0) return r; + if (r == O_DIRECTORY) + return -EINVAL; if (streq(subsystem, "drivers")) { /* the 'drivers' pseudo-subsystem is special, and needs the real From 072f5f9b18850ece1d14ff2e5d75a792821bfdd4 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 20 Apr 2022 01:58:28 +0900 Subject: [PATCH 3/7] string-util: introduce string_replace_char() --- src/basic/string-util.c | 12 ++++++++++++ src/basic/string-util.h | 2 ++ src/test/test-string-util.c | 8 ++++++++ 3 files changed, 22 insertions(+) diff --git a/src/basic/string-util.c b/src/basic/string-util.c index 6ceaeaf9df..71ce921288 100644 --- a/src/basic/string-util.c +++ b/src/basic/string-util.c @@ -1162,3 +1162,15 @@ bool streq_skip_trailing_chars(const char *s1, const char *s2, const char *ok) { return in_charset(s1, ok) && in_charset(s2, ok); } + +char *string_replace_char(char *str, char old_char, char new_char) { + assert(str); + assert(old_char != '\0'); + assert(new_char != '\0'); + assert(old_char != new_char); + + for (char *p = strchr(str, old_char); p; p = strchr(p + 1, old_char)) + *p = new_char; + + return str; +} diff --git a/src/basic/string-util.h b/src/basic/string-util.h index ffb69e69cc..b871224f7b 100644 --- a/src/basic/string-util.h +++ b/src/basic/string-util.h @@ -233,3 +233,5 @@ static inline int string_contains_word(const char *string, const char *separator } bool streq_skip_trailing_chars(const char *s1, const char *s2, const char *ok); + +char *string_replace_char(char *str, char old_char, char new_char); diff --git a/src/test/test-string-util.c b/src/test/test-string-util.c index 93b674baab..63bb74ac10 100644 --- a/src/test/test-string-util.c +++ b/src/test/test-string-util.c @@ -935,6 +935,14 @@ TEST(strextendf) { assert_se(streq(p, "<77>,<99>,< 88>,<00001234>")); } +TEST(string_replace_char) { + assert_se(streq(string_replace_char(strdupa(""), 'a', 'b'), "")); + assert_se(streq(string_replace_char(strdupa("abc"), 'a', 'b'), "bbc")); + assert_se(streq(string_replace_char(strdupa("hoge"), 'a', 'b'), "hoge")); + assert_se(streq(string_replace_char(strdupa("aaaa"), 'a', 'b'), "bbbb")); + assert_se(streq(string_replace_char(strdupa("aaaa"), 'a', '\t'), "\t\t\t\t")); +} + TEST(streq_skip_trailing_chars) { /* NULL is WHITESPACE by default*/ assert_se(streq_skip_trailing_chars("foo bar", "foo bar", NULL)); From 146f4482b2f6dccb2a53543dca81cec40e7f6fa4 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 20 Apr 2022 02:15:01 +0900 Subject: [PATCH 4/7] string-util: introduce strspn_from_end() --- src/basic/string-util.c | 15 +++++++++++++++ src/basic/string-util.h | 2 ++ src/test/test-string-util.c | 12 ++++++++++++ 3 files changed, 29 insertions(+) diff --git a/src/basic/string-util.c b/src/basic/string-util.c index 71ce921288..128aea99c0 100644 --- a/src/basic/string-util.c +++ b/src/basic/string-util.c @@ -1174,3 +1174,18 @@ char *string_replace_char(char *str, char old_char, char new_char) { return str; } + +size_t strspn_from_end(const char *str, const char *accept) { + size_t n = 0; + + if (isempty(str)) + return 0; + + if (isempty(accept)) + return 0; + + for (const char *p = str + strlen(str); p > str && strchr(accept, p[-1]); p--) + n++; + + return n; +} diff --git a/src/basic/string-util.h b/src/basic/string-util.h index b871224f7b..60e80484a3 100644 --- a/src/basic/string-util.h +++ b/src/basic/string-util.h @@ -235,3 +235,5 @@ static inline int string_contains_word(const char *string, const char *separator bool streq_skip_trailing_chars(const char *s1, const char *s2, const char *ok); char *string_replace_char(char *str, char old_char, char new_char); + +size_t strspn_from_end(const char *str, const char *accept); diff --git a/src/test/test-string-util.c b/src/test/test-string-util.c index 63bb74ac10..00163d501e 100644 --- a/src/test/test-string-util.c +++ b/src/test/test-string-util.c @@ -943,6 +943,18 @@ TEST(string_replace_char) { assert_se(streq(string_replace_char(strdupa("aaaa"), 'a', '\t'), "\t\t\t\t")); } +TEST(strspn_from_end) { + assert_se(strspn_from_end(NULL, NULL) == 0); + assert_se(strspn_from_end("hoge", NULL) == 0); + assert_se(strspn_from_end(NULL, DIGITS) == 0); + assert_se(strspn_from_end("", DIGITS) == 0); + assert_se(strspn_from_end("hoge", DIGITS) == 0); + assert_se(strspn_from_end("1234", DIGITS) == 4); + assert_se(strspn_from_end("aaa1234", DIGITS) == 4); + assert_se(strspn_from_end("aaa1234aaa", DIGITS) == 0); + assert_se(strspn_from_end("aaa12aa34", DIGITS) == 2); +} + TEST(streq_skip_trailing_chars) { /* NULL is WHITESPACE by default*/ assert_se(streq_skip_trailing_chars("foo bar", "foo bar", NULL)); From 9dbfcaf229e6dbf2af1ec1e9d4ac769047db8db0 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 20 Apr 2022 02:19:44 +0900 Subject: [PATCH 5/7] sd-device: use string_replace_char() and strspn_from_end() --- src/libsystemd/sd-device/sd-device.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index bf9c753db5..c7f4c48d42 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -1173,7 +1173,7 @@ _public_ int sd_device_get_devname(sd_device *device, const char **devname) { static int device_set_sysname_and_sysnum(sd_device *device) { _cleanup_free_ char *sysname = NULL; - char *p; + size_t len, n; int r; assert(device); @@ -1185,14 +1185,15 @@ static int device_set_sysname_and_sysnum(sd_device *device) { return -EINVAL; /* some devices have '!' in their name, change that to '/' */ - for (p = strchrnul(sysname, '!'); *p != '\0'; p = strchrnul(p, '!')) - *p = '/'; + string_replace_char(sysname, '!', '/'); - /* trailing number (refuse number only sysname)*/ - for (; p > sysname && isdigit(p[-1]); p--) - ; + n = strspn_from_end(sysname, DIGITS); + len = strlen(sysname); + assert(n <= len); + if (n == len) + n = 0; /* Do not set sysnum for number only sysname. */ - device->sysnum = p > sysname && *p != '\0' ? p : NULL; + device->sysnum = n > 0 ? sysname + len - n : NULL; return free_and_replace(device->sysname, sysname); } From 2a86ed67edc91e99e58d93140d9f37609141611b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 20 Apr 2022 02:23:11 +0900 Subject: [PATCH 6/7] efi-api: use string_replace_char() --- src/shared/efi-api.c | 8 -------- src/shared/efi-api.h | 5 ++++- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/shared/efi-api.c b/src/shared/efi-api.c index b2c6af8754..b9faae025b 100644 --- a/src/shared/efi-api.c +++ b/src/shared/efi-api.c @@ -546,11 +546,3 @@ bool efi_has_tpm2(void) { } #endif - -char *efi_tilt_backslashes(char *s) { - for (char *p = s; *p; p++) - if (*p == '\\') - *p = '/'; - - return s; -} diff --git a/src/shared/efi-api.h b/src/shared/efi-api.h index 5acc9e83ce..c36524f404 100644 --- a/src/shared/efi-api.h +++ b/src/shared/efi-api.h @@ -3,6 +3,7 @@ #include "efivars-fundamental.h" #include "efivars.h" +#include "string-util.h" /* Various calls for interfacing with EFI variables from the official UEFI specs. */ @@ -65,4 +66,6 @@ static inline bool efi_has_tpm2(void) { #endif -char *efi_tilt_backslashes(char *s); +static inline char *efi_tilt_backslashes(char *s) { + return string_replace_char(s, '\\', '/'); +} From 5a560d4cc377e28e3d460dbded486f01851caff3 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Wed, 20 Apr 2022 02:27:14 +0900 Subject: [PATCH 7/7] firewall-util: emphasize that nfnl_netlink_sendv() takes at least one message Addresses https://github.com/systemd/systemd/pull/23090#discussion_r853002631. --- src/shared/firewall-util-nft.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/firewall-util-nft.c b/src/shared/firewall-util-nft.c index 1970834878..2f98e791c2 100644 --- a/src/shared/firewall-util-nft.c +++ b/src/shared/firewall-util-nft.c @@ -31,7 +31,7 @@ static int nfnl_netlink_sendv( sd_netlink *nfnl, - sd_netlink_message **messages, + sd_netlink_message *messages[static 1], size_t msgcount) { _cleanup_free_ uint32_t *serial = NULL;