diff --git a/lua/lua_crun.c b/lua/lua_crun.c index c88d6eb19e..ff0561ca62 100644 --- a/lua/lua_crun.c +++ b/lua/lua_crun.c @@ -512,9 +512,10 @@ luacrun_ctx_status_container (lua_State *S) cleanup_container libcrun_container_t *container = NULL; cleanup_free char *dir = NULL; - dir = libcrun_get_state_directory (state_root, id); - if (dir == NULL) + ret = libcrun_get_state_directory (&dir, state_root, id, &crun_err); + if (UNLIKELY (ret < 0)) { + libcrun_error_release (&crun_err); lua_pushnil (S); lua_pushstring (S, "cannot get state directory"); return 2; @@ -526,6 +527,7 @@ luacrun_ctx_status_container (lua_State *S) lua_pop (S, 1); if (container == NULL) { + libcrun_error_release (&crun_err); lua_pushnil (S); lua_pushstring (S, "error loading config.json"); return 2; diff --git a/src/crun.c b/src/crun.c index f356a95c44..70efd4b8b6 100644 --- a/src/crun.c +++ b/src/crun.c @@ -239,7 +239,18 @@ static struct argp_option options[] = { { "debug", OPTION_DEBUG, 0, 0, "produce static void print_version (FILE *stream, struct argp_state *state arg_unused) { - cleanup_free char *rundir = libcrun_get_state_directory (arguments.root, NULL); + libcrun_error_t err = NULL; + cleanup_free char *rundir = NULL; + int ret; + + ret = libcrun_get_state_directory (&rundir, arguments.root, NULL, &err); + if (UNLIKELY (ret < 0)) + { + libcrun_error_release (&err); + fprintf (stderr, "Failed to get state directory\n"); + exit (EXIT_FAILURE); + } + fprintf (stream, "%s version %s\n", PACKAGE_NAME, PACKAGE_VERSION); fprintf (stream, "commit: %s\n", GIT_VERSION); fprintf (stream, "rundir: %s\n", rundir); diff --git a/src/libcrun/cgroup-systemd.c b/src/libcrun/cgroup-systemd.c index b6163a8baf..dde516a532 100644 --- a/src/libcrun/cgroup-systemd.c +++ b/src/libcrun/cgroup-systemd.c @@ -1551,7 +1551,9 @@ enter_systemd_cgroup_scope (runtime_spec_schema_config_linux_resources *resource *can_retry = false; - state_dir = libcrun_get_state_directory (state_root, NULL); + ret = libcrun_get_state_directory (&state_dir, state_root, NULL, err); + if (UNLIKELY (ret < 0)) + return ret; i = 0; boolean_opts[i++] = "Delegate"; @@ -1937,7 +1939,9 @@ libcrun_update_resources_systemd (struct libcrun_cgroup_status *cgroup_status, int sd_err, ret; int cgroup_mode; - state_dir = libcrun_get_state_directory (state_root, NULL); + ret = libcrun_get_state_directory (&state_dir, state_root, NULL, err); + if (UNLIKELY (ret < 0)) + return ret; cgroup_mode = libcrun_get_cgroup_mode (err); if (UNLIKELY (cgroup_mode < 0)) diff --git a/src/libcrun/container.c b/src/libcrun/container.c index ecefc7e766..db3deeb5e6 100644 --- a/src/libcrun/container.c +++ b/src/libcrun/container.c @@ -1604,9 +1604,9 @@ read_container_config_from_state (libcrun_container_t **container, const char *s *container = NULL; - dir = libcrun_get_state_directory (state_root, id); - if (UNLIKELY (dir == NULL)) - return crun_make_error (err, 0, "cannot get state directory from `%s`", state_root); + ret = libcrun_get_state_directory (&dir, state_root, id, err); + if (UNLIKELY (ret < 0)) + return ret; ret = append_paths (&config_file, err, dir, "config.json", NULL); if (UNLIKELY (ret < 0)) @@ -2041,9 +2041,9 @@ wait_for_process (struct wait_for_process_args *args, libcrun_error_t *err) struct libcrun_load_seccomp_notify_conf_s conf; memset (&conf, 0, sizeof conf); - state_root = libcrun_get_state_directory (args->context->state_root, args->context->id); - if (UNLIKELY (state_root == NULL)) - return crun_make_error (err, 0, "cannot get state directory"); + ret = libcrun_get_state_directory (&state_root, args->context->state_root, args->context->id, err); + if (UNLIKELY (ret < 0)) + return ret; ret = append_paths (&oci_config_path, err, state_root, "config.json", NULL); if (UNLIKELY (ret < 0)) @@ -2777,9 +2777,9 @@ libcrun_copy_config_file (const char *id, const char *state_root, libcrun_contai cleanup_free char *buffer = NULL; size_t len; - dir = libcrun_get_state_directory (state_root, id); - if (UNLIKELY (dir == NULL)) - return crun_make_error (err, 0, "cannot get state directory"); + ret = libcrun_get_state_directory (&dir, state_root, id, err); + if (UNLIKELY (ret < 0)) + return ret; ret = append_paths (&dest_path, err, dir, "config.json", NULL); if (UNLIKELY (ret < 0)) @@ -3259,12 +3259,9 @@ libcrun_container_state (libcrun_context_t *context, const char *id, FILE *out, cleanup_container libcrun_container_t *container = NULL; cleanup_free char *dir = NULL; - dir = libcrun_get_state_directory (state_root, id); - if (UNLIKELY (dir == NULL)) - { - ret = crun_make_error (err, 0, "cannot get state directory"); - goto exit; - } + ret = libcrun_get_state_directory (&dir, state_root, id, err); + if (UNLIKELY (ret < 0)) + goto exit; ret = append_paths (&config_file, err, dir, "config.json", NULL); if (UNLIKELY (ret < 0)) @@ -3598,9 +3595,9 @@ libcrun_container_exec_with_options (libcrun_context_t *context, const char *id, return ret; container_status = ret; - dir = libcrun_get_state_directory (state_root, id); - if (UNLIKELY (dir == NULL)) - return crun_make_error (err, 0, "cannot get state directory"); + ret = libcrun_get_state_directory (&dir, state_root, id, err); + if (UNLIKELY (ret < 0)) + return ret; ret = append_paths (&config_file, err, dir, "config.json", NULL); if (UNLIKELY (ret < 0)) @@ -4474,9 +4471,9 @@ libcrun_container_update_intel_rdt (libcrun_context_t *context, const char *id, cleanup_free char *dir = NULL; int ret; - dir = libcrun_get_state_directory (context->state_root, id); - if (UNLIKELY (dir == NULL)) - return crun_make_error (err, 0, "cannot get state directory"); + ret = libcrun_get_state_directory (&dir, context->state_root, id, err); + if (UNLIKELY (ret < 0)) + return ret; ret = append_paths (&config_file, err, dir, "config.json", NULL); if (UNLIKELY (ret < 0)) diff --git a/src/libcrun/handlers/krun.c b/src/libcrun/handlers/krun.c index c1c870ea3c..4157411f5b 100644 --- a/src/libcrun/handlers/krun.c +++ b/src/libcrun/handlers/krun.c @@ -195,9 +195,9 @@ libkrun_configure_container (void *cookie, enum handler_configure_phase phase, cleanup_free char *config = NULL; size_t config_size; - state_dir = libcrun_get_state_directory (context->state_root, context->id); - if (UNLIKELY (state_dir == NULL)) - return crun_make_error (err, 0, "could not retrieve the state directory"); + ret = libcrun_get_state_directory (&state_dir, context->state_root, context->id, err); + if (UNLIKELY (ret < 0)) + return ret; ret = append_paths (&origin_config_path, err, state_dir, "config.json", NULL); if (UNLIKELY (ret < 0)) diff --git a/src/libcrun/linux.c b/src/libcrun/linux.c index 2c70a2428a..acfb5b0bbe 100644 --- a/src/libcrun/linux.c +++ b/src/libcrun/linux.c @@ -2362,7 +2362,9 @@ get_notify_fd (libcrun_context_t *context, libcrun_container_t *container, int * if (host_path == NULL) { - state_dir = libcrun_get_state_directory (context->state_root, context->id); + ret = libcrun_get_state_directory (&state_dir, context->state_root, context->id, err); + if (UNLIKELY (ret < 0)) + return ret; ret = append_paths (&host_notify_socket_path, err, state_dir, "notify/notify", NULL); if (UNLIKELY (ret < 0)) @@ -2406,7 +2408,7 @@ do_notify_socket (libcrun_container_t *container, const char *rootfs, libcrun_er const char *notify_socket = container->context->notify_socket; cleanup_free char *host_notify_socket_path = NULL; cleanup_free char *container_notify_socket_path = NULL; - cleanup_free char *state_dir = libcrun_get_state_directory (container->context->state_root, container->context->id); + cleanup_free char *state_dir = NULL; uid_t container_root_uid = -1; gid_t container_root_gid = -1; int notify_socket_tree_fd; @@ -2414,6 +2416,10 @@ do_notify_socket (libcrun_container_t *container, const char *rootfs, libcrun_er if (notify_socket == NULL) return 0; + ret = libcrun_get_state_directory (&state_dir, container->context->state_root, container->context->id, err); + if (UNLIKELY (ret < 0)) + return ret; + ret = append_paths (&container_notify_socket_path, err, rootfs, notify_socket, "notify", NULL); if (UNLIKELY (ret < 0)) return ret; @@ -4284,9 +4290,9 @@ prepare_and_send_dev_mounts (libcrun_container_t *container, int sync_socket_hos if (! has_userns || is_empty_string (container->context->id) || geteuid () > 0) return send_mounts (sync_socket_host, dev_fds, how_many, def->linux->devices_len, err); - state_dir = libcrun_get_state_directory (container->context->state_root, container->context->id); - if (state_dir == NULL) - return send_mounts (sync_socket_host, dev_fds, how_many, def->linux->devices_len, err); + ret = libcrun_get_state_directory (&state_dir, container->context->state_root, container->context->id, err); + if (UNLIKELY (ret < 0)) + return ret; ret = append_paths (&devs_path, err, state_dir, "devs", NULL); if (UNLIKELY (ret < 0)) diff --git a/src/libcrun/seccomp.c b/src/libcrun/seccomp.c index ae50f137a5..cf76735aa9 100644 --- a/src/libcrun/seccomp.c +++ b/src/libcrun/seccomp.c @@ -456,10 +456,11 @@ open_rundir_dirfd (const char *state_root, libcrun_error_t *err) { cleanup_free char *dir = NULL; int dirfd; + int ret; - dir = libcrun_get_state_directory (state_root, NULL); - if (UNLIKELY (dir == NULL)) - return crun_make_error (err, 0, "cannot get state directory"); + ret = libcrun_get_state_directory (&dir, state_root, NULL, err); + if (UNLIKELY (ret < 0)) + return ret; dirfd = TEMP_FAILURE_RETRY (open (dir, O_PATH | O_DIRECTORY | O_CLOEXEC)); if (UNLIKELY (dirfd < 0)) diff --git a/src/libcrun/status.c b/src/libcrun/status.c index 47d913b061..836104cfbe 100644 --- a/src/libcrun/status.c +++ b/src/libcrun/status.c @@ -32,18 +32,34 @@ #define YAJL_STR(x) ((const unsigned char *) (x)) +#define STEAL_POINTER(x, y) \ + do \ + { \ + *x = y; \ + y = NULL; \ + } while (0) + struct pid_stat { char state; unsigned long long starttime; }; -static char * -get_run_directory (const char *state_root) +/* If ID is not NULL, then ennsure that it does not contain any slash. */ +static int +validate_id (const char *id, libcrun_error_t *err) +{ + if (id && strchr (id, '/') != NULL) + return crun_make_error (err, 0, "invalid character `/` in the ID `%s`", id); + + return 0; +} + +static int +get_run_directory (char **out, const char *state_root, libcrun_error_t *err) { int ret; - char *root = NULL; - libcrun_error_t err = NULL; + cleanup_free char *root = NULL; if (state_root) root = xstrdup (state_root); @@ -52,57 +68,69 @@ get_run_directory (const char *state_root) const char *runtime_dir = getenv ("XDG_RUNTIME_DIR"); if (runtime_dir && runtime_dir[0] != '\0') { - ret = append_paths (&root, &err, runtime_dir, "crun", NULL); + ret = append_paths (&root, err, runtime_dir, "crun", NULL); if (UNLIKELY (ret < 0)) - { - crun_error_release (&err); - return NULL; - } + return ret; } } if (root == NULL) root = xstrdup ("/run/crun"); - ret = crun_ensure_directory (root, 0700, false, &err); + ret = crun_ensure_directory (root, 0700, false, err); if (UNLIKELY (ret < 0)) - crun_error_release (&err); - return root; + return ret; + + STEAL_POINTER (out, root); + + return 0; } -char * -libcrun_get_state_directory (const char *state_root, const char *id) +int +libcrun_get_state_directory (char **out, const char *state_root, const char *id, libcrun_error_t *err) { int ret; - char *path; - libcrun_error_t *err = NULL; - cleanup_free char *root = get_run_directory (state_root); + cleanup_free char *path = NULL; + cleanup_free char *root = NULL; + + ret = validate_id (id, err); + if (UNLIKELY (ret < 0)) + return ret; + + ret = get_run_directory (&root, state_root, err); + if (UNLIKELY (ret < 0)) + return ret; ret = append_paths (&path, err, root, id, NULL); if (UNLIKELY (ret < 0)) - { - crun_error_release (err); - return NULL; - } + return ret; + + STEAL_POINTER (out, path); - return path; + return 0; } -static char * -get_state_directory_status_file (const char *state_root, const char *id) +static int +get_state_directory_status_file (char **out, const char *state_root, const char *id, libcrun_error_t *err) { - cleanup_free char *root = get_run_directory (state_root); - libcrun_error_t *err = NULL; + cleanup_free char *root = NULL; char *path = NULL; int ret; + ret = validate_id (id, err); + if (UNLIKELY (ret < 0)) + return ret; + + ret = get_run_directory (&root, state_root, err); + if (UNLIKELY (ret < 0)) + return ret; + ret = append_paths (&path, err, root, id, "status", NULL); if (UNLIKELY (ret < 0)) - { - crun_error_release (err); - return NULL; - } + return ret; - return path; + STEAL_POINTER (out, path); + + return 0; } static int @@ -173,8 +201,8 @@ libcrun_write_container_status (const char *state_root, const char *id, libcrun_ libcrun_error_t *err) { int r, ret; - cleanup_free char *file = get_state_directory_status_file (state_root, id); cleanup_free char *file_tmp = NULL; + cleanup_free char *file = NULL; size_t len; cleanup_close int fd_write = -1; const unsigned char *buf = NULL; @@ -182,6 +210,10 @@ libcrun_write_container_status (const char *state_root, const char *id, libcrun_ const char *tmp; yajl_gen gen = NULL; + ret = get_state_directory_status_file (&file, state_root, id, err); + if (UNLIKELY (ret < 0)) + return ret; + ret = read_pid_stat (status->pid, &st, err); if (UNLIKELY (ret < 0)) return ret; @@ -348,9 +380,13 @@ libcrun_read_container_status (libcrun_container_status_t *status, const char *s cleanup_free char *buffer = NULL; char err_buffer[256]; int ret; - cleanup_free char *file = get_state_directory_status_file (state_root, id); + cleanup_free char *file = NULL; yajl_val tree, tmp; + ret = get_state_directory_status_file (&file, state_root, id, err); + if (UNLIKELY (ret < 0)) + return ret; + ret = read_all_file (file, &buffer, NULL, err); if (UNLIKELY (ret < 0)) return ret; @@ -437,18 +473,22 @@ libcrun_read_container_status (libcrun_container_status_t *status, const char *s int libcrun_status_check_directories (const char *state_root, const char *id, libcrun_error_t *err) { + cleanup_free char *run_directory = NULL; cleanup_free char *dir = NULL; - cleanup_free char *run_directory = get_run_directory (state_root); int ret; + ret = get_run_directory (&run_directory, state_root, err); + if (UNLIKELY (ret < 0)) + return ret; + libcrun_debug ("Checking run directory: %s", run_directory); ret = crun_ensure_directory (run_directory, 0700, false, err); if (UNLIKELY (ret < 0)) return ret; - dir = libcrun_get_state_directory (state_root, id); - if (UNLIKELY (dir == NULL)) - return crun_make_error (err, 0, "cannot get state directory"); + ret = libcrun_get_state_directory (&dir, state_root, id, err); + if (UNLIKELY (ret < 0)) + return ret; ret = crun_path_exists (dir, err); if (UNLIKELY (ret < 0)) @@ -529,9 +569,13 @@ libcrun_container_delete_status (const char *state_root, const char *id, libcrun cleanup_close int dfd = -1; cleanup_free char *dir = NULL; - dir = get_run_directory (state_root); - if (UNLIKELY (dir == NULL)) - return crun_make_error (err, 0, "cannot get state directory"); + ret = validate_id (id, err); + if (UNLIKELY (ret < 0)) + return ret; + + ret = get_run_directory (&dir, state_root, err); + if (UNLIKELY (ret < 0)) + return ret; rundir_dfd = TEMP_FAILURE_RETRY (open (dir, O_DIRECTORY | O_PATH | O_CLOEXEC)); if (UNLIKELY (rundir_dfd < 0)) @@ -572,21 +616,27 @@ libcrun_free_container_status (libcrun_container_status_t *status) } int -libcrun_get_containers_list (libcrun_container_list_t **ret, const char *state_root, libcrun_error_t *err) +libcrun_get_containers_list (libcrun_container_list_t **out, const char *state_root, libcrun_error_t *err) { struct dirent *next; cleanup_container_list libcrun_container_list_t *tmp = NULL; - cleanup_free char *path = get_run_directory (state_root); + cleanup_free char *root = NULL; cleanup_dir DIR *dir = NULL; + int ret; - *ret = NULL; - dir = opendir (path); + *out = NULL; + + ret = get_run_directory (&root, state_root, err); + if (UNLIKELY (ret < 0)) + return ret; + + dir = opendir (root); if (UNLIKELY (dir == NULL)) - return crun_make_error (err, errno, "cannot opendir `%s`", path); + return crun_make_error (err, errno, "cannot opendir `%s`", root); for (next = readdir (dir); next; next = readdir (dir)) { - int r, exists; + int exists; cleanup_free char *status_file = NULL; libcrun_container_list_t *next_container; @@ -594,9 +644,9 @@ libcrun_get_containers_list (libcrun_container_list_t **ret, const char *state_r if (next->d_name[0] == '.') continue; - r = append_paths (&status_file, err, path, next->d_name, "status", NULL); - if (UNLIKELY (r < 0)) - return r; + ret = append_paths (&status_file, err, root, next->d_name, "status", NULL); + if (UNLIKELY (ret < 0)) + return ret; exists = crun_path_exists (status_file, err); if (exists < 0) @@ -615,8 +665,9 @@ libcrun_get_containers_list (libcrun_container_list_t **ret, const char *state_r next_container->next = tmp; tmp = next_container; } - *ret = tmp; - tmp = NULL; + + STEAL_POINTER (out, tmp); + return 0; } @@ -677,10 +728,14 @@ libcrun_is_container_running (libcrun_container_status_t *status, libcrun_error_ int libcrun_status_create_exec_fifo (const char *state_root, const char *id, libcrun_error_t *err) { - cleanup_free char *state_dir = libcrun_get_state_directory (state_root, id); + cleanup_free char *state_dir = NULL; cleanup_free char *fifo_path = NULL; int ret, fd = -1; + ret = libcrun_get_state_directory (&state_dir, state_root, id, err); + if (UNLIKELY (ret < 0)) + return ret; + ret = append_paths (&fifo_path, err, state_dir, "exec.fifo", NULL); if (UNLIKELY (ret < 0)) return ret; @@ -700,7 +755,7 @@ libcrun_status_create_exec_fifo (const char *state_root, const char *id, libcrun int libcrun_status_write_exec_fifo (const char *state_root, const char *id, libcrun_error_t *err) { - cleanup_free char *state_dir = libcrun_get_state_directory (state_root, id); + cleanup_free char *state_dir = NULL; cleanup_free char *fifo_path = NULL; char buffer[1] = { 0, @@ -708,6 +763,10 @@ libcrun_status_write_exec_fifo (const char *state_root, const char *id, libcrun_ cleanup_close int fd = -1; int ret; + ret = libcrun_get_state_directory (&state_dir, state_root, id, err); + if (UNLIKELY (ret < 0)) + return ret; + ret = append_paths (&fifo_path, err, state_dir, "exec.fifo", NULL); if (UNLIKELY (ret < 0)) return ret; @@ -730,10 +789,14 @@ libcrun_status_write_exec_fifo (const char *state_root, const char *id, libcrun_ int libcrun_status_has_read_exec_fifo (const char *state_root, const char *id, libcrun_error_t *err) { - cleanup_free char *state_dir = libcrun_get_state_directory (state_root, id); + cleanup_free char *state_dir = NULL; cleanup_free char *fifo_path = NULL; int ret; + ret = libcrun_get_state_directory (&state_dir, state_root, id, err); + if (UNLIKELY (ret < 0)) + return ret; + ret = append_paths (&fifo_path, err, state_dir, "exec.fifo", NULL); if (UNLIKELY (ret < 0)) return ret; diff --git a/src/libcrun/status.h b/src/libcrun/status.h index 70f33139e7..cd6c0ced16 100644 --- a/src/libcrun/status.h +++ b/src/libcrun/status.h @@ -55,7 +55,7 @@ LIBCRUN_PUBLIC int libcrun_read_container_status (libcrun_container_status_t *st const char *id, libcrun_error_t *err); LIBCRUN_PUBLIC void libcrun_free_containers_list (libcrun_container_list_t *list); LIBCRUN_PUBLIC int libcrun_is_container_running (libcrun_container_status_t *status, libcrun_error_t *err); -LIBCRUN_PUBLIC char *libcrun_get_state_directory (const char *state_root, const char *id); +LIBCRUN_PUBLIC int libcrun_get_state_directory (char **out, const char *state_root, const char *id, libcrun_error_t *err); LIBCRUN_PUBLIC int libcrun_container_delete_status (const char *state_root, const char *id, libcrun_error_t *err); LIBCRUN_PUBLIC int libcrun_get_containers_list (libcrun_container_list_t **ret, const char *state_root, libcrun_error_t *err); diff --git a/tests/test_start.py b/tests/test_start.py index da74820c26..9b0744275f 100755 --- a/tests/test_start.py +++ b/tests/test_start.py @@ -540,6 +540,22 @@ def test_run_keep(): return 0 +def test_invalid_id(): + conf = base_config() + conf['process']['args'] = ['./init', 'echo', 'hello'] + conf['process']['cwd'] = "/sbin" + add_all_namespaces(conf) + try: + out, _ = run_and_get_output(conf, id_container="this/is/invalid") + return -1 + except Exception as e: + err = e.output.decode() + if "invalid character `/` in the ID" in err: + return 0 + sys.stderr.write("Got error: %s\n" % err) + return -1 + return 0 + all_tests = { "start" : test_start, "start-override-config" : test_start_override_config, @@ -562,6 +578,7 @@ def test_run_keep(): "unknown-sysctl": test_unknown_sysctl, "ioprio": test_ioprio, "run-keep": test_run_keep, + "invalid-id": test_invalid_id, } if __name__ == "__main__":