Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

linux: new mount option "idmap" #780

Merged
merged 6 commits into from
Nov 16, 2021
Merged

Conversation

giuseppe
Copy link
Member

when the "idmap" mount option is specified, create the mount outside of the container user namespace context and pass the mount fd to the container init process.

Signed-off-by: Giuseppe Scrivano gscrivan@redhat.com

giuseppe added a commit to giuseppe/libpod that referenced this pull request Nov 15, 2021
pass down the "idmap" mount option to the OCI runtime.

Needs: containers/crun#780

Closes: containers#12154

[NO NEW TESTS NEEDED] there is no crun version yet that support the
new feature.

Test case (must run as root):

podman run --rm -v foo:/foo alpine touch /foo/bar

podman run --uidmap 0:1:1000 --rm -v foo:/foo:idmap alpine ls -l /foo
total 0
-rw-r--r--    1 root     root             0 Nov 15 14:01 bar

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
@giuseppe giuseppe force-pushed the idmapped-mounts branch 2 times, most recently from 1494375 to cc04828 Compare November 15, 2021 14:14
giuseppe added a commit to giuseppe/libpod that referenced this pull request Nov 15, 2021
pass down the "idmap" mount option to the OCI runtime.

Needs: containers/crun#780

Closes: containers#12154

[NO NEW TESTS NEEDED] there is no crun version yet that support the
new feature.

Test case (must run as root):

podman run --rm -v foo:/foo alpine touch /foo/bar

podman run --uidmap 0:1:1000 --rm -v foo:/foo:idmap alpine ls -l /foo
total 0
-rw-r--r--    1 root     root             0 Nov 15 14:01 bar

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
@giuseppe giuseppe force-pushed the idmapped-mounts branch 2 times, most recently from e5452b4 to 1f284e6 Compare November 15, 2021 14:32
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
generalize the mechanism of opening mount targets earlier.  This will
be necessary later to allow creating idmapped mounts before entering
the user namespace.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
prevent leaks on errors since it is called when the container is
freed.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
preparation patch to enable the creation of mounts before joining or
creating a user namespace.  This is needed for creating idmapped
mounts that are usable from the container user namespace.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
@giuseppe giuseppe force-pushed the idmapped-mounts branch 2 times, most recently from 39e8611 to bb5ec37 Compare November 15, 2021 15:20
giuseppe added a commit to giuseppe/libpod that referenced this pull request Nov 15, 2021
pass down the "idmap" mount option to the OCI runtime.

Needs: containers/crun#780

Closes: containers#12154

[NO NEW TESTS NEEDED] there is no crun version yet that support the
new feature.

Test case (must run as root):

podman run --rm -v foo:/foo alpine touch /foo/bar

podman run --uidmap 0:1:1000 --rm -v foo:/foo:idmap alpine ls -l /foo
total 0
-rw-r--r--    1 root     root             0 Nov 15 14:01 bar

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
@giuseppe giuseppe force-pushed the idmapped-mounts branch 2 times, most recently from 342882e to d3803e0 Compare November 15, 2021 16:06
giuseppe added a commit to giuseppe/libpod that referenced this pull request Nov 15, 2021
pass down the "idmap" mount option to the OCI runtime.

Needs: containers/crun#780

Closes: containers#12154

[NO NEW TESTS NEEDED] there is no crun version yet that support the
new feature.

Test case (must run as root):

podman run --rm -v foo:/foo alpine touch /foo/bar

podman run --uidmap 0:1:1000 --rm -v foo:/foo:idmap alpine ls -l /foo
total 0
-rw-r--r--    1 root     root             0 Nov 15 14:01 bar

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
@rhatdan
Copy link
Member

rhatdan commented Nov 15, 2021

LGTM
@flouthoc PTAL

when the "idmap" mount option is specified, create the mount outside
of the container user namespace context and pass the mount fd to the
container init process.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
@flouthoc
Copy link
Collaborator

flouthoc commented Nov 16, 2021

Just a small question so when mount is id mapped we are mounting it before moving to user namespace and sending fd via socket while sending other mounts via socket with payload so they can be mounted inside the user namespace.

@giuseppe
Copy link
Member Author

Just a small question so when mount is id mapped we are mounting it before moving to user namespace and sending fd via socket while sending other mounts via socket with payload so they can be mounted inside the user namespace.

yes, the mounts are created outside the user namespace but after the user namespace is created. That is why we cannot do it before the fork.

@flouthoc
Copy link
Collaborator

LGTM

@flouthoc flouthoc merged commit d6d78a3 into containers:main Nov 16, 2021
giuseppe added a commit to giuseppe/libpod that referenced this pull request Nov 18, 2021
pass down the "idmap" mount option to the OCI runtime.

Needs: containers/crun#780

Closes: containers#12154

[NO NEW TESTS NEEDED] there is no crun version yet that support the
new feature.

Test case (must run as root):

podman run --rm -v foo:/foo alpine touch /foo/bar

podman run --uidmap 0:1:1000 --rm -v foo:/foo:idmap alpine ls -l /foo
total 0
-rw-r--r--    1 root     root             0 Nov 15 14:01 bar

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
giuseppe added a commit to giuseppe/libpod that referenced this pull request Nov 18, 2021
pass down the "idmap" mount option to the OCI runtime.

Needs: containers/crun#780

Closes: containers#12154

[NO NEW TESTS NEEDED] there is no crun version yet that support the
new feature.

Test case (must run as root):

podman run --rm -v foo:/foo alpine touch /foo/bar

podman run --uidmap 0:1:1000 --rm -v foo:/foo:idmap alpine ls -l /foo
total 0
-rw-r--r--    1 root     root             0 Nov 15 14:01 bar

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
giuseppe added a commit to giuseppe/libpod that referenced this pull request Nov 18, 2021
pass down the "idmap" mount option to the OCI runtime.

Needs: containers/crun#780

Closes: containers#12154

[NO NEW TESTS NEEDED] there is no crun version yet that support the
new feature.

Test case (must run as root):

podman run --rm -v foo:/foo alpine touch /foo/bar

podman run --uidmap 0:1:1000 --rm -v foo:/foo:idmap alpine ls -l /foo
total 0
-rw-r--r--    1 root     root             0 Nov 15 14:01 bar

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
@AkihiroSuda
Copy link

Do you have plan to extend the OCI spec to support idmapped rootfs too?

@giuseppe
Copy link
Member Author

giuseppe commented Dec 9, 2021

I've not thought about that yet. Is the use case to apply the recursive read-only flag to the entire container? How would it work with mounts that need to be writeable?

@AkihiroSuda
Copy link

I've not thought about that yet. Is the use case to apply the recursive read-only flag to the entire container? How would it work with mounts that need to be writeable?

The main usecase of idmapped rootfs would be to save the time overhead of chowning.

Haven't thought much about recursive read-only mounts with idmapped rootfs.

@giuseppe
Copy link
Member Author

giuseppe commented Dec 9, 2021

sorry, somehow my brain got confused with "recursive mounts".

I am not sure. Is it something the OCI runtime should deal with?

So far setting the rootfs is a responsibility of the container engine, but I see the value of reusing the existing code and hiding the mount from the host.

One thing to think about, do we want to set the idmapped for the whole overlay mount or apply it only to the lower layers? In the latter case, then OCI runtime can't help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants