diff --git a/libocispec b/libocispec index 20bc912e5..5dfe2f406 160000 --- a/libocispec +++ b/libocispec @@ -1 +1 @@ -Subproject commit 20bc912e572993240deaf0b11cb8dea10b359750 +Subproject commit 5dfe2f406dc2d0f244aec621292e4e0a52149240 diff --git a/src/libcrun/cgroup.c b/src/libcrun/cgroup.c index 7397d2a29..032d5213e 100644 --- a/src/libcrun/cgroup.c +++ b/src/libcrun/cgroup.c @@ -226,7 +226,7 @@ write_controller_file (const char *path, int controllers_to_enable, libcrun_erro libcrun_error_t tmp_err = NULL; int exists; - exists = crun_path_exists (subtree_control, err); + exists = crun_path_exists (subtree_control, &tmp_err); if (UNLIKELY (exists < 0)) { crun_error_release (&tmp_err); @@ -2752,9 +2752,85 @@ update_cgroup_v1_resources (runtime_spec_schema_config_linux_resources *resource return ret; } + if (resources->unified && resources->unified->len > 0) + return crun_make_error (err, 0, "invalid configuration: cannot use unified on cgroup v1"); + + return 0; +} + +static int +write_unified_resources (int cgroup_dirfd, runtime_spec_schema_config_linux_resources *resources, libcrun_error_t *err) +{ + size_t i; + int ret; + + for (i = 0; i < resources->unified->len; i++) + { + size_t len; + + if (strchr (resources->unified->keys[i], '/')) + return crun_make_error (err, 0, "key `%s` must be a file name without any slash", resources->unified->keys[i]); + + len = strlen (resources->unified->values[i]); + ret = write_file_at (cgroup_dirfd, resources->unified->keys[i], resources->unified->values[i], len, err); + if (UNLIKELY (ret < 0)) + { + errno = crun_error_get_errno (err); + + /* If the file is not found, try to give a more meaningful error message. */ + if (errno == ENOENT || errno == EPERM || errno == EACCES) + { + cleanup_free char *controllers = NULL; + libcrun_error_t tmp_err = NULL; + cleanup_free char *key = NULL; + char *saveptr = NULL; + bool found = false; + const char *token; + char *it; + + /* Check if the specified controller is enabled. */ + key = xstrdup (resources->unified->keys[i]); + + it = strchr (key, '.'); + if (it == NULL) + { + crun_error_release (err); + return crun_make_error (err, 0, "the specified key has not the form CONTROLLER.VALUE `%s`", resources->unified->keys[i]); + } + *it = '\0'; + + /* cgroup. files are not part of a controller. Return the original error. */ + if (strcmp (key, "cgroup") == 0) + return ret; + + /* If the cgroup.controllers file cannot be read, return the original error. */ + if (read_all_file_at (cgroup_dirfd, "cgroup.controllers", &controllers, NULL, &tmp_err) < 0) + { + crun_error_release (&tmp_err); + return ret; + } + for (token = strtok_r (controllers, " \n", &saveptr); token; token = strtok_r (NULL, " \n", &saveptr)) + { + if (strcmp (token, key) == 0) + { + found = true; + break; + } + } + if (! found) + { + crun_error_release (err); + return crun_make_error (err, 0, "the requested controller `%s` is not available", key); + } + } + return ret; + } + } + return 0; } + static int update_cgroup_v2_resources (runtime_spec_schema_config_linux_resources *resources, char *path, libcrun_error_t *err) { @@ -2828,6 +2904,14 @@ update_cgroup_v2_resources (runtime_spec_schema_config_linux_resources *resource return ret; } + /* Write unified resources if any. They have higher precedence and override any previous setting. */ + if (resources->unified) + { + ret = write_unified_resources (cgroup_dirfd, resources, err); + if (UNLIKELY (ret < 0)) + return ret; + } + return 0; } diff --git a/src/libcrun/utils.c b/src/libcrun/utils.c index cf0399193..a7966652b 100644 --- a/src/libcrun/utils.c +++ b/src/libcrun/utils.c @@ -746,20 +746,28 @@ read_all_fd (int fd, const char *description, char **out, size_t *len, libcrun_e } int -read_all_file (const char *path, char **out, size_t *len, libcrun_error_t *err) +read_all_file_at (int dirfd, const char *path, char **out, size_t *len, libcrun_error_t *err) { cleanup_close int fd; - if (strcmp (path, "-") == 0) - path = "/dev/stdin"; - - fd = TEMP_FAILURE_RETRY (open (path, O_RDONLY)); + fd = TEMP_FAILURE_RETRY (openat (dirfd, path, O_RDONLY)); if (UNLIKELY (fd < 0)) return crun_make_error (err, errno, "error opening file `%s`", path); return read_all_fd (fd, path, out, len, err); } +int +read_all_file (const char *path, char **out, size_t *len, libcrun_error_t *err) +{ + cleanup_close int fd = -1; + + if (strcmp (path, "-") == 0) + path = "/dev/stdin"; + + return read_all_file_at (AT_FDCWD, path, out, len, err); +} + int open_unix_domain_client_socket (const char *path, int dgram, libcrun_error_t *err) { diff --git a/src/libcrun/utils.h b/src/libcrun/utils.h index 02772f0d3..9aa50fa79 100644 --- a/src/libcrun/utils.h +++ b/src/libcrun/utils.h @@ -183,6 +183,8 @@ int read_all_fd (int fd, const char *description, char **out, size_t *len, libcr int read_all_file (const char *path, char **out, size_t *len, libcrun_error_t *err); +int read_all_file_at (int dirfd, const char *path, char **out, size_t *len, libcrun_error_t *err); + int open_unix_domain_client_socket (const char *path, int dgram, libcrun_error_t *err); int open_unix_domain_socket (const char *path, int dgram, libcrun_error_t *err); diff --git a/tests/init.c b/tests/init.c index 3ecd3393b..52afdcaec 100644 --- a/tests/init.c +++ b/tests/init.c @@ -136,7 +136,7 @@ int main (int argc, char **argv) if (strcmp (argv[1], "echo") == 0) { if (argc < 3) - error (EXIT_FAILURE, 0, "'cat' requires an argument"); + error (EXIT_FAILURE, 0, "'echo' requires an argument"); fputs (argv[2], stdout); exit (0); } diff --git a/tests/test_resources.py b/tests/test_resources.py index 2a789c8d2..69c383610 100755 --- a/tests/test_resources.py +++ b/tests/test_resources.py @@ -23,6 +23,10 @@ import sys from tests_utils import * + +def is_cgroup_v2_unified(): + return subprocess.check_output("stat -c%T -f /sys/fs/cgroup".split()).decode("utf-8").strip() == "cgroup2fs" + def test_resources_pid_limit(): if os.getuid() != 0: return 77 @@ -42,8 +46,88 @@ def test_resources_pid_limit(): return -1 return 0 +def test_resources_unified_invalid_controller(): + if not is_cgroup_v2_unified() or os.geteuid() != 0: + return 77 + + conf = base_config() + add_all_namespaces(conf, cgroupns=True) + conf['process']['args'] = ['/init', 'pause'] + + conf['linux']['resources'] = {} + conf['linux']['resources']['unified'] = { + "foo.bar": "doesntmatter" + } + cid = None + try: + out, cid = run_and_get_output(conf, command='run', detach=True) + # must raise an exception, fail if it doesn't. + return -1 + except Exception as e: + if 'the requested controller `foo` is not available' in e.stdout.decode("utf-8").strip(): + return 0 + return -1 + finally: + if cid is not None: + run_crun_command(["delete", "-f", cid]) + return 0 + +def test_resources_unified_invalid_key(): + if not is_cgroup_v2_unified() or os.geteuid() != 0: + return 77 + + conf = base_config() + add_all_namespaces(conf, cgroupns=True) + conf['process']['args'] = ['/init', 'pause'] + + conf['linux']['resources'] = {} + conf['linux']['resources']['unified'] = { + "NOT-A-VALID-KEY": "doesntmatter" + } + cid = None + try: + out, cid = run_and_get_output(conf, command='run', detach=True) + # must raise an exception, fail if it doesn't. + return -1 + except Exception as e: + if 'the specified key has not the form CONTROLLER.VALUE `NOT-A-VALID-KEY`' in e.stdout.decode("utf-8").strip(): + return 0 + return -1 + finally: + if cid is not None: + run_crun_command(["delete", "-f", cid]) + return 0 + +def test_resources_unified(): + if not is_cgroup_v2_unified() or os.geteuid() != 0: + return 77 + + conf = base_config() + add_all_namespaces(conf, cgroupns=True) + conf['process']['args'] = ['/init', 'pause'] + + conf['linux']['resources'] = {} + conf['linux']['resources']['unified'] = { + "memory.high": "1073741824" + } + cid = None + try: + _, cid = run_and_get_output(conf, command='run', detach=True) + out = run_crun_command(["exec", cid, "/init", "cat", "/sys/fs/cgroup/memory.high"]) + if "1073741824" not in out: + return -1 + finally: + if cid is not None: + run_crun_command(["delete", "-f", cid]) + return 0 + + + all_tests = { "resources-pid-limit" : test_resources_pid_limit, + "resources-unified" : test_resources_unified, + "resources-unified-invalid-controller" : test_resources_unified_invalid_controller, + "resources-unified-invalid-key" : test_resources_unified_invalid_key, } if __name__ == "__main__": diff --git a/tests/tests_utils.py b/tests/tests_utils.py index f2a6e363d..f9313608f 100755 --- a/tests/tests_utils.py +++ b/tests/tests_utils.py @@ -144,11 +144,14 @@ def parse_proc_status(content): r[k] = v.strip() return r -def add_all_namespaces(conf): +def add_all_namespaces(conf, cgroupns=False): has = {} for i in conf['linux']['namespaces']: has[i['type']] = i['type'] - for i in ['pid', 'user', 'ipc', 'uts', 'network']: + namespaces = ['pid', 'user', 'ipc', 'uts', 'network'] + if cgroupns: + namespaces = namespaces + ["cgroup"] + for i in namespaces: if i not in has: conf['linux']['namespaces'].append({"type" : i})