Skip to content

criu: fix missing umount() in error path #2019

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
giuseppe merged 1 commit into from
Feb 19, 2026
+64 −17
Merged

criu: fix missing umount() in error path #2019

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 64 additions & 17 deletions src/libcrun/criu.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1087,66 +1087,100 @@ libcrun_container_restore_linux_criu (libcrun_container_status_t *status, libcru
const int open_flags_for_inherit = O_RDONLY; /* Cannot be O_CLOEXEC as it is passed to the child process. */
int value = libcrun_find_namespace (def->linux->namespaces[i]->type);
if (UNLIKELY (value < 0))
return crun_make_error (err, 0, "invalid namespace type: `%s`", def->linux->namespaces[i]->type);
Copy link

@gemini-code-assist gemini-code-assist bot Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This change correctly routes the error to the out_umount label, ensuring proper cleanup is performed before the function exits. This is a critical improvement for resource management.

          ret = crun_make_error (err, 0, "invalid namespace type: `%s`", def->linux->namespaces[i]->type);
          goto out_umount;

{
ret = crun_make_error (err, 0, "invalid namespace type: `%s`", def->linux->namespaces[i]->type);
goto out_umount;
}

if (value == CLONE_NEWNET && def->linux->namespaces[i]->path != NULL)
{
inherit_new_net_fd = open (def->linux->namespaces[i]->path, open_flags_for_inherit);
if (UNLIKELY (inherit_new_net_fd < 0))
return crun_make_error (err, errno, "unable to open(): `%s`", def->linux->namespaces[i]->path);
Copy link

@gemini-code-assist gemini-code-assist bot Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Changing the direct return to a goto out_umount ensures that the umount cleanup routine is executed in case of an error opening the network namespace path. This prevents potential resource leaks.

            ret = crun_make_error (err, errno, "unable to open(): `%s`", def->linux->namespaces[i]->path);
            goto out_umount;

{
ret = crun_make_error (err, errno, "unable to open(): `%s`", def->linux->namespaces[i]->path);
goto out_umount;
}

ret = libcriu_wrapper->criu_add_inherit_fd (inherit_new_net_fd, CRIU_EXT_NETNS);
if (UNLIKELY (ret < 0))
return crun_make_error (err, -ret, "CRIU: failed adding fd");
Copy link

@gemini-code-assist gemini-code-assist bot Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This modification correctly directs the error flow to the out_umount label, which is essential for consistent error handling and resource cleanup.

            ret = crun_make_error (err, -ret, "CRIU: failed adding fd");
            goto out_umount;

{
ret = crun_make_error (err, -ret, "CRIU: failed adding fd");
goto out_umount;
}
}

if (value == CLONE_NEWPID && def->linux->namespaces[i]->path != NULL)
{
inherit_new_pid_fd = open (def->linux->namespaces[i]->path, open_flags_for_inherit);
if (UNLIKELY (inherit_new_pid_fd < 0))
return crun_make_error (err, errno, "unable to open(): `%s`", def->linux->namespaces[i]->path);
Copy link

@gemini-code-assist gemini-code-assist bot Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Ensuring that errors during PID namespace path opening lead to out_umount is vital for preventing resource leaks and maintaining a clean state.

            ret = crun_make_error (err, errno, "unable to open(): `%s`", def->linux->namespaces[i]->path);
            goto out_umount;

{
ret = crun_make_error (err, errno, "unable to open(): `%s`", def->linux->namespaces[i]->path);
goto out_umount;
}

ret = libcriu_wrapper->criu_add_inherit_fd (inherit_new_pid_fd, CRIU_EXT_PIDNS);
if (UNLIKELY (ret < 0))
return crun_make_error (err, -ret, "CRIU: failed adding fd");
Copy link

@gemini-code-assist gemini-code-assist bot Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This change correctly routes the error to the out_umount label, ensuring proper cleanup is performed before the function exits.

            ret = crun_make_error (err, -ret, "CRIU: failed adding fd");
            goto out_umount;

{
ret = crun_make_error (err, -ret, "CRIU: failed adding fd");
goto out_umount;
}
}

# ifdef CRIU_JOIN_NS_SUPPORT
if (value == CLONE_NEWTIME && def->linux->namespaces[i]->path != NULL)
{
if (libcriu_wrapper->criu_join_ns_add == NULL)
return crun_make_error (err, 0, "shared time namespace restore is supported in CRIU >= 3.16.1");
Copy link

@gemini-code-assist gemini-code-assist bot Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This ensures that if criu_join_ns_add is not available for time namespaces, the error path correctly leads to out_umount for cleanup.

            ret = crun_make_error (err, 0, "shared time namespace restore is supported in CRIU >= 3.16.1");
            goto out_umount;

{
ret = crun_make_error (err, 0, "shared time namespace restore is supported in CRIU >= 3.16.1");
goto out_umount;
}

ret = libcriu_wrapper->criu_join_ns_add ("time", def->linux->namespaces[i]->path, NULL);
if (UNLIKELY (ret < 0))
return crun_make_error (err, -ret, "CRIU: failed adding external namespace `%s`", def->linux->namespaces[i]->path);
Copy link

@gemini-code-assist gemini-code-assist bot Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Properly handling errors from criu_join_ns_add by jumping to out_umount is essential for maintaining resource integrity.

            ret = crun_make_error (err, -ret, "CRIU: failed adding external namespace `%s`", def->linux->namespaces[i]->path);
            goto out_umount;

{
ret = crun_make_error (err, -ret, "CRIU: failed adding external namespace `%s`", def->linux->namespaces[i]->path);
goto out_umount;
}
}

if (value == CLONE_NEWIPC && def->linux->namespaces[i]->path != NULL)
{
if (libcriu_wrapper->criu_join_ns_add == NULL)
return crun_make_error (err, 0, "shared ipc namespace restore is supported in CRIU >= 3.16.1");
Copy link

@gemini-code-assist gemini-code-assist bot Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This ensures that if criu_join_ns_add is not available for IPC namespaces, the error path correctly leads to out_umount for cleanup.

            ret = crun_make_error (err, 0, "shared ipc namespace restore is supported in CRIU >= 3.16.1");
            goto out_umount;

{
ret = crun_make_error (err, 0, "shared ipc namespace restore is supported in CRIU >= 3.16.1");
goto out_umount;
}

ret = libcriu_wrapper->criu_join_ns_add ("ipc", def->linux->namespaces[i]->path, NULL);
if (UNLIKELY (ret < 0))
return crun_make_error (err, -ret, "CRIU: failed adding external namespace `%s`", def->linux->namespaces[i]->path);
Copy link

@gemini-code-assist gemini-code-assist bot Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Properly handling errors from criu_join_ns_add by jumping to out_umount is essential for maintaining resource integrity.

            ret = crun_make_error (err, -ret, "CRIU: failed adding external namespace `%s`", def->linux->namespaces[i]->path);
            goto out_umount;

{
ret = crun_make_error (err, -ret, "CRIU: failed adding external namespace `%s`", def->linux->namespaces[i]->path);
goto out_umount;
}
}

if (value == CLONE_NEWUTS && def->linux->namespaces[i]->path != NULL)
{
if (libcriu_wrapper->criu_join_ns_add == NULL)
return crun_make_error (err, 0, "shared uts namespace restore is supported in CRIU >= 3.16.1");
Copy link

@gemini-code-assist gemini-code-assist bot Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This ensures that if criu_join_ns_add is not available for UTS namespaces, the error path correctly leads to out_umount for cleanup.

            ret = crun_make_error (err, 0, "shared uts namespace restore is supported in CRIU >= 3.16.1");
            goto out_umount;

{
ret = crun_make_error (err, 0, "shared uts namespace restore is supported in CRIU >= 3.16.1");
goto out_umount;
}

ret = libcriu_wrapper->criu_join_ns_add ("uts", def->linux->namespaces[i]->path, NULL);
if (UNLIKELY (ret < 0))
return crun_make_error (err, -ret, "CRIU: failed adding external namespace `%s`", def->linux->namespaces[i]->path);
Copy link

@gemini-code-assist gemini-code-assist bot Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Properly handling errors from criu_join_ns_add by jumping to out_umount is essential for maintaining resource integrity.

            ret = crun_make_error (err, -ret, "CRIU: failed adding external namespace `%s`", def->linux->namespaces[i]->path);
            goto out_umount;

{
ret = crun_make_error (err, -ret, "CRIU: failed adding external namespace `%s`", def->linux->namespaces[i]->path);
goto out_umount;
}
}
# endif
}

/* Set up CRIU config file */
if (UNLIKELY (handle_criu_config_file (container, err)))
return -1;
Comment on lines 1148 to -1149
Copy link

@gemini-code-assist gemini-code-assist bot Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This change correctly integrates the handle_criu_config_file error into the out_umount cleanup path, ensuring consistent error handling.

  ret = handle_criu_config_file (container, err);
  if (UNLIKELY (ret < 0))
    goto out_umount;

ret = handle_criu_config_file (container, err);
if (UNLIKELY (ret < 0))
goto out_umount;

/* Tell CRIU if cgroup v1 needs to be handled. */
ret = restore_cgroup_v1_mount (def, err);
Expand All @@ -1168,7 +1202,10 @@ libcrun_container_restore_linux_criu (libcrun_container_status_t *status, libcru
{
ret = libcriu_wrapper->criu_add_cg_root (NULL, status->cgroup_path);
if (UNLIKELY (ret != 0))
return crun_make_error (err, 0, "error setting CRIU cgroup root to `%s`", status->cgroup_path);
Copy link

@gemini-code-assist gemini-code-assist bot Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Routing this error to out_umount ensures that any resources allocated before this point are properly cleaned up, which is crucial for stability.

            ret = crun_make_error (err, 0, "error setting CRIU cgroup root to `%s`", status->cgroup_path);
            goto out_umount;

{
ret = crun_make_error (err, 0, "error setting CRIU cgroup root to `%s`", status->cgroup_path);
goto out_umount;
}
}

if (cr_options->manage_cgroups_mode == -1)
Expand All @@ -1182,13 +1219,19 @@ libcrun_container_restore_linux_criu (libcrun_container_status_t *status, libcru
{
ret = libcriu_wrapper->criu_set_network_lock (cr_options->network_lock_method);
if (UNLIKELY (ret < 0))
return crun_make_error (err, 0, "CRIU: failed setting network lock");
Copy link

@gemini-code-assist gemini-code-assist bot Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This change correctly integrates the criu_set_network_lock error into the out_umount cleanup path, ensuring consistent error handling.

            ret = crun_make_error (err, 0, "CRIU: failed setting network lock");
            goto out_umount;

{
ret = crun_make_error (err, 0, "CRIU: failed setting network lock");
goto out_umount;
}
}

libcriu_wrapper->criu_set_log_level (4);
ret = libcriu_wrapper->criu_set_log_file (CRIU_RESTORE_LOG_FILE);
if (UNLIKELY (ret < 0))
return crun_make_error (err, -ret, "error setting CRIU log file to `%s`", CRIU_RESTORE_LOG_FILE);
Copy link

@gemini-code-assist gemini-code-assist bot Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Ensuring that errors during setting the CRIU log file lead to out_umount is vital for preventing resource leaks and maintaining a clean state.

    ret = crun_make_error (err, -ret, "error setting CRIU log file to `%s`", CRIU_RESTORE_LOG_FILE);
    goto out_umount;

{
ret = crun_make_error (err, -ret, "error setting CRIU log file to `%s`", CRIU_RESTORE_LOG_FILE);
goto out_umount;
}

/* criu_restore() returns the PID of the process of the restored process
* tree. This PID will not be the same as status->pid if the container is
Expand All @@ -1212,8 +1255,12 @@ libcrun_container_restore_linux_criu (libcrun_container_status_t *status, libcru
ret_out = umount (root);
if (UNLIKELY (ret_out == -1))
{
int saved_errno = errno;
rmdir (root);
return crun_make_error (err, errno, "error unmounting restore directory `%s`", root);
if (ret < 0)
return crun_error_wrap (err, "error unmounting restore directory `%s`", root);
else
return crun_make_error (err, saved_errno, "error unmounting restore directory `%s`", root);
}
out:
ret_out = rmdir (root);
Expand Down