cgroups: make device cgroup handling smarter and simpler

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>

diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c
index 0d815c16f65a688f468f875b7fa223e12751034e..5fb9ff66cb0f0654cc0d657d4723b848ac503730 100644 (file)
--- 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 ae281eeaffac2c5423c483a0635955952ac30f78..f1442de31bf2c4776d9f0ce3da0e552e44a921bf 100644 (file)
--- 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 f4983f0657a731fe338eb55c16aabbfc7c2e43a0..d1de5794f973e132cd0957fe0987a173f2014126 100644 (file)
--- 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 b0049991654cc646998a2dbae1782ad9c392c079..5ff185380d410501aed965903f67cc1202f43623 100644 (file)
--- 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 24b6cbb92e683fd72025cfdce33bb4af0c47f8b1..43e8668c1717de5e812d1e8de48196bcaf9c649a 100644 (file)
--- 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 490356b41b5031952d70b29a17bee6786b4f50f1..ceae29fea5390e5fbbe9806e98a9b0c5c667737b 100644 (file)
--- 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 {