Repository navigation
podstorage: Make the storage bind mount namespace private - #2588
Conversation
bind_storage_roots() unshares a mount namespace in the podman child and bind mounts the storage root onto /run/bootc/storage, but never changes the propagation of the new namespace. `install --source-imgref` skips our usual unshare, so when run directly on a systemd host, where / and /run are shared, the child's mounts are peers of the host's and the bind propagates back. It stays mounted on the host's /run/bootc/storage (and /target/run/bootc/storage) after bootc exits. If the storage directory is later removed, e.g. `rm -rf /ostree` before retrying a failed install, the leaked mount's root is deleted and every following bind onto /run/bootc/storage fails with ENOENT until reboot: error: Installing to filesystem: Creating imgstorage: Initializing images: No such file or directory (os error 2) Mark the new namespace recursively private right after the unshare, as ChrootCmd already does, so the bind stays inside the child. Add a booted test that installs with / made shared inside run_install's MountFlags=slave namespace and checks that /run/bootc/storage is not left mounted. Closes: bootc-dev#2455 Assisted-by: AI Signed-off-by: Shubham Samrat <shubhamsamrat382@gmail.com>
cgwalters
left a comment
cgwalters
left a comment
There was a problem hiding this comment.
Hmm...I think we should be doing the unshare in the to-existing-root case most likely too.
| # duration: 30m | ||
| # extra: | ||
| # # bootloader=none is not supported for composefs (see test-install-no-boot-dir.nu) | ||
| # fixme_skip_if_composefs: true |
There was a problem hiding this comment.
This doesn't seem strictly necessary?
|
|
||
| tap run_install $"mount --make-rshared / | ||
| bootc install to-filesystem --disable-selinux --bootloader=none --source-imgref ($target_image) /var/mnt | ||
| if findmnt /run/bootc/storage; then echo 'error: /run/bootc/storage mount leaked'; exit 1; fi" |
There was a problem hiding this comment.
I think this could be glommed onto an existing test, but ok for now.
| // https://github.com/bootc-dev/bootc/issues/2455 | ||
| rustix::mount::mount_change( | ||
| c"/", | ||
| rustix::mount::MountPropagationFlags::PRIVATE |
There was a problem hiding this comment.
I think this is fine, but probably going forward we should actually prefer SLAVE so we do pick up mounts from the host into our mountns. But it's unlikely to matter because the process here is short-lived.
|
Thanks for the review and the merge! Agreed on the I can also switch |
What
Make the mount namespace that
bind_storage_roots()creates for podman recursively private right after the unshare, asChrootCmdalready does. The bind onto/run/bootc/storagethen stays inside the child.Add a booted tmt test (
test-install-shared-propagation) that makes/shared insiderun_install'sMountFlags=slavenamespace, installs with--source-imgref, and checks that/run/bootc/storageisn't left mounted.Closes: #2455
Why
install --source-imgrefskips bootc's own unshare. On a bare systemd host, where/and/runare shared, the podman child's bind propagates back and stays mounted on the host's/run/bootc/storageand/target/run/bootc/storageafter bootc exits. If the storage directory is then removed, for examplerm -rf /ostreebefore a retry, the leaked mount points at a deleted directory, and every later install fails withCreating imgstorage: Initializing images: No such file or directory (os error 2)until reboot. Details and strace output are in #2455 (comment).Testing
On fresh Fedora 44 VMs (aarch64, SELinux enforcing), I ran
to-existing-root --source-imgref. First I installed with a missing image tag, which fails after the image store is created. Then I ranrm -rf /ostreeand did a full install:/run/bootc/storagemountsInstallation complete!I ran the new test's steps by hand on Fedora 44, because I can't run tmt locally without nested virt. They pass with this PR and fail on 1.17.1 with
error: /run/bootc/storage mount leaked. I'm relying on CI for the real tmt run.make validateandcargo test -p bootc-lib --libpass.Not covered here
/targetbind of the host root also stays mounted after a--source-imgrefto-existing-root on a bare host. Unsharing in that path would cover it. I'm happy to do that as a follow-up if you want it./run/bootc/storagemount from an older bootc still needs a reboot, or oneumount /run/bootc/storage.Assisted-by: AI. I reproduced the bug and reviewed and tested every change myself.