criu: fix missing `umount()` in error path by eriksjolund · Pull Request #2019 · containers/crun

eriksjolund · 2026-02-15T12:32:05Z

Fix missing umount() in error path.

Fix possible error leak.

Closes: #2020

gemini-code-assist · 2026-02-15T12:32:19Z

Summary of Changes

Hello @eriksjolund, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request improves the robustness of the CRIU container restoration process by ensuring that critical cleanup operations, specifically unmounting resources, are consistently performed whenever an error occurs. By modifying various error return paths to jump to a common cleanup label, the changes prevent resource leaks and enhance the stability of container lifecycle management under failure conditions.

Highlights

Error Handling Consistency: Standardized error handling in libcrun_container_restore_linux_criu to consistently use goto out_umount; for various failure conditions.
Resource Cleanup: Ensured that the umount cleanup routine is invoked in more error paths, preventing potential resource leaks during CRIU container restoration.
Namespace and CRIU Configuration Errors: Extended cleanup to cover errors related to invalid namespace types, failures in opening/adding inherited file descriptors, issues with joining namespaces, and problems setting up CRIU configuration, cgroup roots, network locks, and log files.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog

src/libcrun/criu.c
- Modified error handling for invalid namespace types to jump to out_umount.
- Updated error paths for opening and adding inherited file descriptors for network and PID namespaces to ensure out_umount is called.
- Adjusted error handling for failures when joining time, IPC, and UTS namespaces to consistently use goto out_umount.
- Changed the error return for handle_criu_config_file to goto out_umount.
- Modified error handling for criu_add_cg_root to jump to out_umount.
- Updated error paths for criu_set_network_lock and criu_set_log_file to ensure out_umount is called.

Activity

The author initiated the pull request with an idea to address seemingly missing goto out_umount; statements in various error paths, aiming to improve cleanup.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

The pull request correctly addresses missing goto out_umount; statements in various error paths within the libcrun_container_restore_linux_criu function. This is a crucial improvement for robust error handling and ensures that necessary cleanup, such as unmounting resources, is performed consistently before exiting the function. This change significantly enhances the reliability and resource management of the CRIU restore process.

gemini-code-assist · 2026-02-15T12:34:19Z

src/libcrun/criu.c

      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);


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;

gemini-code-assist · 2026-02-15T12:34:19Z

src/libcrun/criu.c

        {
          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);


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;

gemini-code-assist · 2026-02-15T12:34:19Z

src/libcrun/criu.c


          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");


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;

gemini-code-assist · 2026-02-15T12:34:19Z

src/libcrun/criu.c

        {
          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);


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;

gemini-code-assist · 2026-02-15T12:34:19Z

src/libcrun/criu.c


          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");


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;

gemini-code-assist · 2026-02-15T12:34:19Z

src/libcrun/criu.c


          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);


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;

gemini-code-assist · 2026-02-15T12:34:19Z

src/libcrun/criu.c

          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");
+            {
+              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);
+            {
+              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");
+            {
+              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);
+            {
+              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;


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;

gemini-code-assist · 2026-02-15T12:34:20Z

src/libcrun/criu.c

    {
      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);


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;

gemini-code-assist · 2026-02-15T12:34:20Z

src/libcrun/criu.c

    {
      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");


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;

gemini-code-assist · 2026-02-15T12:34:20Z

src/libcrun/criu.c

  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);


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;

eriksjolund · 2026-02-15T12:46:11Z

Okay, gemini-code-assist liked my edits. This makes me more confident. I now removed the draft status.

gemini-code-assist · 2026-02-15T12:46:22Z

That's great to hear, @eriksjolund! I'm glad the summary was helpful. I'll be proceeding with a more detailed review of the changes shortly. Thanks for removing the draft status!

packit-as-a-service · 2026-02-15T12:48:55Z

TMT tests failed. @containers/packit-build please check.

eriksjolund · 2026-02-15T13:04:02Z

I guess the PR is not correct because there are errors in CI.

Copy-paste from
https://artifacts.dev.testing-farm.io/e37942d2-3812-4baa-88c8-2e0d3b41fc8a/

# [12:48:19.825003520] # podman  container checkpoint ff006268c2772351eeacd04d0a1eb799260a583dbad5562ee3a5e81d44070052
# [12:48:19.907462292] CRIU checkpointing failed -52.  Please check CRIU logfile /var/lib/containers/storage/overlay-containers/ff006268c2772351eeacd04d0a1eb799260a583dbad5562ee3a5e81d44070052/userdata/dump.log: Invalid exchange
# Error: `/usr/bin/crun checkpoint --image-path /var/lib/containers/storage/overlay-containers/ff006268c2772351eeacd04d0a1eb799260a583dbad5562ee3a5e81d44070052/userdata/checkpoint --work-path /var/lib/containers/storage/overlay-containers/ff006268c2772351eeacd04d0a1eb799260a583dbad5562ee3a5e81d44070052/userdata ff006268c2772351eeacd04d0a1eb799260a583dbad5562ee3a5e81d44070052` failed: exit status 1
# [12:48:19.911386015] [ rc=125 (** EXPECTED 0 **) ]
# #/vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
# #| FAIL: exit code is 125; expected 0
# #\^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

eriksjolund · 2026-02-15T13:29:22Z

I noticed that if umount() fails, then the return values might not be the same as before.

crun/src/libcrun/criu.c

Lines 1212 to 1216 in 94fda0b

    
           ret_out = umount (root); 
        
           if (UNLIKELY (ret_out == -1)) 
        
             { 
        
               rmdir (root); 
        
               return crun_make_error (err, errno, "error unmounting restore directory `%s`", root);

I'm thinking about using use crun_error_wrap() on line 1216. That would also fix #2020

giuseppe · 2026-02-16T08:25:03Z

ret_out = umount (root);
if (UNLIKELY (ret_out == -1))
{
rmdir (root);
return crun_make_error (err, errno, "error unmounting restore directory %s", root);

do we just need to store errno before calling rmdir?

int saved_errno = errno;
rmdir (root);
return crun_make_error (err, saved_errno, "error unmounting restore directory `%s`", root);

eriksjolund · 2026-02-17T15:32:23Z

do we just need to store errno before calling rmdir?

int saved_errno = errno;
rmdir (root);
return crun_make_error (err, saved_errno, "error unmounting restore directory %s", root);

Yes, I think so. I pushed a new version where I tried to fix #2020 at the same time. I just want to see what happens with the CI tests.

Fix possible error leak. Closes: containers#2020 Signed-off-by: Erik Sjölund <erik.sjolund@gmail.com>

giuseppe

LGTM

giuseppe

LGTM

rst0git · 2026-02-19T10:38:29Z

@eriksjolund Thank you for fixing this!

gemini-code-assist bot reviewed Feb 15, 2026

View reviewed changes

eriksjolund marked this pull request as ready for review February 15, 2026 12:46

eriksjolund force-pushed the criu-add-goto branch from 02fdb36 to 2d202bf Compare February 17, 2026 15:31

criu: fix missing umount() in error path

11701f4

Fix possible error leak. Closes: containers#2020 Signed-off-by: Erik Sjölund <erik.sjolund@gmail.com>

eriksjolund force-pushed the criu-add-goto branch from 2d202bf to 11701f4 Compare February 17, 2026 15:37

giuseppe approved these changes Feb 17, 2026

View reviewed changes

giuseppe approved these changes Feb 19, 2026

View reviewed changes

giuseppe merged commit ad54f48 into containers:main Feb 19, 2026
43 of 47 checks passed

Comments

Conversation

eriksjolund commented Feb 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist bot commented Feb 15, 2026

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Feb 15, 2026

Choose a reason for hiding this comment

Uh oh!

eriksjolund commented Feb 15, 2026

Uh oh!

gemini-code-assist bot commented Feb 15, 2026

Uh oh!

packit-as-a-service bot commented Feb 15, 2026

Uh oh!

eriksjolund commented Feb 15, 2026

Uh oh!

eriksjolund commented Feb 15, 2026

Uh oh!

giuseppe commented Feb 16, 2026

Uh oh!

eriksjolund commented Feb 17, 2026

Uh oh!

giuseppe left a comment

Choose a reason for hiding this comment

Uh oh!

giuseppe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rst0git commented Feb 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

eriksjolund commented Feb 15, 2026 •

edited

Loading