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

Support recursively-readonly mounts with mount_setattr (kernel 5.12) #2823

Closed
AkihiroSuda opened this issue Feb 25, 2021 · 5 comments · Fixed by #3272
Closed

Support recursively-readonly mounts with mount_setattr (kernel 5.12) #2823

AkihiroSuda opened this issue Feb 25, 2021 · 5 comments · Fixed by #3272
Milestone

Comments

@AkihiroSuda
Copy link
Member

The current OCI mount with options: ["rbind", "ro"] is (surprisingly) not recursively read-only.

mount_setattr(2) introduced in kernel 5.12 can be used for creating recursively-readonly bind mounts:

struct mount_attr attr = {
  .attr_set   = MOUNT_ATTR_RDONLY,
};
rc = mount_setattr(-1, "/mnt/ro", AT_RECURSIVE, &attr, sizeof(attr));

runc implementation will need runtime spec PR to be approved: opencontainers/runtime-spec#1090

@AkihiroSuda AkihiroSuda added this to the post-1.0 milestone Feb 25, 2021
@cyphar
Copy link
Member

cyphar commented Mar 18, 2021

Yes please! Though it should be noted this is technically going to be a behavioural change in runc, because our current implementation (unless I'm mistaken) doesn't currently make mounts read-only recursively. (Funnily enough, mount_settattr(2) partially came out of discussions where I mentioned that MS_REC|MS_RDONLY|MS_BIND|MS_REMOUNT doesn't do what you'd expect -- because we were doing that in runc.)

@AkihiroSuda
Copy link
Member Author

Though it should be noted this is technically going to be a behavioural change in runc

I was thinking of keeping the current ["rbind","ro"] behavior as-is (i.e., recursively writable), and adding new JSON fields for MOUNT_ATTR_RDONLY: opencontainers/runtime-spec#1090

@cyphar
Copy link
Member

cyphar commented Mar 18, 2021

Right, but I was thinking of readonly: true in particular -- currently that doesn't recursively mount the root as read-only, and it probably should (with the exception of /proc).

@cyphar cyphar modified the milestones: post-1.0, 1.1.0 Jun 11, 2021
@AkihiroSuda
Copy link
Member Author

Moving to v1.2 milestone, waiting for opencontainers/runtime-spec#1090 (comment)

@AkihiroSuda
Copy link
Member Author

Opened PR #3272

@AkihiroSuda AkihiroSuda modified the milestones: 1.2.0, post-1.0 Nov 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants