From 124e05b3b60c253d83ba5e122aca34be719391ff Mon Sep 17 00:00:00 2001 From: =?utf8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 8 Nov 2019 15:51:28 +0100 Subject: [PATCH] core: move bpf devices implementation to bpf-devices.[ch] and rename The naming of the functions was a complete mess: the most specific functions which don't know anything about cgroups had "cgroup_" prefix, while more general functions which took a node path and a cgroup for reporting had no prefix. Let's use "bpf_devices_" for the latter group, and "bpf_prog_*" for the rest. The main goal of this move is to split the implementation from the calling code and add unit tests in a later patch. --- src/core/bpf-devices.c | 165 ++++++++++++++++++++++++++++++++++++++++++++++-- src/core/bpf-devices.h | 12 ++-- src/core/cgroup.c | 167 +++---------------------------------------------- 3 files changed, 174 insertions(+), 170 deletions(-) diff --git a/src/core/bpf-devices.c b/src/core/bpf-devices.c index 3b8e0ac..41b751b 100644 --- a/src/core/bpf-devices.c +++ b/src/core/bpf-devices.c @@ -1,8 +1,16 @@ /* SPDX-License-Identifier: LGPL-2.1+ */ + +#include #include #include "bpf-devices.h" #include "bpf-program.h" +#include "fd-util.h" +#include "fileio.h" +#include "parse-util.h" +#include "stat-util.h" +#include "stdio-util.h" +#include "string-util.h" #define PASS_JUMP_OFF 4096 @@ -29,7 +37,7 @@ static int bpf_access_type(const char *acc) { return r; } -int cgroup_bpf_whitelist_device(BPFProgram *prog, int type, int major, int minor, const char *acc) { +static int bpf_prog_whitelist_device(BPFProgram *prog, int type, int major, int minor, const char *acc) { int r, access; assert(prog); @@ -56,7 +64,7 @@ int cgroup_bpf_whitelist_device(BPFProgram *prog, int type, int major, int minor return r; } -int cgroup_bpf_whitelist_major(BPFProgram *prog, int type, int major, const char *acc) { +static int bpf_prog_whitelist_major(BPFProgram *prog, int type, int major, const char *acc) { int r, access; assert(prog); @@ -82,7 +90,7 @@ 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) { +static int bpf_prog_whitelist_class(BPFProgram *prog, int type, const char *acc) { int r, access; assert(prog); @@ -107,7 +115,7 @@ int cgroup_bpf_whitelist_class(BPFProgram *prog, int type, const char *acc) { return r; } -int cgroup_init_device_bpf(BPFProgram **ret, CGroupDevicePolicy policy, bool whitelist) { +int bpf_devices_cgroup_init(BPFProgram **ret, CGroupDevicePolicy policy, bool whitelist) { const struct bpf_insn pre_insn[] = { /* load device type to r2 */ BPF_LDX_MEM(BPF_H, BPF_REG_2, BPF_REG_1, @@ -150,7 +158,7 @@ int cgroup_init_device_bpf(BPFProgram **ret, CGroupDevicePolicy policy, bool whi return 0; } -int cgroup_apply_device_bpf(Unit *u, BPFProgram *prog, CGroupDevicePolicy policy, bool whitelist) { +int bpf_devices_apply_policy(Unit *u, BPFProgram *prog, CGroupDevicePolicy policy, bool whitelist) { _cleanup_free_ char *path = NULL; int r; @@ -262,3 +270,150 @@ int bpf_devices_supported(void) { return supported = 1; } + +static int whitelist_device_pattern(BPFProgram *prog, const char *path, char type, const unsigned *maj, const unsigned *min, const char *acc) { + assert(IN_SET(type, 'b', 'c')); + + if (cg_all_unified() > 0) { + if (!prog) + return 0; + + const int bpf_type = type == 'c' ? BPF_DEVCG_DEV_CHAR : BPF_DEVCG_DEV_BLOCK; + if (maj && min) + return bpf_prog_whitelist_device(prog, bpf_type, *maj, *min, acc); + else if (maj) + return bpf_prog_whitelist_major(prog, bpf_type, *maj, acc); + else + return bpf_prog_whitelist_class(prog, bpf_type, acc); + + } else { + char buf[2+DECIMAL_STR_MAX(unsigned)*2+2+4]; + int r; + + if (maj && min) + xsprintf(buf, "%c %u:%u %s", type, *maj, *min, acc); + else if (maj) + xsprintf(buf, "%c %u:* %s", type, *maj, acc); + else + xsprintf(buf, "%c *:* %s", type, acc); + + /* Changing the devices list of a populated cgroup might result in EINVAL, hence ignore + * EINVAL here. */ + + r = cg_set_attribute("devices", path, "devices.allow", buf); + if (r < 0) + log_full_errno(IN_SET(r, -ENOENT, -EROFS, -EINVAL, -EACCES, -EPERM) ? LOG_DEBUG : LOG_WARNING, + r, "Failed to set devices.allow on %s: %m", path); + + return r; + } +} + +int bpf_devices_whitelist_device(BPFProgram *prog, const char *path, const char *node, const char *acc) { + mode_t mode; + dev_t rdev; + int r; + + assert(path); + assert(acc); + assert(strlen(acc) <= 3); + + /* 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 = device_path_parse_major_minor(node, &mode, &rdev); + if (r < 0) { + if (r != -ENODEV) + return log_warning_errno(r, "Couldn't parse major/minor from device path '%s': %m", node); + + struct stat st; + if (stat(node, &st) < 0) + return log_warning_errno(errno, "Couldn't stat device %s: %m", node); + + if (!S_ISCHR(st.st_mode) && !S_ISBLK(st.st_mode)) + return log_warning_errno(SYNTHETIC_ERRNO(ENODEV), "%s is not a device.", node); + + mode = st.st_mode; + rdev = (dev_t) st.st_rdev; + } + + unsigned maj = major(rdev), min = minor(rdev); + return whitelist_device_pattern(prog, path, S_ISCHR(mode) ? 'c' : 'b', &maj, &min, acc); +} + +int bpf_devices_whitelist_major(BPFProgram *prog, const char *path, const char *name, char type, const char *acc) { + 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 */ + return whitelist_device_pattern(prog, path, type, NULL, NULL, acc); + + if (safe_atou(name, &maj) >= 0 && DEVICE_MAJOR_VALID(maj)) + /* The name is numeric and suitable as major. In that case, let's take its major, and create + * the entry directly. */ + return whitelist_device_pattern(prog, path, type, &maj, NULL, acc); + + _cleanup_fclose_ FILE *f = NULL; + bool good = false; + + f = fopen("/proc/devices", "re"); + if (!f) + return log_warning_errno(errno, "Cannot open /proc/devices to resolve %s: %m", name); + + for (;;) { + _cleanup_free_ char *line = NULL; + char *w, *p; + + r = read_line(f, LONG_LINE_MAX, &line); + if (r < 0) + return log_warning_errno(r, "Failed to read /proc/devices: %m"); + if (r == 0) + break; + + if (type == 'c' && streq(line, "Character devices:")) { + good = true; + continue; + } + + if (type == 'b' && streq(line, "Block devices:")) { + good = true; + continue; + } + + if (isempty(line)) { + good = false; + continue; + } + + if (!good) + continue; + + p = strstrip(line); + + w = strpbrk(p, WHITESPACE); + if (!w) + continue; + *w = 0; + + r = safe_atou(p, &maj); + if (r < 0) + continue; + if (maj <= 0) + continue; + + w++; + w += strspn(w, WHITESPACE); + + if (fnmatch(name, w, 0) != 0) + continue; + + (void) whitelist_device_pattern(prog, path, type, &maj, NULL, acc); + } + + return 0; +} diff --git a/src/core/bpf-devices.h b/src/core/bpf-devices.h index 504b090..744d5f8 100644 --- a/src/core/bpf-devices.h +++ b/src/core/bpf-devices.h @@ -7,11 +7,9 @@ struct BPFProgram; -int bpf_devices_supported(void); - -int cgroup_bpf_whitelist_device(BPFProgram *prog, int type, int major, int minor, const char *acc); -int cgroup_bpf_whitelist_major(BPFProgram *prog, int type, int major, const char *acc); -int cgroup_bpf_whitelist_class(BPFProgram *prog, int type, const char *acc); +int bpf_devices_cgroup_init(BPFProgram **ret, CGroupDevicePolicy policy, bool whitelist); +int bpf_devices_apply_policy(Unit *u, BPFProgram *prog, CGroupDevicePolicy policy, bool whitelist); -int cgroup_init_device_bpf(BPFProgram **ret, CGroupDevicePolicy policy, bool whitelist); -int cgroup_apply_device_bpf(Unit *u, BPFProgram *prog, CGroupDevicePolicy policy, bool whitelist); +int bpf_devices_supported(void); +int bpf_devices_whitelist_device(BPFProgram *prog, const char *path, const char *node, const char *acc); +int bpf_devices_whitelist_major(BPFProgram *prog, const char *path, const char *name, char type, const char *acc); diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 48a320f..9857a68 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -1,7 +1,6 @@ /* SPDX-License-Identifier: LGPL-2.1+ */ #include -#include #include "sd-messages.h" @@ -667,153 +666,6 @@ static int lookup_block_device(const char *p, dev_t *ret) { return 0; } -static int whitelist_device_pattern(BPFProgram *prog, const char *path, char type, const unsigned *maj, const unsigned *min, const char *acc) { - assert(IN_SET(type, 'b', 'c')); - - if (cg_all_unified() > 0) { - if (!prog) - return 0; - - const int bpf_type = type == 'c' ? BPF_DEVCG_DEV_CHAR : BPF_DEVCG_DEV_BLOCK; - if (maj && min) - return cgroup_bpf_whitelist_device(prog, bpf_type, *maj, *min, acc); - else if (maj) - return cgroup_bpf_whitelist_major(prog, bpf_type, *maj, acc); - else - return cgroup_bpf_whitelist_class(prog, bpf_type, acc); - - } else { - char buf[2+DECIMAL_STR_MAX(unsigned)*2+2+4]; - int r; - - if (maj && min) - xsprintf(buf, "%c %u:%u %s", type, *maj, *min, acc); - else if (maj) - xsprintf(buf, "%c %u:* %s", type, *maj, acc); - else - xsprintf(buf, "%c *:* %s", type, acc); - - /* Changing the devices list of a populated cgroup might result in EINVAL, hence ignore - * EINVAL here. */ - - r = cg_set_attribute("devices", path, "devices.allow", buf); - if (r < 0) - log_full_errno(IN_SET(r, -ENOENT, -EROFS, -EINVAL, -EACCES, -EPERM) ? LOG_DEBUG : LOG_WARNING, - r, "Failed to set devices.allow on %s: %m", path); - - return r; - } -} - -static int whitelist_device(BPFProgram *prog, const char *path, const char *node, const char *acc) { - mode_t mode; - dev_t rdev; - int r; - - assert(path); - assert(acc); - assert(strlen(acc) <= 3); - - /* 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 = device_path_parse_major_minor(node, &mode, &rdev); - if (r < 0) { - if (r != -ENODEV) - return log_warning_errno(r, "Couldn't parse major/minor from device path '%s': %m", node); - - struct stat st; - if (stat(node, &st) < 0) - return log_warning_errno(errno, "Couldn't stat device %s: %m", node); - - if (!S_ISCHR(st.st_mode) && !S_ISBLK(st.st_mode)) - return log_warning_errno(SYNTHETIC_ERRNO(ENODEV), "%s is not a device.", node); - - mode = st.st_mode; - rdev = (dev_t) st.st_rdev; - } - - unsigned maj = major(rdev), min = minor(rdev); - return whitelist_device_pattern(prog, path, S_ISCHR(mode) ? 'c' : 'b', &maj, &min, acc); -} - -static int whitelist_major(BPFProgram *prog, const char *path, const char *name, char type, const char *acc) { - 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 */ - return whitelist_device_pattern(prog, path, type, NULL, NULL, acc); - - if (safe_atou(name, &maj) >= 0 && DEVICE_MAJOR_VALID(maj)) - /* The name is numeric and suitable as major. In that case, let's take its major, and create - * the entry directly. */ - return whitelist_device_pattern(prog, path, type, &maj, NULL, acc); - - _cleanup_fclose_ FILE *f = NULL; - bool good = false; - - f = fopen("/proc/devices", "re"); - if (!f) - return log_warning_errno(errno, "Cannot open /proc/devices to resolve %s: %m", name); - - for (;;) { - _cleanup_free_ char *line = NULL; - char *w, *p; - - r = read_line(f, LONG_LINE_MAX, &line); - if (r < 0) - return log_warning_errno(r, "Failed to read /proc/devices: %m"); - if (r == 0) - break; - - if (type == 'c' && streq(line, "Character devices:")) { - good = true; - continue; - } - - if (type == 'b' && streq(line, "Block devices:")) { - good = true; - continue; - } - - if (isempty(line)) { - good = false; - continue; - } - - if (!good) - continue; - - p = strstrip(line); - - w = strpbrk(p, WHITESPACE); - if (!w) - continue; - *w = 0; - - r = safe_atou(p, &maj); - if (r < 0) - continue; - if (maj <= 0) - continue; - - w++; - w += strspn(w, WHITESPACE); - - if (fnmatch(name, w, 0) != 0) - continue; - - (void) whitelist_device_pattern(prog, path, type, &maj, NULL, acc); - } - - return 0; -} - static bool cgroup_context_has_cpu_weight(CGroupContext *c) { return c->cpu_weight != CGROUP_WEIGHT_INVALID || c->startup_cpu_weight != CGROUP_WEIGHT_INVALID; @@ -1385,7 +1237,7 @@ static void cgroup_context_apply( CGroupDeviceAllow *a; if (cg_all_unified() > 0) { - r = cgroup_init_device_bpf(&prog, c->device_policy, c->device_allow); + r = bpf_devices_cgroup_init(&prog, c->device_policy, c->device_allow); if (r < 0) log_unit_warning_errno(u, r, "Failed to initialize device control bpf program: %m"); } else { @@ -1415,13 +1267,12 @@ static void cgroup_context_apply( "/run/systemd/inaccessible/chr\0" "rwm\0" "/run/systemd/inaccessible/blk\0" "rwm\0"; - const char *x, *y; - - NULSTR_FOREACH_PAIR(x, y, auto_devices) - (void) whitelist_device(prog, path, x, y); + const char *node, *acc; + NULSTR_FOREACH_PAIR(node, acc, auto_devices) + (void) bpf_devices_whitelist_device(prog, path, node, acc); /* PTS (/dev/pts) devices may not be duplicated, but accessed */ - (void) whitelist_major(prog, path, "pts", 'c', "rw"); + (void) bpf_devices_whitelist_major(prog, path, "pts", 'c', "rw"); } LIST_FOREACH(device_allow, a, c->device_allow) { @@ -1441,16 +1292,16 @@ static void cgroup_context_apply( acc[k++] = 0; if (path_startswith(a->path, "/dev/")) - (void) whitelist_device(prog, path, a->path, acc); + (void) bpf_devices_whitelist_device(prog, path, a->path, acc); else if ((val = startswith(a->path, "block-"))) - (void) whitelist_major(prog, path, val, 'b', acc); + (void) bpf_devices_whitelist_major(prog, path, val, 'b', acc); else if ((val = startswith(a->path, "char-"))) - (void) whitelist_major(prog, path, val, 'c', acc); + (void) bpf_devices_whitelist_major(prog, path, val, 'c', acc); else 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); + r = bpf_devices_apply_policy(u, prog, c->device_policy, c->device_allow); if (r < 0) { static bool warned = false; -- 2.7.4