From fb2430c6e56e31c1b4220719640fc544e289214b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 28 Jun 2018 20:57:15 +0200 Subject: [PATCH 01/17] stat-util: add macros for checking whether major and minor values are in range As it turns out glibc and the Linux kernel have different ideas about the size of dev_t and how many bits exist for the major and the minor. When validating major/minor numbers we should check against the kernel's actual sizes, hence add macros for this. --- src/basic/stat-util.h | 19 +++++++++++++++++++ src/test/test-stat-util.c | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/src/basic/stat-util.h b/src/basic/stat-util.h index 84400a6083..fe4a4bb717 100644 --- a/src/basic/stat-util.h +++ b/src/basic/stat-util.h @@ -62,3 +62,22 @@ int fd_verify_regular(int fd); int stat_verify_directory(const struct stat *st); int fd_verify_directory(int fd); + +/* glibc and the Linux kernel have different ideas about the major/minor size. These calls will check whether the + * specified major is valid by the Linux kernel's standards, not by glibc's. Linux has 20bits of minor, and 12 bits of + * major space. See MINORBITS in linux/kdev_t.h in the kernel sources. (If you wonder why we define _y here, instead of + * comparing directly >= 0: it's to trick out -Wtype-limits, which would otherwise complain if the type is unsigned, as + * such a test would be pointless in such a case.) */ + +#define DEVICE_MAJOR_VALID(x) \ + ({ \ + typeof(x) _x = (x), _y = 0; \ + _x >= _y && _x < (UINT32_C(1) << 12); \ + \ + }) + +#define DEVICE_MINOR_VALID(x) \ + ({ \ + typeof(x) _x = (x), _y = 0; \ + _x >= _y && _x < (UINT32_C(1) << 20); \ + }) diff --git a/src/test/test-stat-util.c b/src/test/test-stat-util.c index 2b0564d8a0..713fbc9a08 100644 --- a/src/test/test-stat-util.c +++ b/src/test/test-stat-util.c @@ -81,12 +81,48 @@ static void test_fd_is_network_ns(void) { assert_se(IN_SET(fd_is_network_ns(fd), 1, -EUCLEAN)); } +static void test_device_major_minor_valid(void) { + /* on glibc dev_t is 64bit, even though in the kernel it is only 32bit */ + assert_cc(sizeof(dev_t) == sizeof(uint64_t)); + + assert_se(DEVICE_MAJOR_VALID(0U)); + assert_se(DEVICE_MINOR_VALID(0U)); + + assert_se(DEVICE_MAJOR_VALID(1U)); + assert_se(DEVICE_MINOR_VALID(1U)); + + assert_se(!DEVICE_MAJOR_VALID(-1U)); + assert_se(!DEVICE_MINOR_VALID(-1U)); + + assert_se(DEVICE_MAJOR_VALID(1U << 10)); + assert_se(DEVICE_MINOR_VALID(1U << 10)); + + assert_se(DEVICE_MAJOR_VALID((1U << 12) - 1)); + assert_se(DEVICE_MINOR_VALID((1U << 20) - 1)); + + assert_se(!DEVICE_MAJOR_VALID((1U << 12))); + assert_se(!DEVICE_MINOR_VALID((1U << 20))); + + assert_se(!DEVICE_MAJOR_VALID(1U << 25)); + assert_se(!DEVICE_MINOR_VALID(1U << 25)); + + assert_se(!DEVICE_MAJOR_VALID(UINT32_MAX)); + assert_se(!DEVICE_MINOR_VALID(UINT32_MAX)); + + assert_se(!DEVICE_MAJOR_VALID(UINT64_MAX)); + assert_se(!DEVICE_MINOR_VALID(UINT64_MAX)); + + assert_se(DEVICE_MAJOR_VALID(major(0))); + assert_se(DEVICE_MINOR_VALID(minor(0))); +} + int main(int argc, char *argv[]) { test_files_same(); test_is_symlink(); test_path_is_fs_type(); test_path_is_temporary_fs(); test_fd_is_network_ns(); + test_device_major_minor_valid(); return 0; } From fa583ab176e86464112987b762f272e9deaf8c8a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 29 Jun 2018 12:13:33 +0200 Subject: [PATCH 02/17] logind: validate majors/minors we receieve via the bus --- src/login/logind-session-dbus.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/login/logind-session-dbus.c b/src/login/logind-session-dbus.c index 5b09a07ffa..4eae281bd8 100644 --- a/src/login/logind-session-dbus.c +++ b/src/login/logind-session-dbus.c @@ -12,6 +12,7 @@ #include "logind-session.h" #include "logind.h" #include "signal-util.h" +#include "stat-util.h" #include "strv.h" #include "util.h" @@ -380,6 +381,9 @@ static int method_take_device(sd_bus_message *message, void *userdata, sd_bus_er if (r < 0) return r; + if (!DEVICE_MAJOR_VALID(major) || !DEVICE_MINOR_VALID(minor)) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Device major/minor is not valid."); + if (!session_is_controller(s, sd_bus_message_get_sender(message))) return sd_bus_error_setf(error, BUS_ERROR_NOT_IN_CONTROL, "You are not in control of this session"); @@ -427,6 +431,9 @@ static int method_release_device(sd_bus_message *message, void *userdata, sd_bus if (r < 0) return r; + if (!DEVICE_MAJOR_VALID(major) || !DEVICE_MINOR_VALID(minor)) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Device major/minor is not valid."); + if (!session_is_controller(s, sd_bus_message_get_sender(message))) return sd_bus_error_setf(error, BUS_ERROR_NOT_IN_CONTROL, "You are not in control of this session"); @@ -455,6 +462,9 @@ static int method_pause_device_complete(sd_bus_message *message, void *userdata, if (r < 0) return r; + if (!DEVICE_MAJOR_VALID(major) || !DEVICE_MINOR_VALID(minor)) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Device major/minor is not valid."); + if (!session_is_controller(s, sd_bus_message_get_sender(message))) return sd_bus_error_setf(error, BUS_ERROR_NOT_IN_CONTROL, "You are not in control of this session"); From de06c0cf77e2a58b4655ad5608f85ff8a6e724c7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 29 Jun 2018 11:58:24 +0200 Subject: [PATCH 03/17] parse-util: rework parse_dev() based on safe_atou() and DEVICE_MAJOR_VALID()/DEVICE_MINOR_VALID() Let's be a bit more careful when parsing major/minor pairs, and filter out more corner cases. This also means using safe_atou() rather than sscanf() to avoid weird negative unsigned handling and such. --- src/basic/parse-util.c | 26 ++++++++++++++++++++------ src/test/test-parse-util.c | 2 ++ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/basic/parse-util.c b/src/basic/parse-util.c index ce8bb12670..718357e290 100644 --- a/src/basic/parse-util.c +++ b/src/basic/parse-util.c @@ -16,6 +16,7 @@ #include "missing.h" #include "parse-util.h" #include "process-util.h" +#include "stat-util.h" #include "string-util.h" int parse_boolean(const char *v) { @@ -731,17 +732,30 @@ int parse_ip_port_range(const char *s, uint16_t *low, uint16_t *high) { } int parse_dev(const char *s, dev_t *ret) { + const char *major; unsigned x, y; - dev_t d; + size_t n; + int r; - if (sscanf(s, "%u:%u", &x, &y) != 2) + n = strspn(s, DIGITS); + if (n == 0) + return -EINVAL; + if (s[n] != ':') return -EINVAL; - d = makedev(x, y); - if ((unsigned) major(d) != x || (unsigned) minor(d) != y) - return -EINVAL; + major = strndupa(s, n); + r = safe_atou(major, &x); + if (r < 0) + return r; - *ret = d; + r = safe_atou(s + n + 1, &y); + if (r < 0) + return r; + + if (!DEVICE_MAJOR_VALID(x) || !DEVICE_MINOR_VALID(y)) + return -ERANGE; + + *ret = makedev(x, y); return 0; } diff --git a/src/test/test-parse-util.c b/src/test/test-parse-util.c index e9aef5e882..d732f402f0 100644 --- a/src/test/test-parse-util.c +++ b/src/test/test-parse-util.c @@ -726,10 +726,12 @@ static void test_parse_dev(void) { assert_se(parse_dev("5", &dev) == -EINVAL); assert_se(parse_dev("5:", &dev) == -EINVAL); assert_se(parse_dev(":5", &dev) == -EINVAL); + assert_se(parse_dev("-1:-1", &dev) == -EINVAL); #if SIZEOF_DEV_T < 8 assert_se(parse_dev("4294967295:4294967295", &dev) == -EINVAL); #endif assert_se(parse_dev("8:11", &dev) >= 0 && major(dev) == 8 && minor(dev) == 11); + assert_se(parse_dev("0:0", &dev) >= 0 && major(dev) == 0 && minor(dev) == 0); } static void test_parse_errno(void) { From cd8194a38919c32f2ba1c606f1d2ac8983e08162 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 28 Jun 2018 22:28:40 +0200 Subject: [PATCH 04/17] path-util: add new path_join_many() API --- src/basic/path-util.c | 64 +++++++++++++++++++++++++++++++++++++++ src/basic/path-util.h | 3 ++ src/test/test-path-util.c | 38 +++++++++++++++++++++++ 3 files changed, 105 insertions(+) diff --git a/src/basic/path-util.c b/src/basic/path-util.c index b7f91ee3ae..ba31c858fd 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -495,6 +495,70 @@ char* path_join(const char *root, const char *path, const char *rest) { rest && rest[0] == '/' ? rest+1 : rest); } +char* path_join_many_internal(const char *first, ...) { + char *joined, *q; + const char *p; + va_list ap; + bool slash; + size_t sz; + + assert(first); + + /* Joins all listed strings until NULL and places an "/" between them unless the strings end/begin already with + * one so that it is unnecessary. Note that "/" which are already duplicate won't be removed. The string + * returned is hence always equal or longer than the sum of the lengths of each individual string. + * + * Note: any listed empty string is simply skipped. This can be useful for concatenating strings of which some + * are optional. + * + * Examples: + * + * path_join_many("foo", "bar") → "foo/bar" + * path_join_many("foo/", "bar") → "foo/bar" + * path_join_many("", "foo", "", "bar", "") → "foo/bar" */ + + sz = strlen(first); + va_start(ap, first); + while ((p = va_arg(ap, char*))) { + + if (*p == 0) /* Skip empty items */ + continue; + + sz += 1 + strlen(p); + } + va_end(ap); + + joined = new(char, sz + 1); + if (!joined) + return NULL; + + if (first[0] != 0) { + q = stpcpy(joined, first); + slash = endswith(first, "/"); + } else { + /* Skip empty items */ + joined[0] = 0; + q = joined; + slash = true; /* no need to generate a slash anymore */ + } + + va_start(ap, first); + while ((p = va_arg(ap, char*))) { + + if (*p == 0) /* Skip empty items */ + continue; + + if (!slash && p[0] != '/') + *(q++) = '/'; + + q = stpcpy(q, p); + slash = endswith(p, "/"); + } + va_end(ap); + + return joined; +} + int find_binary(const char *name, char **ret) { int last_error, r; const char *p; diff --git a/src/basic/path-util.h b/src/basic/path-util.h index 53b980d3c1..868d64e17d 100644 --- a/src/basic/path-util.h +++ b/src/basic/path-util.h @@ -50,6 +50,9 @@ int path_compare(const char *a, const char *b) _pure_; bool path_equal(const char *a, const char *b) _pure_; bool path_equal_or_files_same(const char *a, const char *b, int flags); char* path_join(const char *root, const char *path, const char *rest); +char* path_join_many_internal(const char *first, ...) _sentinel_; +#define path_join_many(x, ...) path_join_many_internal(x, __VA_ARGS__, NULL) + char* path_simplify(char *path, bool kill_dots); static inline bool path_equal_ptr(const char *a, const char *b) { diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c index 7feae54068..e3bcfcd4bf 100644 --- a/src/test/test-path-util.c +++ b/src/test/test-path-util.c @@ -567,6 +567,43 @@ static void test_path_startswith_set(void) { assert_se(streq_ptr(PATH_STARTSWITH_SET("/foo2/bar", "/foo/quux", "", "/zzz"), NULL)); } +static void test_path_join_many(void) { + char *j; + + assert_se(streq_ptr(j = path_join_many("", NULL), "")); + free(j); + + assert_se(streq_ptr(j = path_join_many("foo", NULL), "foo")); + free(j); + + assert_se(streq_ptr(j = path_join_many("foo", "bar"), "foo/bar")); + free(j); + + assert_se(streq_ptr(j = path_join_many("", "foo", "", "bar", ""), "foo/bar")); + free(j); + + assert_se(streq_ptr(j = path_join_many("", "", "", "", "foo", "", "", "", "bar", "", "", ""), "foo/bar")); + free(j); + + assert_se(streq_ptr(j = path_join_many("", "/", "", "/foo/", "", "/", "", "/bar/", "", "/", ""), "//foo///bar//")); + free(j); + + assert_se(streq_ptr(j = path_join_many("/", "foo", "/", "bar", "/"), "/foo/bar/")); + free(j); + + assert_se(streq_ptr(j = path_join_many("foo", "bar", "baz"), "foo/bar/baz")); + free(j); + + assert_se(streq_ptr(j = path_join_many("foo/", "bar", "/baz"), "foo/bar/baz")); + free(j); + + assert_se(streq_ptr(j = path_join_many("foo/", "/bar/", "/baz"), "foo//bar//baz")); + free(j); + + assert_se(streq_ptr(j = path_join_many("//foo/", "///bar/", "///baz//"), "//foo////bar////baz//")); + free(j); +} + int main(int argc, char **argv) { test_setup_logging(LOG_DEBUG); @@ -588,6 +625,7 @@ int main(int argc, char **argv) { test_skip_dev_prefix(); test_empty_or_root(); test_path_startswith_set(); + test_path_join_many(); test_systemd_installation_has_version(argv[1]); /* NULL is OK */ From 61e0111df98f1c98b48902634a0612210419e11f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 30 Jul 2018 21:29:34 +0200 Subject: [PATCH 05/17] path-util: port path_join() over to path_join_many() We should probably drop path_join() entirely in the long run (and then rename path_join_many() to it?), but for now let's make one a wrapper for the other. --- src/basic/path-util.c | 14 -------------- src/basic/path-util.h | 6 +++++- 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/src/basic/path-util.c b/src/basic/path-util.c index ba31c858fd..7d1e0f3f2d 100644 --- a/src/basic/path-util.c +++ b/src/basic/path-util.c @@ -481,20 +481,6 @@ bool path_equal_or_files_same(const char *a, const char *b, int flags) { return path_equal(a, b) || files_same(a, b, flags) > 0; } -char* path_join(const char *root, const char *path, const char *rest) { - assert(path); - - if (!isempty(root)) - return strjoin(root, endswith(root, "/") ? "" : "/", - path[0] == '/' ? path+1 : path, - rest ? (endswith(path, "/") ? "" : "/") : NULL, - rest && rest[0] == '/' ? rest+1 : rest); - else - return strjoin(path, - rest ? (endswith(path, "/") ? "" : "/") : NULL, - rest && rest[0] == '/' ? rest+1 : rest); -} - char* path_join_many_internal(const char *first, ...) { char *joined, *q; const char *p; diff --git a/src/basic/path-util.h b/src/basic/path-util.h index 868d64e17d..8bda450ff3 100644 --- a/src/basic/path-util.h +++ b/src/basic/path-util.h @@ -49,9 +49,13 @@ char* path_startswith(const char *path, const char *prefix) _pure_; int path_compare(const char *a, const char *b) _pure_; bool path_equal(const char *a, const char *b) _pure_; bool path_equal_or_files_same(const char *a, const char *b, int flags); -char* path_join(const char *root, const char *path, const char *rest); char* path_join_many_internal(const char *first, ...) _sentinel_; #define path_join_many(x, ...) path_join_many_internal(x, __VA_ARGS__, NULL) +static inline char* path_join(const char *root, const char *path, const char *rest) { + assert(path); + + return path_join_many(strempty(root), path, rest); +} char* path_simplify(char *path, bool kill_dots); From 3a47c40d97ddafe5e9ffd76be2c31a3072e064dd Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 29 Jun 2018 12:01:02 +0200 Subject: [PATCH 06/17] tree-wide: port various parts of the code to use parse_dev() --- src/basic/blockdev-util.c | 22 ++++++++++++---------- src/libsystemd/sd-device/sd-device.c | 15 ++++++++------- src/tmpfiles/tmpfiles.c | 11 ++++------- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/basic/blockdev-util.c b/src/basic/blockdev-util.c index 42b311eccd..3017ecd55d 100644 --- a/src/basic/blockdev-util.c +++ b/src/basic/blockdev-util.c @@ -10,12 +10,13 @@ #include "fd-util.h" #include "fileio.h" #include "missing.h" +#include "parse-util.h" #include "stat-util.h" int block_get_whole_disk(dev_t d, dev_t *ret) { char p[SYS_BLOCK_PATH_MAX("/partition")]; _cleanup_free_ char *s = NULL; - unsigned n, m; + dev_t devt; int r; assert(ret); @@ -38,16 +39,16 @@ int block_get_whole_disk(dev_t d, dev_t *ret) { if (r < 0) return r; - r = sscanf(s, "%u:%u", &m, &n); - if (r != 2) - return -EINVAL; + r = parse_dev(s, &devt); + if (r < 0) + return r; /* Only return this if it is really good enough for us. */ - xsprintf_sys_block_path(p, "/queue", makedev(m, n)); + xsprintf_sys_block_path(p, "/queue", devt); if (access(p, F_OK) < 0) return -ENOENT; - *ret = makedev(m, n); + *ret = devt; return 0; } @@ -85,8 +86,8 @@ int block_get_originating(dev_t dt, dev_t *ret) { _cleanup_free_ char *t = NULL; char p[SYS_BLOCK_PATH_MAX("/slaves")]; struct dirent *de, *found = NULL; - unsigned maj, min; const char *q; + dev_t devt; int r; /* For the specified block device tries to chase it through the layers, in case LUKS-style DM stacking is used, @@ -148,13 +149,14 @@ int block_get_originating(dev_t dt, dev_t *ret) { if (r < 0) return r; - if (sscanf(t, "%u:%u", &maj, &min) != 2) + r = parse_dev(t, &devt); + if (r < 0) return -EINVAL; - if (maj == 0) + if (major(devt) == 0) return -ENOENT; - *ret = makedev(maj, min); + *ret = devt; return 1; } diff --git a/src/libsystemd/sd-device/sd-device.c b/src/libsystemd/sd-device/sd-device.c index dc75f91e21..39def1681e 100644 --- a/src/libsystemd/sd-device/sd-device.c +++ b/src/libsystemd/sd-device/sd-device.c @@ -594,16 +594,17 @@ _public_ int sd_device_new_from_device_id(sd_device **ret, const char *id) { switch (id[0]) { case 'b': - case 'c': - { - char type; - int maj, min; + case 'c': { + dev_t devt; - r = sscanf(id, "%c%i:%i", &type, &maj, &min); - if (r != 3) + if (isempty(id)) return -EINVAL; - return sd_device_new_from_devnum(ret, type, makedev(maj, min)); + r = parse_dev(id + 1, &devt); + if (r < 0) + return r; + + return sd_device_new_from_devnum(ret, id[0], devt); } case 'n': { diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index eeeb1d1850..ed81c49d08 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -2636,24 +2636,21 @@ static int parse_line(const char *fname, unsigned line, const char *buffer, bool break; case CREATE_CHAR_DEVICE: - case CREATE_BLOCK_DEVICE: { - unsigned major, minor; - + case CREATE_BLOCK_DEVICE: if (!i.argument) { *invalid_config = true; log_error("[%s:%u] Device file requires argument.", fname, line); return -EBADMSG; } - if (sscanf(i.argument, "%u:%u", &major, &minor) != 2) { + r = parse_dev(i.argument, &i.major_minor); + if (r < 0) { *invalid_config = true; - log_error("[%s:%u] Can't parse device file major/minor '%s'.", fname, line, i.argument); + log_error_errno(r, "[%s:%u] Can't parse device file major/minor '%s'.", fname, line, i.argument); return -EBADMSG; } - i.major_minor = makedev(major, minor); break; - } case SET_XATTR: case RECURSIVE_SET_XATTR: From 74c48bf5a8005f20dc4ef7b7d05b96572d880b25 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 29 Jun 2018 12:03:33 +0200 Subject: [PATCH 07/17] core: add special handling for devices cgroup allow lists for /dev/block/* and /dev/char/* device nodes This adds some code to hanlde /dev/block/* and /dev/char/* device node paths specially: instead of actually stat()ing them we'll just parse the major/minor name from the name. This is useful 'hack' to allow clients to install whitelists for devices that don't actually have to exist. Also, let's similarly handle /run/systemd/inaccessible/{blk|chr}. This allows us to simplify our built-in default whitelist to not require a "ignore_enoent" mode for these nodes. In general we should be careful with hardcoding major/minor numbers, but in this case this should safe. --- src/core/cgroup.c | 82 ++++++++++++++++++++++++++++++++++++----------- 1 file changed, 64 insertions(+), 18 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 598b396186..8f3e646ad6 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -19,6 +19,7 @@ #include "process-util.h" #include "procfs-util.h" #include "special.h" +#include "stat-util.h" #include "stdio-util.h" #include "string-table.h" #include "string-util.h" @@ -407,31 +408,76 @@ static int lookup_block_device(const char *p, dev_t *ret) { return 0; } +static int shortcut_special_device_path(const char *p, struct stat *ret) { + const char *w; + mode_t mode; + dev_t devt; + int r; + + assert(p); + assert(ret); + + if (path_equal(p, "/run/systemd/inaccessible/chr")) { + *ret = (struct stat) { + .st_mode = S_IFCHR, + .st_rdev = makedev(0, 0), + }; + return 0; + } + + if (path_equal(p, "/run/systemd/inaccessible/blk")) { + *ret = (struct stat) { + .st_mode = S_IFBLK, + .st_rdev = makedev(0, 0), + }; + return 0; + } + + w = path_startswith(p, "/dev/block/"); + if (w) + mode = S_IFBLK; + else { + w = path_startswith(p, "/dev/char/"); + if (!w) + return -ENODEV; + + mode = S_IFCHR; + } + + r = parse_dev(w, &devt); + if (r < 0) + return r; + + *ret = (struct stat) { + .st_mode = mode, + .st_rdev = devt, + }; + + return 0; +} + static int whitelist_device(BPFProgram *prog, const char *path, const char *node, const char *acc) { struct stat st; - bool ignore_notfound; int r; assert(path); assert(acc); - if (node[0] == '-') { - /* Non-existent paths starting with "-" must be silently ignored */ - node++; - ignore_notfound = true; - } else - ignore_notfound = false; + /* Some special handling for /dev/block/%u:%u, /dev/char/%u:%u, /run/systemd/inaccessible/chr and + * /run/systemd/inaccessible/blk paths. Instead of stat()ing these we parse out the major/minor directly. This + * means clients can use these path without the device node actually around */ + r = shortcut_special_device_path(node, &st); + if (r < 0) { + if (r != -ENODEV) + return log_warning_errno(r, "Couldn't parse major/minor from device path '%s': %m", node); - if (stat(node, &st) < 0) { - if (errno == ENOENT && ignore_notfound) - return 0; + if (stat(node, &st) < 0) + return log_warning_errno(errno, "Couldn't stat device %s: %m", node); - return log_warning_errno(errno, "Couldn't stat device %s: %m", node); - } - - if (!S_ISCHR(st.st_mode) && !S_ISBLK(st.st_mode)) { - log_warning("%s is not a device.", node); - return -ENODEV; + if (!S_ISCHR(st.st_mode) && !S_ISBLK(st.st_mode)) { + log_warning("%s is not a device.", node); + return -ENODEV; + } } if (cg_all_unified() > 0) { @@ -1098,8 +1144,8 @@ static void cgroup_context_apply( "/dev/tty\0" "rwm\0" "/dev/ptmx\0" "rwm\0" /* Allow /run/systemd/inaccessible/{chr,blk} devices for mapping InaccessiblePaths */ - "-/run/systemd/inaccessible/chr\0" "rwm\0" - "-/run/systemd/inaccessible/blk\0" "rwm\0"; + "/run/systemd/inaccessible/chr\0" "rwm\0" + "/run/systemd/inaccessible/blk\0" "rwm\0"; const char *x, *y; From 8e8b5d2e6d91180a57844b09cdbdcbc1fa466bfa Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 29 Jun 2018 12:09:29 +0200 Subject: [PATCH 08/17] cgroups: beef up DeviceAllow= syntax a bit Previously we'd allow pattern expressions such as "char-input" to match all input devices. Internally, this would look up the right major to test in /proc/devices. With this commit the syntax is slightly extended: - "char-*" can be used to match any kind of character device, and similar "block-*. This expression would work previously already, but instead of actually installing a wildcard match it would install many individual matches for everything listed in /proc/devices. - "char-" with "" being a numerical parameter works now too. This allows clients to install whitelist items by specifying the major directly. The main reason to add these is to provide limited compat support for clients that for some reason contain whitelists with major/minor numbers (such as OCI containers). --- src/core/bpf-devices.c | 26 +++++++++++++++++++++ src/core/bpf-devices.h | 1 + src/core/cgroup.c | 51 +++++++++++++++++++++++++++++++++++++----- 3 files changed, 73 insertions(+), 5 deletions(-) diff --git a/src/core/bpf-devices.c b/src/core/bpf-devices.c index d8915244a7..dade7f0490 100644 --- a/src/core/bpf-devices.c +++ b/src/core/bpf-devices.c @@ -84,6 +84,32 @@ int cgroup_bpf_whitelist_major(BPFProgram *prog, int type, int major, const char return r; } +int cgroup_bpf_whitelist_class(BPFProgram *prog, int type, const char *acc) { + struct bpf_insn insn[] = { + BPF_JMP_IMM(BPF_JNE, BPF_REG_2, type, 5), /* compare device type */ + BPF_MOV32_REG(BPF_REG_1, BPF_REG_3), /* calculate access type */ + BPF_ALU32_IMM(BPF_AND, BPF_REG_1, 0), + BPF_JMP_REG(BPF_JNE, BPF_REG_1, BPF_REG_3, 1), /* compare access type */ + BPF_JMP_A(PASS_JUMP_OFF), /* jump to PASS */ + }; + int r, access; + + assert(prog); + assert(acc); + + access = bpf_access_type(acc); + if (access <= 0) + return -EINVAL; + + insn[2].imm = access; + + r = bpf_program_add_instructions(prog, insn, ELEMENTSOF(insn)); + if (r < 0) + log_error_errno(r, "Extending device control BPF program failed: %m"); + + return r; +} + int cgroup_init_device_bpf(BPFProgram **ret, CGroupDevicePolicy policy, bool whitelist) { struct bpf_insn pre_insn[] = { /* load device type to r2 */ diff --git a/src/core/bpf-devices.h b/src/core/bpf-devices.h index f9a6eec028..8d3de3bd94 100644 --- a/src/core/bpf-devices.h +++ b/src/core/bpf-devices.h @@ -11,6 +11,7 @@ int bpf_devices_supported(void); int cgroup_bpf_whitelist_device(BPFProgram *p, int type, int major, int minor, const char *acc); int cgroup_bpf_whitelist_major(BPFProgram *p, int type, int major, const char *acc); +int cgroup_bpf_whitelist_class(BPFProgram *prog, int type, const char *acc); int cgroup_init_device_bpf(BPFProgram **ret, CGroupDevicePolicy policy, bool whitelist); int cgroup_apply_device_bpf(Unit *u, BPFProgram *p, CGroupDevicePolicy policy, bool whitelist); diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 8f3e646ad6..dd9b992ef1 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -509,21 +509,64 @@ static int whitelist_device(BPFProgram *prog, const char *path, const char *node static int whitelist_major(BPFProgram *prog, const char *path, const char *name, char type, const char *acc) { _cleanup_fclose_ FILE *f = NULL; - char *p, *w; + char buf[2+DECIMAL_STR_MAX(unsigned)+3+4]; bool good = false; + unsigned maj; int r; assert(path); assert(acc); assert(IN_SET(type, 'b', 'c')); + if (streq(name, "*")) { + /* If the name is a wildcard, then apply this list to all devices of this type */ + + if (cg_all_unified() > 0) { + if (!prog) + return 0; + + (void) cgroup_bpf_whitelist_class(prog, type == 'c' ? BPF_DEVCG_DEV_CHAR : BPF_DEVCG_DEV_BLOCK, acc); + } else { + xsprintf(buf, "%c *:* %s", type, acc); + + r = cg_set_attribute("devices", path, "devices.allow", buf); + if (r < 0) + log_full_errno(IN_SET(r, -ENOENT, -EROFS, -EINVAL, -EACCES) ? LOG_DEBUG : LOG_WARNING, r, + "Failed to set devices.allow on %s: %m", path); + return 0; + } + } + + if (safe_atou(name, &maj) >= 0 && DEVICE_MAJOR_VALID(maj)) { + /* The name is numeric and suitable as major. In that case, let's take is major, and create the entry + * directly */ + + if (cg_all_unified() > 0) { + if (!prog) + return 0; + + (void) cgroup_bpf_whitelist_major(prog, + type == 'c' ? BPF_DEVCG_DEV_CHAR : BPF_DEVCG_DEV_BLOCK, + maj, acc); + } else { + xsprintf(buf, "%c %u:* %s", type, maj, acc); + + r = cg_set_attribute("devices", path, "devices.allow", buf); + if (r < 0) + log_full_errno(IN_SET(r, -ENOENT, -EROFS, -EINVAL, -EACCES) ? LOG_DEBUG : LOG_WARNING, r, + "Failed to set devices.allow on %s: %m", path); + } + + return 0; + } + f = fopen("/proc/devices", "re"); if (!f) return log_warning_errno(errno, "Cannot open /proc/devices to resolve %s (%c): %m", name, type); for (;;) { _cleanup_free_ char *line = NULL; - unsigned maj; + char *w, *p; r = read_line(f, LONG_LINE_MAX, &line); if (r < 0) @@ -576,8 +619,6 @@ static int whitelist_major(BPFProgram *prog, const char *path, const char *name, type == 'c' ? BPF_DEVCG_DEV_CHAR : BPF_DEVCG_DEV_BLOCK, maj, acc); } else { - char buf[2+DECIMAL_STR_MAX(unsigned)+3+4]; - sprintf(buf, "%c %u:* %s", type, @@ -1179,7 +1220,7 @@ static void cgroup_context_apply( else if ((val = startswith(a->path, "char-"))) (void) whitelist_major(prog, path, val, 'c', acc); else - log_unit_debug(u, "Ignoring device %s while writing cgroup attribute.", a->path); + log_unit_debug(u, "Ignoring device '%s' while writing cgroup attribute.", a->path); } r = cgroup_apply_device_bpf(u, prog, c->device_policy, c->device_allow); From 846b3bd61e1d575b0b28f73c4d15385f94bb1662 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 29 Jun 2018 15:57:49 +0200 Subject: [PATCH 09/17] stat-util: add new APIs device_path_make_{major_minor|canonical}() and device_path_parse_major_minor() device_path_make_{major_minor|canonical) generate device node paths given a mode_t and a dev_t. We have similar code all over the place, let's unify this in one place. The former will generate a "/dev/char/" or "/dev/block" path, and never go to disk. The latter then goes to disk and resolves that path to the actual path of the device node. device_path_parse_major_minor() reverses device_path_make_major_minor(), also withozut going to disk. We have similar code doing something like this at various places, let's unify this in a single set of functions. This also allows us to teach them special tricks, for example handling of the /run/systemd/inaccessible/{blk|chr} device nodes, which we use for masking device nodes, and which do not exist in /dev/char/* and /dev/block/* --- src/basic/stat-util.c | 98 +++++++++++++++++++++++++++++++++++++++ src/basic/stat-util.h | 4 ++ src/core/cgroup.c | 52 +-------------------- src/test/test-stat-util.c | 40 ++++++++++++++++ 4 files changed, 144 insertions(+), 50 deletions(-) diff --git a/src/basic/stat-util.c b/src/basic/stat-util.c index 8b63eb360b..57700e2388 100644 --- a/src/basic/stat-util.c +++ b/src/basic/stat-util.c @@ -10,11 +10,13 @@ #include #include +#include "alloc-util.h" #include "dirent-util.h" #include "fd-util.h" #include "fs-util.h" #include "macro.h" #include "missing.h" +#include "parse-util.h" #include "stat-util.h" #include "string-util.h" @@ -319,3 +321,99 @@ int fd_verify_directory(int fd) { return stat_verify_directory(&st); } + +int device_path_make_major_minor(mode_t mode, dev_t devno, char **ret) { + const char *t; + + /* Generates the /dev/{char|block}/MAJOR:MINOR path for a dev_t */ + + if (S_ISCHR(mode)) + t = "char"; + else if (S_ISBLK(mode)) + t = "block"; + else + return -ENODEV; + + if (asprintf(ret, "/dev/%s/%u:%u", t, major(devno), minor(devno)) < 0) + return -ENOMEM; + + return 0; + +} + +int device_path_make_canonical(mode_t mode, dev_t devno, char **ret) { + _cleanup_free_ char *p = NULL; + int r; + + /* Finds the canonical path for a device, i.e. resolves the /dev/{char|block}/MAJOR:MINOR path to the end. */ + + assert(ret); + + if (major(devno) == 0 && minor(devno) == 0) { + char *s; + + /* A special hack to make sure our 'inaccessible' device nodes work. They won't have symlinks in + * /dev/block/ and /dev/char/, hence we handle them specially here. */ + + if (S_ISCHR(mode)) + s = strdup("/run/systemd/inaccessible/chr"); + else if (S_ISBLK(mode)) + s = strdup("/run/systemd/inaccessible/blk"); + else + return -ENODEV; + + if (!s) + return -ENOMEM; + + *ret = s; + return 0; + } + + r = device_path_make_major_minor(mode, devno, &p); + if (r < 0) + return r; + + return chase_symlinks(p, NULL, 0, ret); +} + +int device_path_parse_major_minor(const char *path, mode_t *ret_mode, dev_t *ret_devno) { + mode_t mode; + dev_t devno; + int r; + + /* Tries to extract the major/minor directly from the device path if we can. Handles /dev/block/ and /dev/char/ + * paths, as well out synthetic inaccessible device nodes. Never goes to disk. Returns -ENODEV if the device + * path cannot be parsed like this. */ + + if (path_equal(path, "/run/systemd/inaccessible/chr")) { + mode = S_IFCHR; + devno = makedev(0, 0); + } else if (path_equal(path, "/run/systemd/inaccessible/blk")) { + mode = S_IFBLK; + devno = makedev(0, 0); + } else { + const char *w; + + w = path_startswith(path, "/dev/block/"); + if (w) + mode = S_IFBLK; + else { + w = path_startswith(path, "/dev/char/"); + if (!w) + return -ENODEV; + + mode = S_IFCHR; + } + + r = parse_dev(w, &devno); + if (r < 0) + return r; + } + + if (ret_mode) + *ret_mode = mode; + if (ret_devno) + *ret_devno = devno; + + return 0; +} diff --git a/src/basic/stat-util.h b/src/basic/stat-util.h index fe4a4bb717..0a08e642b5 100644 --- a/src/basic/stat-util.h +++ b/src/basic/stat-util.h @@ -81,3 +81,7 @@ int fd_verify_directory(int fd); typeof(x) _x = (x), _y = 0; \ _x >= _y && _x < (UINT32_C(1) << 20); \ }) + +int device_path_make_major_minor(mode_t mode, dev_t devno, char **ret); +int device_path_make_canonical(mode_t mode, dev_t devno, char **ret); +int device_path_parse_major_minor(const char *path, mode_t *ret_mode, dev_t *ret_devno); diff --git a/src/core/cgroup.c b/src/core/cgroup.c index dd9b992ef1..72af5e855f 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -408,56 +408,8 @@ static int lookup_block_device(const char *p, dev_t *ret) { return 0; } -static int shortcut_special_device_path(const char *p, struct stat *ret) { - const char *w; - mode_t mode; - dev_t devt; - int r; - - assert(p); - assert(ret); - - if (path_equal(p, "/run/systemd/inaccessible/chr")) { - *ret = (struct stat) { - .st_mode = S_IFCHR, - .st_rdev = makedev(0, 0), - }; - return 0; - } - - if (path_equal(p, "/run/systemd/inaccessible/blk")) { - *ret = (struct stat) { - .st_mode = S_IFBLK, - .st_rdev = makedev(0, 0), - }; - return 0; - } - - w = path_startswith(p, "/dev/block/"); - if (w) - mode = S_IFBLK; - else { - w = path_startswith(p, "/dev/char/"); - if (!w) - return -ENODEV; - - mode = S_IFCHR; - } - - r = parse_dev(w, &devt); - if (r < 0) - return r; - - *ret = (struct stat) { - .st_mode = mode, - .st_rdev = devt, - }; - - return 0; -} - static int whitelist_device(BPFProgram *prog, const char *path, const char *node, const char *acc) { - struct stat st; + struct stat st = {}; int r; assert(path); @@ -466,7 +418,7 @@ static int whitelist_device(BPFProgram *prog, const char *path, const char *node /* Some special handling for /dev/block/%u:%u, /dev/char/%u:%u, /run/systemd/inaccessible/chr and * /run/systemd/inaccessible/blk paths. Instead of stat()ing these we parse out the major/minor directly. This * means clients can use these path without the device node actually around */ - r = shortcut_special_device_path(node, &st); + r = device_path_parse_major_minor(node, &st.st_mode, &st.st_rdev); if (r < 0) { if (r != -ENODEV) return log_warning_errno(r, "Couldn't parse major/minor from device path '%s': %m", node); diff --git a/src/test/test-stat-util.c b/src/test/test-stat-util.c index 713fbc9a08..4201edac97 100644 --- a/src/test/test-stat-util.c +++ b/src/test/test-stat-util.c @@ -11,6 +11,7 @@ #include "missing.h" #include "mount-util.h" #include "stat-util.h" +#include "path-util.h" static void test_files_same(void) { _cleanup_close_ int fd = -1; @@ -116,6 +117,44 @@ static void test_device_major_minor_valid(void) { assert_se(DEVICE_MINOR_VALID(minor(0))); } +static void test_device_path_make_canonical_one(const char *path) { + _cleanup_free_ char *resolved = NULL, *raw = NULL; + struct stat st; + dev_t devno; + mode_t mode; + int r; + + assert_se(stat(path, &st) >= 0); + r = device_path_make_canonical(st.st_mode, st.st_rdev, &resolved); + if (r == -ENOENT) /* maybe /dev/char/x:y and /dev/block/x:y are missing in this test environment, because we + * run in a container or so? */ + return; + + assert_se(r >= 0); + assert_se(path_equal(path, resolved)); + + assert_se(device_path_make_major_minor(st.st_mode, st.st_rdev, &raw) >= 0); + assert_se(device_path_parse_major_minor(raw, &mode, &devno) >= 0); + + assert_se(st.st_rdev == devno); + assert_se((st.st_mode & S_IFMT) == (mode & S_IFMT)); +} + +static void test_device_path_make_canonical(void) { + + test_device_path_make_canonical_one("/dev/null"); + test_device_path_make_canonical_one("/dev/zero"); + test_device_path_make_canonical_one("/dev/full"); + test_device_path_make_canonical_one("/dev/random"); + test_device_path_make_canonical_one("/dev/urandom"); + test_device_path_make_canonical_one("/dev/tty"); + + if (is_device_node("/run/systemd/inaccessible/chr") > 0) { + test_device_path_make_canonical_one("/run/systemd/inaccessible/chr"); + test_device_path_make_canonical_one("/run/systemd/inaccessible/blk"); + } +} + int main(int argc, char *argv[]) { test_files_same(); test_is_symlink(); @@ -123,6 +162,7 @@ int main(int argc, char *argv[]) { test_path_is_temporary_fs(); test_fd_is_network_ns(); test_device_major_minor_valid(); + test_device_path_make_canonical(); return 0; } From 54b22b2643de50945aaaf720a444ea4467ddb0db Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 29 Jun 2018 16:49:23 +0200 Subject: [PATCH 10/17] tree-wide: port various parts of the code over to the new device_major_minor_path() calls --- src/basic/terminal-util.c | 73 ++++++++++++++++++++------------------ src/partition/growfs.c | 36 +++++++++++++------ src/shared/bootspec.c | 8 +++-- src/shared/dissect-image.c | 5 +-- 4 files changed, 71 insertions(+), 51 deletions(-) diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index a5e4de00b0..ceba7d0ff8 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -979,53 +979,56 @@ int get_ctty_devnr(pid_t pid, dev_t *d) { return 0; } -int get_ctty(pid_t pid, dev_t *_devnr, char **r) { - char fn[STRLEN("/dev/char/") + 2*DECIMAL_STR_MAX(unsigned) + 1 + 1], *b = NULL; - _cleanup_free_ char *s = NULL; - const char *p; +int get_ctty(pid_t pid, dev_t *ret_devnr, char **ret) { + _cleanup_free_ char *fn = NULL, *b = NULL; dev_t devnr; - int k; + int r; - assert(r); + r = get_ctty_devnr(pid, &devnr); + if (r < 0) + return r; - k = get_ctty_devnr(pid, &devnr); - if (k < 0) - return k; + r = device_path_make_canonical(S_IFCHR, devnr, &fn); + if (r < 0) { + if (r != -ENOENT) /* No symlink for this in /dev/char/? */ + return r; - sprintf(fn, "/dev/char/%u:%u", major(devnr), minor(devnr)); - - k = readlink_malloc(fn, &s); - if (k < 0) { - - if (k != -ENOENT) - return k; - - /* This is an ugly hack */ if (major(devnr) == 136) { + /* This is an ugly hack: PTY devices are not listed in /dev/char/, as they don't follow the + * Linux device model. This means we have no nice way to match them up against their actual + * device node. Let's hence do the check by the fixed, assigned major number. Normally we try + * to avoid such fixed major/minor matches, but there appears to nother nice way to handle + * this. */ + if (asprintf(&b, "pts/%u", minor(devnr)) < 0) return -ENOMEM; } else { - /* Probably something like the ptys which have no - * symlink in /dev/char. Let's return something - * vaguely useful. */ + /* Probably something similar to the ptys which have no symlink in /dev/char/. Let's return + * something vaguely useful. */ - b = strdup(fn + 5); - if (!b) - return -ENOMEM; + r = device_path_make_major_minor(S_IFCHR, devnr, &fn); + if (r < 0) + return r; } - } else { - p = PATH_STARTSWITH_SET(s, "/dev/", "../"); - if (!p) - p = s; - - b = strdup(p); - if (!b) - return -ENOMEM; } - *r = b; - if (_devnr) - *_devnr = devnr; + if (!b) { + const char *w; + + w = path_startswith(fn, "/dev/"); + if (w) { + b = strdup(w); + if (!b) + return -ENOMEM; + } else + b = TAKE_PTR(fn); + } + + if (ret) + *ret = TAKE_PTR(b); + + if (ret_devnr) + *ret_devnr = devnr; return 0; } diff --git a/src/partition/growfs.c b/src/partition/growfs.c index 8e04eb3c23..b8dc9f2e93 100644 --- a/src/partition/growfs.c +++ b/src/partition/growfs.c @@ -23,6 +23,7 @@ #include "parse-util.h" #include "path-util.h" #include "pretty-print.h" +#include "stat-util.h" #include "strv.h" static const char *arg_target = NULL; @@ -69,13 +70,16 @@ static int resize_btrfs(const char *path, int mountfd, int devfd, uint64_t numbl #if HAVE_LIBCRYPTSETUP static int resize_crypt_luks_device(dev_t devno, const char *fstype, dev_t main_devno) { - char devpath[DEV_NUM_PATH_MAX], main_devpath[DEV_NUM_PATH_MAX]; - _cleanup_close_ int main_devfd = -1; + _cleanup_free_ char *devpath = NULL, *main_devpath = NULL; _cleanup_(crypt_freep) struct crypt_device *cd = NULL; + _cleanup_close_ int main_devfd = -1; uint64_t size; int r; - xsprintf_dev_num_path(main_devpath, "block", main_devno); + r = device_path_make_major_minor(S_IFBLK, main_devno, &main_devpath); + if (r < 0) + return log_error_errno(r, "Failed to format device major/minor path: %m"); + main_devfd = open(main_devpath, O_RDONLY|O_CLOEXEC); if (main_devfd < 0) return log_error_errno(errno, "Failed to open \"%s\": %m", main_devpath); @@ -85,8 +89,10 @@ static int resize_crypt_luks_device(dev_t devno, const char *fstype, dev_t main_ main_devpath); log_debug("%s is %"PRIu64" bytes", main_devpath, size); + r = device_path_make_major_minor(S_IFBLK, devno, &devpath); + if (r < 0) + return log_error_errno(r, "Failed to format major/minor path: %m"); - xsprintf_dev_num_path(devpath, "block", devno); r = crypt_init(&cd, devpath); if (r < 0) return log_error_errno(r, "crypt_init(\"%s\") failed: %m", devpath); @@ -115,9 +121,8 @@ static int resize_crypt_luks_device(dev_t devno, const char *fstype, dev_t main_ #endif static int maybe_resize_slave_device(const char *mountpath, dev_t main_devno) { + _cleanup_free_ char *fstype = NULL, *devpath = NULL; dev_t devno; - char devpath[DEV_NUM_PATH_MAX]; - _cleanup_free_ char *fstype = NULL; int r; #if HAVE_LIBCRYPTSETUP @@ -137,7 +142,10 @@ static int maybe_resize_slave_device(const char *mountpath, dev_t main_devno) { if (devno == main_devno) return 0; - xsprintf_dev_num_path(devpath, "block", devno); + r = device_path_make_major_minor(S_IFBLK, devno, &devpath); + if (r < 0) + return log_error_errno(r, "Failed to format device major/minor path: %m"); + r = probe_filesystem(devpath, &fstype); if (r == -EUCLEAN) return log_warning_errno(r, "Cannot reliably determine probe \"%s\", refusing to proceed.", devpath); @@ -222,12 +230,13 @@ static int parse_argv(int argc, char *argv[]) { } int main(int argc, char *argv[]) { - dev_t devno; _cleanup_close_ int mountfd = -1, devfd = -1; - int blocksize; + _cleanup_free_ char *devpath = NULL; uint64_t size, numblocks; - char devpath[DEV_NUM_PATH_MAX], fb[FORMAT_BYTES_MAX]; + char fb[FORMAT_BYTES_MAX]; struct statfs sfs; + dev_t devno; + int blocksize; int r; log_setup_service(); @@ -264,7 +273,12 @@ int main(int argc, char *argv[]) { return EXIT_FAILURE; } - xsprintf_dev_num_path(devpath, "block", devno); + r = device_path_make_major_minor(S_IFBLK, devno, &devpath); + if (r < 0) { + log_error_errno(r, "Failed to format device major/minor path: %m"); + return EXIT_FAILURE; + } + devfd = open(devpath, O_RDONLY|O_CLOEXEC); if (devfd < 0) { log_error_errno(errno, "Failed to open \"%s\": %m", devpath); diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index db4fd411e2..7e276f1edf 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -405,7 +405,7 @@ static int verify_esp( sd_id128_t *ret_uuid) { #if HAVE_BLKID _cleanup_(blkid_free_probep) blkid_probe b = NULL; - char t[DEV_NUM_PATH_MAX]; + _cleanup_free_ char *node = NULL; const char *v; #endif uint64_t pstart = 0, psize = 0; @@ -469,9 +469,11 @@ static int verify_esp( goto finish; #if HAVE_BLKID - xsprintf_dev_num_path(t, "block", st.st_dev); + r = device_path_make_major_minor(S_IFBLK, st.st_dev, &node); + if (r < 0) + return log_error_errno(r, "Failed to format major/minor device path: %m"); errno = 0; - b = blkid_new_probe_from_filename(t); + b = blkid_new_probe_from_filename(node); if (!b) return log_error_errno(errno ?: ENOMEM, "Failed to open file system \"%s\": %m", p); diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index 18620a3b19..01b06dbc0b 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -219,8 +219,9 @@ int dissect_image( return -ENOMEM; } - if (asprintf(&n, "/dev/block/%u:%u", major(st.st_rdev), minor(st.st_rdev)) < 0) - return -ENOMEM; + r = device_path_make_major_minor(st.st_mode, st.st_rdev, &n); + if (r < 0) + return r; m->partitions[PARTITION_ROOT] = (DissectedPartition) { .found = true, From d5aecba6e0b7c73657c4cf544ce57289115098e7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 2 Jul 2018 18:20:03 +0200 Subject: [PATCH 11/17] cgroup: use device_path_parse_major_minor() also for block device paths Not only when we populate the "devices" cgroup controller we need major/minor numbers, but for the io/blkio one it's the same, hence let's use the same logic for both. --- src/core/cgroup.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 72af5e855f..11f9611b71 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -376,16 +376,23 @@ int cgroup_add_device_allow(CGroupContext *c, const char *dev, const char *mode) } static int lookup_block_device(const char *p, dev_t *ret) { - struct stat st; + struct stat st = {}; int r; assert(p); assert(ret); - if (stat(p, &st) < 0) - return log_warning_errno(errno, "Couldn't stat device '%s': %m", p); + r = device_path_parse_major_minor(p, &st.st_mode, &st.st_rdev); + if (r == -ENODEV) { /* not a parsable device node, need to go to disk */ + if (stat(p, &st) < 0) + return log_warning_errno(errno, "Couldn't stat device '%s': %m", p); + } else if (r < 0) + return log_warning_errno(r, "Failed to parse major/minor from path '%s': %m", p); - if (S_ISBLK(st.st_mode)) + if (S_ISCHR(st.st_mode)) { + log_warning("Device node '%s' is a character device, but block device needed.", p); + return -ENOTBLK; + } else if (S_ISBLK(st.st_mode)) *ret = st.st_rdev; else if (major(st.st_dev) != 0) *ret = st.st_dev; /* If this is not a device node then use the block device this file is stored on */ From d7391698042b87aa8dc05627fa9838fb5ec7abc0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 24 Jul 2018 17:00:58 +0200 Subject: [PATCH 12/17] capability: add new type for maintaining all five cap sets as one --- src/basic/capability-util.c | 125 ++++++++++++++++++++++++++++++++++++ src/basic/capability-util.h | 24 +++++++ 2 files changed, 149 insertions(+) diff --git a/src/basic/capability-util.c b/src/basic/capability-util.c index 6ae35e078b..a3f3ca9f52 100644 --- a/src/basic/capability-util.c +++ b/src/basic/capability-util.c @@ -359,3 +359,128 @@ bool ambient_capabilities_supported(void) { return cache; } + +int capability_quintet_enforce(const CapabilityQuintet *q) { + _cleanup_cap_free_ cap_t c = NULL; + int r; + + if (q->ambient != (uint64_t) -1) { + unsigned long i; + bool changed = false; + + c = cap_get_proc(); + if (!c) + return -errno; + + /* In order to raise the ambient caps set we first need to raise the matching inheritable + permitted + * cap */ + for (i = 0; i <= cap_last_cap(); i++) { + uint64_t m = UINT64_C(1) << i; + cap_value_t cv = (cap_value_t) i; + cap_flag_value_t old_value_inheritable, old_value_permitted; + + if ((q->ambient & m) == 0) + continue; + + if (cap_get_flag(c, cv, CAP_INHERITABLE, &old_value_inheritable) < 0) + return -errno; + if (cap_get_flag(c, cv, CAP_PERMITTED, &old_value_permitted) < 0) + return -errno; + + if (old_value_inheritable == CAP_SET && old_value_permitted == CAP_SET) + continue; + + if (cap_set_flag(c, CAP_INHERITABLE, 1, &cv, CAP_SET) < 0) + return -errno; + + if (cap_set_flag(c, CAP_PERMITTED, 1, &cv, CAP_SET) < 0) + return -errno; + + changed = true; + } + + if (changed) + if (cap_set_proc(c) < 0) + return -errno; + + r = capability_ambient_set_apply(q->ambient, false); + if (r < 0) + return r; + } + + if (q->inheritable != (uint64_t) -1 || q->permitted != (uint64_t) -1 || q->effective != (uint64_t) -1) { + bool changed = false; + unsigned long i; + + if (!c) { + c = cap_get_proc(); + if (!c) + return -errno; + } + + for (i = 0; i <= cap_last_cap(); i++) { + uint64_t m = UINT64_C(1) << i; + cap_value_t cv = (cap_value_t) i; + + if (q->inheritable != (uint64_t) -1) { + cap_flag_value_t old_value, new_value; + + if (cap_get_flag(c, cv, CAP_INHERITABLE, &old_value) < 0) + return -errno; + + new_value = (q->inheritable & m) ? CAP_SET : CAP_CLEAR; + + if (old_value != new_value) { + changed = true; + + if (cap_set_flag(c, CAP_INHERITABLE, 1, &cv, new_value) < 0) + return -errno; + } + } + + if (q->permitted != (uint64_t) -1) { + cap_flag_value_t old_value, new_value; + + if (cap_get_flag(c, cv, CAP_PERMITTED, &old_value) < 0) + return -errno; + + new_value = (q->permitted & m) ? CAP_SET : CAP_CLEAR; + + if (old_value != new_value) { + changed = true; + + if (cap_set_flag(c, CAP_PERMITTED, 1, &cv, new_value) < 0) + return -errno; + } + } + + if (q->effective != (uint64_t) -1) { + cap_flag_value_t old_value, new_value; + + if (cap_get_flag(c, cv, CAP_EFFECTIVE, &old_value) < 0) + return -errno; + + new_value = (q->effective & m) ? CAP_SET : CAP_CLEAR; + + if (old_value != new_value) { + changed = true; + + if (cap_set_flag(c, CAP_EFFECTIVE, 1, &cv, new_value) < 0) + return -errno; + } + } + } + + if (changed) + if (cap_set_proc(c) < 0) + return -errno; + } + + if (q->bounding != (uint64_t) -1) { + r = capability_bounding_set_drop(q->bounding, false); + if (r < 0) + return r; + } + + return 0; +} diff --git a/src/basic/capability-util.h b/src/basic/capability-util.h index 59591d4b52..e0e0b1c0fa 100644 --- a/src/basic/capability-util.h +++ b/src/basic/capability-util.h @@ -43,3 +43,27 @@ bool ambient_capabilities_supported(void); /* Identical to linux/capability.h's CAP_TO_MASK(), but uses an unsigned 1U instead of a signed 1 for shifting left, in * order to avoid complaints about shifting a signed int left by 31 bits, which would make it negative. */ #define CAP_TO_MASK_CORRECTED(x) (1U << ((x) & 31U)) + +typedef struct CapabilityQuintet { + /* Stores all five types of capabilities in one go. Note that we use (uint64_t) -1 for unset here. This hence + * needs to be updated as soon as Linux learns more than 63 caps. */ + uint64_t effective; + uint64_t bounding; + uint64_t inheritable; + uint64_t permitted; + uint64_t ambient; +} CapabilityQuintet; + +assert_cc(CAP_LAST_CAP < 64); + +#define CAPABILITY_QUINTET_NULL { (uint64_t) -1, (uint64_t) -1, (uint64_t) -1, (uint64_t) -1, (uint64_t) -1 } + +static inline bool capability_quintet_is_set(const CapabilityQuintet *q) { + return q->effective != (uint64_t) -1 || + q->bounding != (uint64_t) -1 || + q->inheritable != (uint64_t) -1 || + q->permitted != (uint64_t) -1 || + q->ambient != (uint64_t) -1; +} + +int capability_quintet_enforce(const CapabilityQuintet *q); From de321f5228291dbd40ee07aa792f2410c16d1fca Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 24 Jul 2018 17:12:27 +0200 Subject: [PATCH 13/17] fs-util: beef up chmod_and_chown() a bit --- src/basic/fs-util.c | 50 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 10 deletions(-) diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index 55651baa80..94efca08ca 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -211,31 +211,62 @@ int readlink_and_make_absolute(const char *p, char **r) { } int chmod_and_chown(const char *path, mode_t mode, uid_t uid, gid_t gid) { + char fd_path[STRLEN("/proc/self/fd/") + DECIMAL_STR_MAX(int) + 1]; + _cleanup_close_ int fd = -1; assert(path); - /* Under the assumption that we are running privileged we - * first change the access mode and only then hand out + /* Under the assumption that we are running privileged we first change the access mode and only then hand out * ownership to avoid a window where access is too open. */ - if (mode != MODE_INVALID) - if (chmod(path, mode) < 0) + fd = open(path, O_PATH|O_CLOEXEC|O_NOFOLLOW); /* Let's acquire an O_PATH fd, as precaution to change mode/owner + * on the same file */ + if (fd < 0) + return -errno; + + xsprintf(fd_path, "/proc/self/fd/%i", fd); + + if (mode != MODE_INVALID) { + + if ((mode & S_IFMT) != 0) { + struct stat st; + + if (stat(fd_path, &st) < 0) + return -errno; + + if ((mode & S_IFMT) != (st.st_mode & S_IFMT)) + return -EINVAL; + } + + if (chmod(fd_path, mode & 07777) < 0) return -errno; + } if (uid != UID_INVALID || gid != GID_INVALID) - if (chown(path, uid, gid) < 0) + if (chown(fd_path, uid, gid) < 0) return -errno; return 0; } int fchmod_and_chown(int fd, mode_t mode, uid_t uid, gid_t gid) { - /* Under the assumption that we are running privileged we - * first change the access mode and only then hand out + /* Under the assumption that we are running privileged we first change the access mode and only then hand out * ownership to avoid a window where access is too open. */ - if (mode != MODE_INVALID) - if (fchmod(fd, mode) < 0) + if (mode != MODE_INVALID) { + + if ((mode & S_IFMT) != 0) { + struct stat st; + + if (fstat(fd, &st) < 0) + return -errno; + + if ((mode & S_IFMT) != (st.st_mode & S_IFMT)) + return -EINVAL; + } + + if (fchmod(fd, mode & 0777) < 0) return -errno; + } if (uid != UID_INVALID || gid != GID_INVALID) if (fchown(fd, uid, gid) < 0) @@ -263,7 +294,6 @@ int fchmod_opath(int fd, mode_t m) { * fchownat() does. */ xsprintf(procfs_path, "/proc/self/fd/%i", fd); - if (chmod(procfs_path, m) < 0) return -errno; From d435a18244ff52132d873859126e7a3ece66a9ba Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 24 Jul 2018 17:15:33 +0200 Subject: [PATCH 14/17] ptyfwd: optionally override terminal width/height --- src/shared/ptyfwd.c | 46 +++++++++++++++++++++++++++++++++++++++++++-- src/shared/ptyfwd.h | 2 ++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/src/shared/ptyfwd.c b/src/shared/ptyfwd.c index 1cb7ea3a19..fe17b3781a 100644 --- a/src/shared/ptyfwd.c +++ b/src/shared/ptyfwd.c @@ -392,11 +392,14 @@ int pty_forward_new( struct winsize ws; int r; - f = new0(PTYForward, 1); + f = new(PTYForward, 1); if (!f) return -ENOMEM; - f->flags = flags; + *f = (struct PTYForward) { + .flags = flags, + .master = -1, + }; if (event) f->event = sd_event_ref(event); @@ -587,3 +590,42 @@ int pty_forward_set_priority(PTYForward *f, int64_t priority) { return 0; } + +int pty_forward_set_width_height(PTYForward *f, unsigned width, unsigned height) { + struct winsize ws; + + assert(f); + + if (width == (unsigned) -1 && height == (unsigned) -1) + return 0; /* noop */ + + if (width != (unsigned) -1 && + (width == 0 || width > USHRT_MAX)) + return -ERANGE; + + if (height != (unsigned) -1 && + (height == 0 || height > USHRT_MAX)) + return -ERANGE; + + if (width == (unsigned) -1 || height == (unsigned) -1) { + if (ioctl(f->master, TIOCGWINSZ, &ws) < 0) + return -errno; + + if (width != (unsigned) -1) + ws.ws_col = width; + if (height != (unsigned) -1) + ws.ws_row = height; + } else + ws = (struct winsize) { + .ws_row = height, + .ws_col = width, + }; + + if (ioctl(f->master, TIOCSWINSZ, &ws) < 0) + return -errno; + + /* Make sure we ignore SIGWINCH window size events from now on */ + f->sigwinch_event_source = sd_event_source_unref(f->sigwinch_event_source); + + return 0; +} diff --git a/src/shared/ptyfwd.h b/src/shared/ptyfwd.h index e4a083ac24..4ebddcd720 100644 --- a/src/shared/ptyfwd.h +++ b/src/shared/ptyfwd.h @@ -37,4 +37,6 @@ bool pty_forward_drain(PTYForward *f); int pty_forward_set_priority(PTYForward *f, int64_t priority); +int pty_forward_set_width_height(PTYForward *f, unsigned width, unsigned height); + DEFINE_TRIVIAL_CLEANUP_FUNC(PTYForward*, pty_forward_free); From 17c58ba97b1cdacf28a0135522081d562628385a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 26 Jul 2018 17:24:51 +0200 Subject: [PATCH 15/17] nspawn: let's also pre-mount /dev/mqueue --- src/nspawn/nspawn-mount.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/nspawn/nspawn-mount.c b/src/nspawn/nspawn-mount.c index 48187079b3..27e123d254 100644 --- a/src/nspawn/nspawn-mount.c +++ b/src/nspawn/nspawn-mount.c @@ -556,6 +556,8 @@ int mount_all(const char *dest, MOUNT_FATAL }, { "tmpfs", "/run", "tmpfs", "mode=755", MS_NOSUID|MS_NODEV|MS_STRICTATIME, MOUNT_FATAL }, + { "mqueue", "/dev/mqueue", "mqueue", NULL, 0, + MOUNT_FATAL }, #if HAVE_SELINUX { "/sys/fs/selinux", "/sys/fs/selinux", NULL, NULL, MS_BIND, From 30874dda3a66c0639773dd23079662fc4bf53afd Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 27 Jul 2018 18:04:11 +0200 Subject: [PATCH 16/17] dev-setup: generalize logic we use to create "inaccessible" device nodes Let's generalize this, so that we can use this in nspawn later on, which is pretty useful as we need to be able to mask files from the inner child of nspawn too, where the host's /run/systemd/inaccessible directory is not visible anymore. Moreover, if nspawn can create these nodes on its own before the payload this means the payload can run with fewer privileges. --- src/core/mount-setup.c | 17 ++--------- src/shared/dev-setup.c | 59 +++++++++++++++++++++++++++++++++++++ src/shared/dev-setup.h | 2 ++ src/test/meson.build | 4 +++ src/test/test-dev-setup.c | 62 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 130 insertions(+), 14 deletions(-) create mode 100644 src/test/test-dev-setup.c diff --git a/src/core/mount-setup.c b/src/core/mount-setup.c index e15d94d98a..4c17395774 100644 --- a/src/core/mount-setup.c +++ b/src/core/mount-setup.c @@ -460,20 +460,9 @@ int mount_setup(bool loaded_policy) { (void) mkdir_label("/run/systemd", 0755); (void) mkdir_label("/run/systemd/system", 0755); - /* Set up inaccessible (and empty) file nodes of all types */ - (void) mkdir_label("/run/systemd/inaccessible", 0000); - (void) mknod("/run/systemd/inaccessible/reg", S_IFREG | 0000, 0); - (void) mkdir_label("/run/systemd/inaccessible/dir", 0000); - (void) mkfifo("/run/systemd/inaccessible/fifo", 0000); - (void) mknod("/run/systemd/inaccessible/sock", S_IFSOCK | 0000, 0); - - /* The following two are likely to fail if we lack the privs for it (for example in an userns environment, if - * CAP_SYS_MKNOD is missing, or if a device node policy prohibit major/minor of 0 device nodes to be - * created). But that's entirely fine. Consumers of these files should carry fallback to use a different node - * then, for example /run/systemd/inaccessible/sock, which is close enough in behaviour and semantics for most - * uses. */ - (void) mknod("/run/systemd/inaccessible/chr", S_IFCHR | 0000, makedev(0, 0)); - (void) mknod("/run/systemd/inaccessible/blk", S_IFBLK | 0000, makedev(0, 0)); + /* Also create /run/systemd/inaccessible nodes, so that we always have something to mount inaccessible nodes + * from. */ + (void) make_inaccessible_nodes(NULL, UID_INVALID, GID_INVALID); return 0; } diff --git a/src/shared/dev-setup.c b/src/shared/dev-setup.c index d117fbfda0..b545c2a1c0 100644 --- a/src/shared/dev-setup.c +++ b/src/shared/dev-setup.c @@ -9,6 +9,7 @@ #include "label.h" #include "log.h" #include "path-util.h" +#include "umask-util.h" #include "user-util.h" #include "util.h" @@ -54,3 +55,61 @@ int dev_setup(const char *prefix, uid_t uid, gid_t gid) { return 0; } + +int make_inaccessible_nodes(const char *root, uid_t uid, gid_t gid) { + static const struct { + const char *name; + mode_t mode; + } table[] = { + { "/run/systemd", S_IFDIR | 0755 }, + { "/run/systemd/inaccessible", S_IFDIR | 0000 }, + { "/run/systemd/inaccessible/reg", S_IFREG | 0000 }, + { "/run/systemd/inaccessible/dir", S_IFDIR | 0000 }, + { "/run/systemd/inaccessible/fifo", S_IFIFO | 0000 }, + { "/run/systemd/inaccessible/sock", S_IFSOCK | 0000 }, + + /* The following two are likely to fail if we lack the privs for it (for example in an userns + * environment, if CAP_SYS_MKNOD is missing, or if a device node policy prohibit major/minor of 0 + * device nodes to be created). But that's entirely fine. Consumers of these files should carry + * fallback to use a different node then, for example /run/systemd/inaccessible/sock, which is close + * enough in behaviour and semantics for most uses. */ + { "/run/systemd/inaccessible/chr", S_IFCHR | 0000 }, + { "/run/systemd/inaccessible/blk", S_IFBLK | 0000 }, + }; + + _cleanup_umask_ mode_t u; + size_t i; + int r; + + u = umask(0000); + + /* Set up inaccessible (and empty) file nodes of all types. This are used to as mount sources for over-mounting + * ("masking") file nodes that shall become inaccessible and empty for specific containers or services. We try + * to lock down these nodes as much as we can, but otherwise try to match them as closely as possible with the + * underlying file, i.e. in the best case we offer the same node type as the underlying node. */ + + for (i = 0; i < ELEMENTSOF(table); i++) { + _cleanup_free_ char *path = NULL; + + path = prefix_root(root, table[i].name); + if (!path) + return log_oom(); + + if (S_ISDIR(table[i].mode)) + r = mkdir(path, table[i].mode & 07777); + else + r = mknod(path, table[i].mode, makedev(0, 0)); + if (r < 0) { + if (errno != EEXIST) + log_debug_errno(errno, "Failed to create '%s', ignoring: %m", path); + continue; + } + + if (uid != UID_INVALID || gid != GID_INVALID) { + if (lchown(path, uid, gid) < 0) + log_debug_errno(errno, "Failed to chown '%s': %m", path); + } + } + + return 0; +} diff --git a/src/shared/dev-setup.h b/src/shared/dev-setup.h index f105f2f20f..72b90ec4de 100644 --- a/src/shared/dev-setup.h +++ b/src/shared/dev-setup.h @@ -4,3 +4,5 @@ #include int dev_setup(const char *prefix, uid_t uid, gid_t gid); + +int make_inaccessible_nodes(const char *root, uid_t uid, gid_t gid); diff --git a/src/test/meson.build b/src/test/meson.build index ade905733e..2635456a4f 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -156,6 +156,10 @@ tests += [ [], []], + [['src/test/test-dev-setup.c'], + [], + []], + [['src/test/test-capability.c'], [], [libcap]], diff --git a/src/test/test-dev-setup.c b/src/test/test-dev-setup.c new file mode 100644 index 0000000000..523cfe43b1 --- /dev/null +++ b/src/test/test-dev-setup.c @@ -0,0 +1,62 @@ +/* SPDX-License-Identifier: LGPL-2.1+ */ + +#include "capability-util.h" +#include "dev-setup.h" +#include "fileio.h" +#include "fs-util.h" +#include "path-util.h" +#include "rm-rf.h" + +int main(int argc, char *argv[]) { + _cleanup_(rm_rf_physical_and_freep) char *p = NULL; + const char *f; + struct stat st; + + if (have_effective_cap(CAP_DAC_OVERRIDE) <= 0) + return EXIT_TEST_SKIP; + + assert_se(mkdtemp_malloc("/tmp/test-dev-setupXXXXXX", &p) >= 0); + + f = prefix_roota(p, "/run"); + assert_se(mkdir(f, 0755) >= 0); + + assert_se(make_inaccessible_nodes(p, 1, 1) >= 0); + + f = prefix_roota(p, "/run/systemd/inaccessible/reg"); + assert_se(stat(f, &st) >= 0); + assert_se(S_ISREG(st.st_mode)); + assert_se((st.st_mode & 07777) == 0000); + + f = prefix_roota(p, "/run/systemd/inaccessible/dir"); + assert_se(stat(f, &st) >= 0); + assert_se(S_ISDIR(st.st_mode)); + assert_se((st.st_mode & 07777) == 0000); + + f = prefix_roota(p, "/run/systemd/inaccessible/fifo"); + assert_se(stat(f, &st) >= 0); + assert_se(S_ISFIFO(st.st_mode)); + assert_se((st.st_mode & 07777) == 0000); + + f = prefix_roota(p, "/run/systemd/inaccessible/sock"); + assert_se(stat(f, &st) >= 0); + assert_se(S_ISSOCK(st.st_mode)); + assert_se((st.st_mode & 07777) == 0000); + + f = prefix_roota(p, "/run/systemd/inaccessible/chr"); + if (stat(f, &st) < 0) + assert_se(errno == ENOENT); + else { + assert_se(S_ISCHR(st.st_mode)); + assert_se((st.st_mode & 07777) == 0000); + } + + f = prefix_roota(p, "/run/systemd/inaccessible/blk"); + if (stat(f, &st) < 0) + assert_se(errno == ENOENT); + else { + assert_se(S_ISBLK(st.st_mode)); + assert_se((st.st_mode & 07777) == 0000); + } + + return EXIT_SUCCESS; +} From 8f2eb7302dc98794247d49c85d8a366d89218aee Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 2 Jul 2018 18:22:07 +0200 Subject: [PATCH 17/17] update TODO --- TODO | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/TODO b/TODO index 41b057033b..d82cfbda67 100644 --- a/TODO +++ b/TODO @@ -95,6 +95,29 @@ Features: * bootspec.c: also enumerate EFI unified kernel images. +* maybe set a special xattr on cgroups that have delegate=yes set, to make it + easy to mark cut points + +* introduce an option (or replacement) for "systemctl show" that outputs all + properties as JSON, similar to busctl's new JSON output. In contrast to that + it should skip the variant type string though. + +* augment CODE_FILE=, CODE_LINE= with something like CODE_BASE= or so which + contains some identifier for the project, which allows us to include + clickable links to source files generating these log messages. The identifier + could be some abberviated URL prefix or so (taking inspiration from Go + imports). For example, for systemd we could use + CODE_BASE=github.com/systemd/systemd/blob/98b0b1123cc or so which is + sufficient to build a link by prefixing "http://" and suffixing the + CODE_FILE. + +* when outputting log data with journalctl and the log data includes references + to configuration files (CONFIG_FILE=), create a clickable link for it. + +* Augment MESSAGE_ID with MESSAGE_BASE, in a similar fashion so that we can + make clickable links from log messages carrying a MESSAGE_ID, that lead to + some explanatory text online. + * maybe extend .path units to expose fanotify() per-mount change events * Add a "systemctl list-units --by-slice" mode or so, which rearranges the