From: Yu Watanabe <watanabe.yu+github@gmail.com>
Date: Thu, 6 Apr 2023 23:45:35 +0000 (+0900)
Subject: sd-device,udev: tag must be a valid filename
X-Git-Tag: v254-rc1~761^2~4
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=0b4c70b47a6dea1841a3be5c132d562d71efe182;p=thirdparty%2Fsystemd.git

sd-device,udev: tag must be a valid filename

All tags are managed under /run/udev/tags, and the directories there are
named with tags. Hence, each tag must be a valid filename.

This also makes all validity check moved to sd-device side, and
makes failure caused by setting invalid tags non-critical.

With this change, an empty string cannot be assigned to TAG=, hence the
test cases are adjusted.
---

diff --git a/src/libsystemd/sd-device/device-private.c b/src/libsystemd/sd-device/device-private.c
index e9d54f514e6..9ded8c17a17 100644
--- a/src/libsystemd/sd-device/device-private.c
+++ b/src/libsystemd/sd-device/device-private.c
@@ -351,6 +351,8 @@ static int device_amend(sd_device *device, const char *key, const char *value) {
                                 return r;
                         if (r == 0)
                                 break;
+                        if (isempty(word))
+                                continue;
 
                         r = device_add_tag(device, word, streq(key, "CURRENT_TAGS"));
                         if (r < 0)
diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c
index 2a5defca65b..5d486957ceb 100644
--- a/src/libsystemd/sd-device/sd-device.c
+++ b/src/libsystemd/sd-device/sd-device.c
@@ -1439,7 +1439,7 @@ _public_ int sd_device_get_diskseq(sd_device *device, uint64_t *ret) {
 static bool is_valid_tag(const char *tag) {
         assert(tag);
 
-        return !strchr(tag, ':') && !strchr(tag, ' ');
+        return in_charset(tag, ALPHANUMERICAL "-_") && filename_is_valid(tag);
 }
 
 int device_add_tag(sd_device *device, const char *tag, bool both) {
diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c
index 68f0a363be4..e3d2adbafdd 100644
--- a/src/udev/udev-rules.c
+++ b/src/udev/udev-rules.c
@@ -2487,16 +2487,14 @@ static int udev_rule_apply_token_to_event(
                 if (token->op == OP_ASSIGN)
                         device_cleanup_tags(dev);
 
-                if (buf[strspn(buf, ALPHANUMERICAL "-_")] != '\0') {
-                        log_event_error(dev, token, "Invalid tag name '%s', ignoring", buf);
-                        break;
-                }
                 if (token->op == OP_REMOVE)
                         device_remove_tag(dev, buf);
                 else {
                         r = device_add_tag(dev, buf, true);
+                        if (r == -ENOMEM)
+                                return log_oom();
                         if (r < 0)
-                                return log_event_error_errno(dev, token, r, "Failed to add tag '%s': %m", buf);
+                                log_event_warning_errno(dev, token, r, "Failed to add tag '%s', ignoring: %m", buf);
                 }
                 break;
         }
diff --git a/test/udev-test.pl b/test/udev-test.pl
index 9b3641b3d94..16c82bec0ca 100755
--- a/test/udev-test.pl
+++ b/test/udev-test.pl
@@ -1808,9 +1808,9 @@ EOF
                                 not_exp_name    => "bad",
                        }],
                 rules           => <<EOF
-KERNEL=="sda", TAG=""
-TAGS=="|foo", SYMLINK+="found"
-TAGS=="aaa|bbb", SYMLINK+="bad"
+KERNEL=="sda", ENV{HOGE}=""
+ENV{HOGE}=="|foo", SYMLINK+="found"
+ENV{HOGE}=="aaa|bbb", SYMLINK+="bad"
 EOF
         },
         {
@@ -1836,9 +1836,9 @@ EOF
                                 not_exp_name    => "bad",
                         }],
                 rules           => <<EOF
-KERNEL=="sda", TAG=""
-TAGS=="foo||bar", SYMLINK+="found"
-TAGS=="aaa|bbb", SYMLINK+="bad"
+KERNEL=="sda", ENV{HOGE}=""
+ENV{HOGE}=="foo||bar", SYMLINK+="found"
+ENV{HOGE}=="aaa|bbb", SYMLINK+="bad"
 EOF
         },
         {
@@ -1864,9 +1864,9 @@ EOF
                                 not_exp_name    => "bad",
                         }],
                 rules           => <<EOF
-KERNEL=="sda", TAG=""
-TAGS=="foo|", SYMLINK+="found"
-TAGS=="aaa|bbb", SYMLINK+="bad"
+KERNEL=="sda", ENV{HOGE}=""
+ENV{HOGE}=="foo|", SYMLINK+="found"
+ENV{HOGE}=="aaa|bbb", SYMLINK+="bad"
 EOF
         },
         {
@@ -1881,6 +1881,25 @@ EOF
 KERNEL=="sda", TAG="c"
 TAGS=="foo||bar||c", SYMLINK+="found"
 TAGS=="aaa||bbb||ccc", SYMLINK+="bad"
+EOF
+        },
+        {
+                desc            => "TAG refuses invalid string",
+                devices => [
+                        {
+                                devpath         => "/devices/pci0000:00/0000:00:1f.2/host0/target0:0:0/0:0:0:0/block/sda",
+                                exp_links       => ["valid", "found"],
+                                not_exp_links   => ["empty", "invalid_char", "path", "bad", "bad2"],
+                        }],
+                rules           => <<EOF
+KERNEL=="sda", TAG+="", TAG+="invalid.char", TAG+="path/is/also/invalid", TAG+="valid"
+TAGS=="", SYMLINK+="empty"
+TAGS=="invalid.char", SYMLINK+="invalid_char"
+TAGS=="path/is/also/invalid", SYMLINK+="path"
+TAGS=="valid", SYMLINK+="valid"
+TAGS=="valid|", SYMLINK+="found"
+TAGS=="aaa|", SYMLINK+="bad"
+TAGS=="aaa|bbb", SYMLINK+="bad2"
 EOF
         },
         {