From: Anita Zhang <the.anitazha@gmail.com>
Date: Thu, 28 May 2020 19:09:32 +0000 (-0700)
Subject: core: check null_or_empty for masked units instead of /dev/null
X-Git-Tag: v246-rc1~6^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=640f3b143d77b02612dc694a6a2be08f98d6c0e4;p=thirdparty%2Fsystemd.git

core: check null_or_empty for masked units instead of /dev/null

There's some inconsistency in the what is considered a masked unit:
some places (i.e. load-fragment.c) use `null_or_empty()` while others
check if the file path is symlinked to "/dev/null". Since the latter
doesn't account for things like non-absolute symlinks to "/dev/null",
this commit switches the check for "/dev/null" to use `null_or_empty_path()`
---

diff --git a/src/core/unit.c b/src/core/unit.c
index 18bf0cd52ab..c739c0a5616 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -5955,7 +5955,7 @@ const char *unit_label_path(const Unit *u) {
                 return NULL;
 
         /* If a unit is masked, then don't read the SELinux label of /dev/null, as that really makes no sense */
-        if (path_equal(p, "/dev/null"))
+        if (null_or_empty_path(p) > 0)
                 return NULL;
 
         return p;
diff --git a/src/shared/install.c b/src/shared/install.c
index bb2eff73879..0f2d407a36a 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -1324,26 +1324,27 @@ static int unit_file_load_or_readlink(
                 const char *path,
                 const char *root_dir,
                 SearchFlags flags) {
-
-        _cleanup_free_ char *target = NULL;
+        struct stat st;
         int r;
 
         r = unit_file_load(c, info, path, root_dir, flags);
         if (r != -ELOOP || (flags & SEARCH_DROPIN))
                 return r;
 
-        /* This is a symlink, let's read it. */
-
-        r = readlink_malloc(path, &target);
-        if (r < 0)
-                return r;
-
-        if (path_equal(target, "/dev/null"))
+        r = chase_symlinks_and_stat(path, root_dir, CHASE_WARN, NULL, &st, NULL);
+        if (r > 0 && null_or_empty(&st))
                 info->type = UNIT_FILE_TYPE_MASKED;
         else {
+                _cleanup_free_ char *target = NULL;
                 const char *bn;
                 UnitType a, b;
 
+                /* This is a symlink, let's read it. */
+
+                r = readlink_malloc(path, &target);
+                if (r < 0)
+                        return r;
+
                 bn = basename(target);
 
                 if (unit_name_is_valid(info->name, UNIT_NAME_PLAIN)) {
diff --git a/src/test/test-install-root.c b/src/test/test-install-root.c
index 515f14b8ca9..d437686baeb 100644
--- a/src/test/test-install-root.c
+++ b/src/test/test-install-root.c
@@ -4,6 +4,7 @@
 
 #include "alloc-util.h"
 #include "fileio.h"
+#include "fs-util.h"
 #include "install.h"
 #include "mkdir.h"
 #include "rm-rf.h"
@@ -23,6 +24,7 @@ static void test_basic_mask_and_enable(const char *root) {
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "b.service", NULL) == -ENOENT);
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "c.service", NULL) == -ENOENT);
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "d.service", NULL) == -ENOENT);
+        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "e.service", NULL) == -ENOENT);
 
         p = strjoina(root, "/usr/lib/systemd/system/a.service");
         assert_se(write_string_file(p,
@@ -150,6 +152,22 @@ static void test_basic_mask_and_enable(const char *root) {
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "b.service", &state) >= 0 && state == UNIT_FILE_ALIAS);
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "c.service", &state) >= 0 && state == UNIT_FILE_ALIAS);
         assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "d.service", &state) >= 0 && state == UNIT_FILE_ALIAS);
+
+        /* Test masking with relative symlinks */
+
+        p = strjoina(root, "/usr/lib/systemd/system/e.service");
+        assert_se(symlink("../../../../../../dev/null", p) >= 0);
+
+        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "e.service", NULL) >= 0);
+        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "e.service", &state) >= 0 && state == UNIT_FILE_MASKED);
+
+        assert_se(unlink(p) == 0);
+        assert_se(symlink("/usr/../dev/null", p) >= 0);
+
+        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "e.service", NULL) >= 0);
+        assert_se(unit_file_get_state(UNIT_FILE_SYSTEM, root, "e.service", &state) >= 0 && state == UNIT_FILE_MASKED);
+
+        assert_se(unlink(p) == 0);
 }
 
 static void test_linked_units(const char *root) {
@@ -1227,6 +1245,12 @@ int main(int argc, char *argv[]) {
         p = strjoina(root, "/usr/lib/systemd/system-preset/");
         assert_se(mkdir_p(p, 0755) >= 0);
 
+        p = strjoina(root, "/dev/");
+        assert_se(mkdir_p(p, 0755) >= 0);
+
+        p = strjoina(root, "/dev/null");
+        assert_se(touch(p) >= 0);
+
         test_basic_mask_and_enable(root);
         test_linked_units(root);
         test_default(root);