libkrun, fs: Restrict the usage of REMOVE_ROOT_DIR_REQ ioctl

[mtjhrc]

Restrict the usage of VIRTIO_IOC_REMOVE_ROOT_DIR_REQ which deletes the root filesystem. Make it only available on the throwaway filesystem used by krun_set_root_disk_remount.

[gemini-code-assist]

Summary of Changes

Hello @mtjhrc, 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 implements a security enhancement by restricting the powerful VIRTIO_IOC_REMOVE_ROOT_DIR_REQ ioctl, which is capable of deleting the root filesystem. By introducing a new configuration flag, allow_root_dir_delete, the ability to perform this operation is now explicitly controlled, preventing unintended or malicious deletion of critical filesystems. This change significantly improves the robustness and security of the virtio-fs device handling within libkrun.

Highlights

• Security Enhancement: Introduced a new allow_root_dir_delete flag to explicitly control the execution of the VIRTIO_IOC_REMOVE_ROOT_DIR_REQ ioctl, which is capable of deleting the root filesystem.
• Restricted IOCTL Usage: The VIRTIO_IOC_REMOVE_ROOT_DIR_REQ ioctl is now conditionally executed only when the allow_root_dir_delete flag is set to true in the filesystem configuration.
• Configuration Updates: Updated libkrun and vmm components to incorporate and pass this new flag, ensuring that root directory deletion is only permitted for specific, intended use cases, such as throwaway filesystems managed by krun_set_root_disk_remount.

🧠 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/devices/src/virtio/fs/device.rs
  • Added allow_root_dir_delete parameter to the Fs::new constructor.
  • Included allow_root_dir_delete in the passthrough::Config initialization.
• src/devices/src/virtio/fs/linux/passthrough.rs
  • Added allow_root_dir_delete field to the Config struct.
  • Set allow_root_dir_delete to false by default in the Config::default implementation.
  • Modified the VIRTIO_IOC_REMOVE_ROOT_DIR_REQ handler to conditionally execute based on self.cfg.allow_root_dir_delete.
• src/devices/src/virtio/fs/macos/passthrough.rs
  • Added allow_root_dir_delete field to the Config struct.
  • Set allow_root_dir_delete to false by default in the Config::default implementation.
  • Modified the VIRTIO_IOC_REMOVE_ROOT_DIR_REQ handler to conditionally execute based on self.cfg.allow_root_dir_delete.
• src/libkrun/src/lib.rs
  • Added allow_root_dir_delete: false to FsDeviceConfig creation in krun_set_root, krun_add_virtiofs, and krun_add_virtiofs2.
  • Added allow_root_dir_delete: true to FsDeviceConfig creation in krun_set_root_disk_remount.
• src/vmm/src/builder.rs
  • Passed the newly introduced allow_root_dir_delete configuration to the Fs::new device constructor.
• src/vmm/src/vmm_config/fs.rs
  • Added a new allow_root_dir_delete boolean field to the FsDeviceConfig struct.

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

The pull request effectively restricts the VIRTIO_IOC_REMOVE_ROOT_DIR_REQ ioctl to only be available on filesystems explicitly configured to allow root directory deletion. This is a significant security and stability improvement, preventing accidental or malicious deletion of critical filesystems. The changes are consistently applied across Linux and macOS implementations and correctly propagate the new allow_root_dir_delete flag through the configuration structs and function calls. The default for new filesystems is false, which is a safe choice, while the specific krun_set_root_disk_remount function correctly sets it to true for its intended use case of a throwaway filesystem.

[slp]

Good one. I think we should go even further and completely get rid of this. Instead, we should allow Passthrough to operate without a root directory, just for opening /init.krun. Should be fairly simple, we just need to fabricate some bogus stat values for /.

[slp]

On a second thought, this approach is much simpler and fixes the issue just fine. Let's get it merged, thanks!

[mtjhrc]

I think we should go even further and completely get rid of this. Instead, we should allow Passthrough to operate without a root directory, just for opening /init.krun.

On a second thought, this approach is much simpler and fixes the issue just fine

Yeah pretty much this, while there is definitely a better way to implement this functionality, this was just a simple improvement for the current status, suitable for 1.7.x.