Move notify handler before mounts to primary uid namespace

[karuboniru]

containers/podman#27822

In krun, the clone3 call places the process in the mapped namespace where it lacks sufficient permissions to read or write the container's definition file. This issue can be resolved by moving the handler invocation to occur before the clone3 call.

Existing handlers in crun are unaffected, as none—except for krun—process actions in HANDLER_CONFIGURE_BEFORE_MOUNTS, so should not break other handlers.

[gemini-code-assist]

Summary of Changes

Hello @karuboniru, 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 addresses a critical permission issue encountered by the krun handler within crun when attempting to access container definition files. By strategically reordering the invocation of the HANDLER_CONFIGURE_BEFORE_MOUNTS notification handler to an earlier stage in the container setup, before the process enters a mapped namespace, krun can now operate with the necessary permissions. This ensures the correct functionality of krun without negatively impacting other existing crun handlers.

Highlights

• Handler Relocation: The libcrun_container_notify_handler for HANDLER_CONFIGURE_BEFORE_MOUNTS has been moved from container_init_setup to libcrun_run_linux_container.
• Permission Fix for krun: This relocation resolves a permission issue where the krun handler could not read or write container definition files because it was placed in a mapped namespace too early by the clone3 call.
• Execution Order Adjustment: The handler now executes earlier in the container initialization process, specifically before the clone3 call, ensuring it operates within the primary UID namespace with sufficient permissions.
• Minimal Impact: The change is targeted at krun's specific needs and is not expected to affect other crun handlers, as they do not process actions in HANDLER_CONFIGURE_BEFORE_MOUNTS.

🧠 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.

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.

Footnotes

1. 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

This pull request moves the libcrun_container_notify_handler call for the HANDLER_CONFIGURE_BEFORE_MOUNTS phase to an earlier stage, before the container process is cloned. This change correctly addresses a permissions issue for the krun handler, which needs to access container definition files from the primary user namespace. The overall logic is sound. I have one suggestion to prevent a potential null pointer dereference when accessing the container's root path.

[gemini-code-assist]

The root object in the container definition can be NULL, especially when a custom handler is used. Accessing container->container_def->root->path without checking if container->container_def->root is NULL could lead to a null pointer dereference. It's safer to check for its existence before accessing the path member.

  ret = libcrun_container_notify_handler (args, HANDLER_CONFIGURE_BEFORE_MOUNTS, container, container->container_def->root ? container->container_def->root->path : NULL, err);

[packit-as-a-service]

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

[giuseppe]

please use your real name for the commit and the Signed-off-by line

[karuboniru]

updated

[giuseppe]

thanks. Looking more into this, I've a doubt, what if some users would prefer to be notified after the user namespace is configured?

Would it be cleaner to have another hook? Something like HANDLER_CONFIGURE_BEFORE_USERNS?

[karuboniru]

Like the updated version?

[giuseppe]

LGTM