Allow image mounts to use mount options, such as noexec

[miyoyo]

Feature request description

As is, the parsing of image mount is (inexplicably?) different from other mount options, in that you cannot pass arguments, as, instead of using parseMountOptions, like glob, bind, tmpfs, ramfs, and volume mounts, it has it's own parsing rules like devpts.

Unlike devpts, image mounts are not a specialised filesystem, they are (according to the docs) just bind mounts, so, there is a disparity between --mount type=bind and --mount type=image.

There is no way to, for example, mount an image as noexec.

Suggest potential solution

Experimentally butchering podman's code does show that it supports mount options for image mounts:

		var overlayMount spec.Mount
		if volume.ReadWrite {
			overlayMount, err = overlay.Mount(contentDir, imagePath, volume.Dest, c.RootUID(), c.RootGID(), c.runtime.store.GraphOptions())
		} else {
			overlayMount, err = overlay.MountReadOnly(contentDir, imagePath, volume.Dest, c.RootUID(), c.RootGID(), c.runtime.store.GraphOptions())
		}
		if err != nil {
			return nil, nil, fmt.Errorf("creating overlay mount for image %q failed: %w", volume.Source, err)
		}
+		overlayMount.Options = append(overlayMount.Options, volume.Options...)
		g.AddMount(overlayMount)

(not showing propagating options through, but it's a copy paste of how other images do it)

Building and running podman does show that it works:

$ podman run --rm -it --security-opt=no-new-privileges --mount type=volume,source=data,target=/data,noexec --read-only --read-only-tmpfs=false --mount type=image,source=out,subpath=/var/lib/neo4j/conf,target=/var/lib/neo4j/conf,noexec=true --entrypoint /bin/bash out

neo4j@44f157292898:~$ mount | grep /var/lib/neo4j/conf
/home/hidden/.local/share/containers/storage/overlay-containers/44f1572928982bb238596fff941527eaf0b185d1d785dac11bfe9d3e115dfbe4/userdata/overlay/3189830976/merge on /var/lib/neo4j/conf type overlay (rw,noexec,relatime,lowerdir=/run/user/1000/containers/overlay-containers/44f1572928982bb238596fff941527eaf0b185d1d785dac11bfe9d3e115dfbe4/userdata/subpath1552514941,upperdir=/home/hidden/.local/share/containers/storage/overlay-containers/44f1572928982bb238596fff941527eaf0b185d1d785dac11bfe9d3e115dfbe4/userdata/overlay/3189830976/upper,workdir=/home/hidden/.local/share/containers/storage/overlay-containers/44f1572928982bb238596fff941527eaf0b185d1d785dac11bfe9d3e115dfbe4/userdata/overlay/3189830976/work,userxattr) 

I don't feel experienced enough with podman's codebase to push a pull request just yet, sorry.

Have you considered any alternatives?

Since this is a container that isn't supposed to have any privileges on the inside, remounting is not an option, I don't understand bind mounts sufficiently to mount an image manually via a --mount type=bind, although I assume it would work.

Additional context

No response

[miyoyo]

Tried my hand at a patch, you can see it in the referenced commit above, I'd appreciate feedback, or if someone else can grab that code and reformulate it into a proper pull request.

[Luap99]

Given it is an overlay mounts and rw is safe (and already supported) allowing users to pass other mounts options should be fine.
That said I don't think your patch to use parseMountOptions() will be correct, there are options that are not actual real mount option and thus must create a proper error, i.e. (relabel, U and some others) as these will not be ahandled int he image mount path. So this likely needs some rework to split out actual mount option parsing from the podman specific ones. And then just share the logic in the right places.

I am currently adding artifact mounts in #25397, so if we add this to image mounts I should add it there as well for consistency.

[miyoyo]

Given it is an overlay mounts and rw is safe (and already supported) allowing users to pass other mounts options should be fine. That said I don't think your patch to use parseMountOptions() will be correct, there are options that are not actual real mount option and thus must create a proper error, i.e. (relabel, U and some others) as these will not be ahandled int he image mount path. So this likely needs some rework to split out actual mount option parsing from the podman specific ones. And then just share the logic in the right places.

Thanks a lot for the feedback!

Seeing how the other mount types are mixed in without considering relabel or U, I'm wondering if parseMountOptions was the right option in the original code in the first place.

If that decision is to stay, the current way seems to be to add or remove switches depending on the mount type.

Here are the option maps:

option	restriction	special
bind-nonrecursive	Bind	✅
bind-propagation	Bind	✅
idmap	not restricted	✅
ro/rw	not restricted	❌
(no)dev	not restricted	❌
(no)exec	not restricted	❌
(no)suid	not restricted	❌
noswap	not restricted	❌
relabel	not restricted	✅
"shared","rshared", "private", "rprivate", "slave", "rslave", "unbindable", "runbindable", "Z", "z", "no-dereference"	not restricted	❌
src/source	TmpFS	✅
subpath/volume-subpath	Volume & Image	✅
target/dst/destination	not restricted	✅
tmpcopyup/notmpcopyup	TmpFS	❌
tmpfs-mode	TmpFS	❌
tmpfs-size	TmpFS	❌
U/chown	not restricted	✅
volume-label	Volume	❓
volume-opt	Volume	✅

volume-opt seems to be a cop-out to allow arbitrary mount options on volumes, I don't know if it's a good idea to keep that.

Out of all of these, here are the ones that could cause issues:

• idmap: I got it to work, or, at least, look like it works, but I don't seem to see it... do anything?
• relabel: Seems to work fine
• U/chown: Seems to work fine

Since everything else is gated, only these three need to be handled for image mounts, which is done in a similar way to how the other mount systems work already.

I feel like I don't understand these mounts enough, though.

[github-actions]

A friendly reminder that this issue had no activity for 30 days.