From: Christian Brauner Date: Fri, 19 Feb 2021 02:45:06 +0000 (+0100) Subject: cgroups: make device cgroup handling smarter and simpler X-Git-Tag: lxc-5.0.0~275^2~10 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a134099de529d5436a359b109dfdf44e13470451;p=thirdparty%2Flxc.git cgroups: make device cgroup handling smarter and simpler Signed-off-by: Christian Brauner --- diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c index 0d815c16f..5fb9ff66c 100644 --- a/src/lxc/cgroups/cgfsng.c +++ b/src/lxc/cgroups/cgfsng.c @@ -2773,19 +2773,9 @@ static int device_cgroup_rule_parse(struct device_item *device, const char *key, device->type = 'a'; device->major = -1; device->minor = -1; - - if (device->allow) /* allow all devices */ - device->global_rule = LXC_BPF_DEVICE_CGROUP_DENYLIST; - else /* deny all devices */ - device->global_rule = LXC_BPF_DEVICE_CGROUP_ALLOWLIST; - - device->allow = -1; return 0; } - /* local rule */ - device->global_rule = LXC_BPF_DEVICE_CGROUP_LOCAL_RULE; - switch (*val) { case 'a': __fallthrough; @@ -2968,7 +2958,6 @@ static int device_cgroup_rule_parse_devpath(struct device_item *device, device->major = MAJOR(sb.st_rdev); device->minor = MINOR(sb.st_rdev); device->allow = 1; - device->global_rule = LXC_BPF_DEVICE_CGROUP_LOCAL_RULE; return 0; } @@ -3106,9 +3095,10 @@ static int bpf_device_cgroup_prepare(struct cgroup_ops *ops, if (ret < 0) return log_error_errno(-1, EINVAL, "Failed to parse device string %s=%s", key, val); - ret = bpf_list_add_device(&conf->devices, &device_item); + ret = bpf_list_add_device(&conf->bpf_devices, &device_item); if (ret < 0) return -1; + return 0; } @@ -3180,10 +3170,11 @@ __cgfsng_ops static bool cgfsng_devices_activate(struct cgroup_ops *ops, struct unified = ops->unified; if (!unified || !unified->bpf_device_controller || - !unified->container_full_path || lxc_list_empty(&conf->devices)) + !unified->container_full_path || + lxc_list_empty(&(conf->bpf_devices).device_item)) return true; - return bpf_cgroup_devices_attach(ops, &conf->devices); + return bpf_cgroup_devices_attach(ops, &conf->bpf_devices); } static bool __cgfsng_delegate_controllers(struct cgroup_ops *ops, const char *cgroup) diff --git a/src/lxc/cgroups/cgroup2_devices.c b/src/lxc/cgroups/cgroup2_devices.c index ae281eeaf..f1442de31 100644 --- a/src/lxc/cgroups/cgroup2_devices.c +++ b/src/lxc/cgroups/cgroup2_devices.c @@ -211,12 +211,6 @@ int bpf_program_append_device(struct bpf_program *prog, struct device_item *devi if (!prog || !device) return ret_set_errno(-1, EINVAL); - /* This is a global rule so no need to append anything. */ - if (device->global_rule > LXC_BPF_DEVICE_CGROUP_LOCAL_RULE) { - prog->device_list_type = device->global_rule; - return 0; - } - ret = bpf_access_mask(device->access, &access_mask); if (ret < 0) return log_error_errno(ret, -ret, "Invalid access mask specified %s", device->access); @@ -296,10 +290,10 @@ int bpf_program_finalize(struct bpf_program *prog) if (!prog) return ret_set_errno(-1, EINVAL); - TRACE("Implementing %s bpf device cgroup program", - prog->device_list_type == LXC_BPF_DEVICE_CGROUP_DENYLIST - ? "denylist" - : "allowlist"); + TRACE("Device bpf program %s all devices by default", + prog->device_list_type == LXC_BPF_DEVICE_CGROUP_ALLOWLIST + ? "blocks" + : "allows"); ins[0] = BPF_MOV64_IMM(BPF_REG_0, prog->device_list_type); ins[1] = BPF_EXIT_INSN(); @@ -436,31 +430,35 @@ void bpf_device_program_free(struct cgroup_ops *ops) } } -int bpf_list_add_device(struct lxc_list *devices, struct device_item *device) +int bpf_list_add_device(struct bpf_devices *bpf_devices, + struct device_item *device) { __do_free struct lxc_list *list_elem = NULL; __do_free struct device_item *new_device = NULL; struct lxc_list *it; - if (!devices || !device) + if (!bpf_devices || !device) return ret_errno(EINVAL); - lxc_list_for_each(it, devices) { - struct device_item *cur = it->elem; - - if (cur->global_rule > LXC_BPF_DEVICE_CGROUP_LOCAL_RULE && - device->global_rule > LXC_BPF_DEVICE_CGROUP_LOCAL_RULE) { - TRACE("Switched from %s to %s", - cur->global_rule == LXC_BPF_DEVICE_CGROUP_ALLOWLIST - ? "allowlist" - : "denylist", - device->global_rule == LXC_BPF_DEVICE_CGROUP_ALLOWLIST - ? "allowlist" - : "denylist"); - cur->global_rule = device->global_rule; - return 1; + /* Check whether this determines the list type. */ + if (device->type == 'a' && + device->major < 0 && + device->minor < 0 && + is_empty_string(device->access)) { + if (device->allow) { + bpf_devices->list_type = LXC_BPF_DEVICE_CGROUP_DENYLIST; + TRACE("Device cgroup will allow (\"denylist\") all devices by default"); + } else { + bpf_devices->list_type = LXC_BPF_DEVICE_CGROUP_ALLOWLIST; + TRACE("Device cgroup will deny (\"allowlist\") all devices by default"); } + return 0; + } + + lxc_list_for_each(it, &bpf_devices->device_item) { + struct device_item *cur = it->elem; + if (cur->type != device->type) continue; if (cur->major != device->major) @@ -476,27 +474,26 @@ int bpf_list_add_device(struct lxc_list *devices, struct device_item *device) */ if (cur->allow != device->allow) { cur->allow = device->allow; - return log_trace(0, "Switched existing rule of bpf device program: type %c, major %d, minor %d, access %s, allow %d, global_rule %d", - cur->type, cur->major, cur->minor, - cur->access, cur->allow, - cur->global_rule); + TRACE("Switched existing rule: type %c, major %d, minor %d, access %s, allow %d", + cur->type, cur->major, cur->minor, cur->access, cur->allow); + } else { + TRACE("Reusing existing rule: type %c, major %d, minor %d, access %s, allow %d", + cur->type, cur->major, cur->minor, cur->access, cur->allow); } - return log_trace(1, "Reusing existing rule of bpf device program: type %c, major %d, minor %d, access %s, allow %d, global_rule %d", - cur->type, cur->major, cur->minor, cur->access, - cur->allow, cur->global_rule); + return 0; } list_elem = malloc(sizeof(*list_elem)); if (!list_elem) - return log_error_errno(-1, ENOMEM, "Failed to allocate new device list"); + return syserrno_set(ENOMEM, "Failed to allocate new device list"); new_device = memdup(device, sizeof(struct device_item)); if (!new_device) - return log_error_errno(-1, ENOMEM, "Failed to allocate new device item"); + return syserrno_set(ENOMEM, "Failed to allocate new device item"); lxc_list_add_elem(list_elem, move_ptr(new_device)); - lxc_list_add_tail(devices, move_ptr(list_elem)); + lxc_list_add_tail(&bpf_devices->device_item, move_ptr(list_elem)); return 0; } @@ -533,7 +530,20 @@ bool bpf_devices_cgroup_supported(void) return log_trace(true, "The bpf device cgroup is supported"); } -static struct bpf_program *__bpf_cgroup_devices(struct lxc_list *devices) +static inline bool bpf_device_add(const struct bpf_program *prog, + struct device_item *device) +{ + /* We're blocking all devices so skip individual deny rules. */ + if (bpf_device_block_all(prog) && !device->allow) + return false; + + /* We're allowing all devices so skip individual allow rules. */ + if (!bpf_device_block_all(prog) && device->allow) + return false; + return true; +} + +static struct bpf_program *__bpf_cgroup_devices(struct bpf_devices *bpf_devices) { __do_bpf_program_free struct bpf_program *prog = NULL; int ret; @@ -547,11 +557,11 @@ static struct bpf_program *__bpf_cgroup_devices(struct lxc_list *devices) if (ret) return syserrno(NULL, "Failed to initialize bpf program"); - bpf_device_set_type(prog, devices); - TRACE("Device bpf %s all devices by default", + prog->device_list_type = bpf_devices->list_type; + TRACE("Device cgroup %s all devices by default", bpf_device_block_all(prog) ? "blocks" : "allows"); - lxc_list_for_each(it, devices) { + lxc_list_for_each(it, &bpf_devices->device_item) { struct device_item *cur = it->elem; if (!bpf_device_add(prog, cur)) { @@ -565,7 +575,7 @@ static struct bpf_program *__bpf_cgroup_devices(struct lxc_list *devices) return syserrno(NULL, "Failed adding rule: type %c, major %d, minor %d, access %s, allow %d", cur->type, cur->major, cur->minor, cur->access, cur->allow); - TRACE("Added rule to bpf device program: type %c, major %d, minor %d, access %s, allow %d", + TRACE("Added rule: type %c, major %d, minor %d, access %s, allow %d", cur->type, cur->major, cur->minor, cur->access, cur->allow); } @@ -576,12 +586,13 @@ static struct bpf_program *__bpf_cgroup_devices(struct lxc_list *devices) return move_ptr(prog); } -bool bpf_cgroup_devices_attach(struct cgroup_ops *ops, struct lxc_list *devices) +bool bpf_cgroup_devices_attach(struct cgroup_ops *ops, + struct bpf_devices *bpf_devices) { __do_bpf_program_free struct bpf_program *prog = NULL; int ret; - prog = __bpf_cgroup_devices(devices); + prog = __bpf_cgroup_devices(bpf_devices); if (!prog) return syserrno(false, "Failed to create bpf program"); @@ -597,8 +608,8 @@ bool bpf_cgroup_devices_attach(struct cgroup_ops *ops, struct lxc_list *devices) } bool bpf_cgroup_devices_update(struct cgroup_ops *ops, - struct device_item *new, - struct lxc_list *devices) + struct bpf_devices *bpf_devices, + struct device_item *new) { __do_bpf_program_free struct bpf_program *prog = NULL; static int can_use_bpf_replace = -1; @@ -615,16 +626,16 @@ bool bpf_cgroup_devices_update(struct cgroup_ops *ops, if (ops->unified->cgfd_limit < 0) return ret_set_errno(false, EBADF); - ret = bpf_list_add_device(devices, new); + ret = bpf_list_add_device(bpf_devices, new); if (ret < 0) return false; /* No previous device program attached. */ prog_old = ops->cgroup2_devices; if (!prog_old) - return bpf_cgroup_devices_attach(ops, devices); + return bpf_cgroup_devices_attach(ops, bpf_devices); - prog = __bpf_cgroup_devices(devices); + prog = __bpf_cgroup_devices(bpf_devices); if (!prog) return syserrno(false, "Failed to create bpf program"); diff --git a/src/lxc/cgroups/cgroup2_devices.h b/src/lxc/cgroups/cgroup2_devices.h index f4983f065..d1de5794f 100644 --- a/src/lxc/cgroups/cgroup2_devices.h +++ b/src/lxc/cgroups/cgroup2_devices.h @@ -54,35 +54,6 @@ static inline bool bpf_device_block_all(const struct bpf_program *prog) return prog->device_list_type == LXC_BPF_DEVICE_CGROUP_ALLOWLIST; } -static inline bool bpf_device_add(const struct bpf_program *prog, - struct device_item *device) -{ - if (device->global_rule > LXC_BPF_DEVICE_CGROUP_LOCAL_RULE) - return false; - - /* We're blocking all devices so skip individual deny rules. */ - if (bpf_device_block_all(prog) && !device->allow) - return false; - - /* We're allowing all devices so skip individual allow rules. */ - if (!bpf_device_block_all(prog) && device->allow) - return false; - return true; -} - -static inline void bpf_device_set_type(struct bpf_program *prog, - struct lxc_list *devices) -{ - struct lxc_list *it; - - lxc_list_for_each (it, devices) { - struct device_item *cur = it->elem; - - if (cur->global_rule > LXC_BPF_DEVICE_CGROUP_LOCAL_RULE) - prog->device_list_type = cur->global_rule; - } -} - __hidden extern struct bpf_program *bpf_program_new(__u32 prog_type); __hidden extern int bpf_program_init(struct bpf_program *prog); __hidden extern int bpf_program_append_device(struct bpf_program *prog, struct device_item *device); @@ -91,13 +62,13 @@ __hidden extern int bpf_program_cgroup_detach(struct bpf_program *prog); __hidden extern void bpf_device_program_free(struct cgroup_ops *ops); __hidden extern bool bpf_devices_cgroup_supported(void); -__hidden extern int bpf_list_add_device(struct lxc_list *devices, +__hidden extern int bpf_list_add_device(struct bpf_devices *bpf_devices, struct device_item *device); __hidden extern bool bpf_cgroup_devices_attach(struct cgroup_ops *ops, - struct lxc_list *devices); + struct bpf_devices *bpf_devices); __hidden extern bool bpf_cgroup_devices_update(struct cgroup_ops *ops, - struct device_item *new, - struct lxc_list *devices); + struct bpf_devices *bpf_devices, + struct device_item *device); static inline void bpf_program_free(struct bpf_program *prog) { diff --git a/src/lxc/commands.c b/src/lxc/commands.c index b00499916..5ff185380 100644 --- a/src/lxc/commands.c +++ b/src/lxc/commands.c @@ -1195,7 +1195,6 @@ static int lxc_cmd_add_bpf_device_cgroup_callback(int fd, struct lxc_cmd_req *re { int ret; struct lxc_cmd_rsp rsp = {}; - struct device_item *device; struct lxc_conf *conf; if (req->datalen <= 0) @@ -1207,9 +1206,10 @@ static int lxc_cmd_add_bpf_device_cgroup_callback(int fd, struct lxc_cmd_req *re if (!req->data) return LXC_CMD_REAP_CLIENT_FD; - device = (struct device_item *)req->data; conf = handler->conf; - if (!bpf_cgroup_devices_update(handler->cgroup_ops, device, &conf->devices)) + if (!bpf_cgroup_devices_update(handler->cgroup_ops, + &conf->bpf_devices, + (struct device_item *)req->data)) rsp.ret = -1; else rsp.ret = 0; diff --git a/src/lxc/conf.c b/src/lxc/conf.c index 24b6cbb92..43e8668c1 100644 --- a/src/lxc/conf.c +++ b/src/lxc/conf.c @@ -2678,7 +2678,9 @@ struct lxc_conf *lxc_conf_init(void) new->logfd = -1; lxc_list_init(&new->cgroup); lxc_list_init(&new->cgroup2); - lxc_list_init(&new->devices); + /* Block ("allowlist") all devices by default. */ + new->bpf_devices.list_type = LXC_BPF_DEVICE_CGROUP_ALLOWLIST; + lxc_list_init(&(new->bpf_devices).device_item); lxc_list_init(&new->network); lxc_list_init(&new->mount_list); lxc_list_init(&new->caps); @@ -3710,13 +3712,15 @@ int lxc_clear_cgroups(struct lxc_conf *c, const char *key, int version) static void lxc_clear_devices(struct lxc_conf *conf) { - struct lxc_list *list = &conf->devices; + struct lxc_list *list = &(conf->bpf_devices).device_item; struct lxc_list *it, *next; lxc_list_for_each_safe(it, list, next) { lxc_list_del(it); free(it); } + + lxc_list_init(&(conf->bpf_devices).device_item); } int lxc_clear_limits(struct lxc_conf *c, const char *key) diff --git a/src/lxc/conf.h b/src/lxc/conf.h index 490356b41..ceae29fea 100644 --- a/src/lxc/conf.h +++ b/src/lxc/conf.h @@ -270,7 +270,6 @@ struct lxc_state_client { }; typedef enum lxc_bpf_devices_rule_t { - LXC_BPF_DEVICE_CGROUP_LOCAL_RULE = -1, LXC_BPF_DEVICE_CGROUP_ALLOWLIST = 0, LXC_BPF_DEVICE_CGROUP_DENYLIST = 1, } lxc_bpf_devices_rule_t; @@ -281,12 +280,11 @@ struct device_item { int minor; char access[4]; int allow; - /* - * LXC_BPF_DEVICE_CGROUP_LOCAL_RULE -> no global rule - * LXC_BPF_DEVICE_CGROUP_ALLOWLIST -> allowlist (deny all) - * LXC_BPF_DEVICE_CGROUP_DENYLIST -> denylist (allow all) - */ - int global_rule; +}; + +struct bpf_devices { + lxc_bpf_devices_rule_t list_type; + struct lxc_list device_item; }; struct timens_offsets { @@ -310,8 +308,7 @@ struct lxc_conf { struct { struct lxc_list cgroup; struct lxc_list cgroup2; - /* This should be reimplemented as a hashmap. */ - struct lxc_list devices; + struct bpf_devices bpf_devices; }; struct {