/cc @AkihiroSuda @justincormack

@AkihiroSuda You're right, a flag may be a better option, but I don't know the moby code base well enough to implement this. Therefore I changed the default and marked this PR as WIP so that it is clear what configuration I intend to have changed.

Also as to my knowledge having the v2 cgroups writeable should be fine, but a security assessment may prove useful here.

cc @giuseppe @kolyshkin @cyphar FYI

Since the cgroup the process is in is the same cgroup where the restrictions are set, I don't think it's at all safe to allow containers to write to cgroup files -- that would allow a container to modify its own restrictions. If we created a sub-cgroup that the container was placed inside, maybe that would be safe but I'm not convinced to be honest.

As an aside, I wonder if you can change the devices eBPF program today with a read-only mount -- IOW does bpf(2) actually stop you from operating on a read-only mounted cgroup? (And the devices cgroup is the strongest argument for not doing this -- a container being able to modify its own devices restrictions would be very bad.)

Today you cannot set eBPF (and afaik not even with this patch) as that is filtered (at least that's the error you get when you apply this patch and try to start systemd.

Ok, I thought docker already creates a sub-cgroup for the containers. That was my fault than. But having cgroup support within containers would still be very valuable.
If this doesn't even work with sub-cgroups, is there a change to get this possibility upstream into the linux kernel so that docker can use it?

Yes, bpf(2) is blocked by the default seccomp profile (unless your container has CAP_SYS_ADMIN) -- the reason I mentioned the devices eBPF is because I hadn't considered whether BPF_PROG_ATTACH actually obeys read-only mounts for attaching (and a quick look at the code tells me it doesn't, but I haven't tested it yet).

Ok, I thought docker already creates a sub-cgroup for the containers. That was my fault than. But having cgroup support within containers would still be very valuable.

I would have to think about how we would implement this, since runc is doing cgroup setup and there isn't a nice way to tell runc to configure a cgroup and then move the program into a sub-cgroup...

If this doesn't even work with sub-cgroups, is there a change to get this possibility upstream into the linux kernel so that docker can use it?

If containers were placed in a sub-cgroup such that the restricted cgroup is a parent, then this should be safe -- Tejun has said that cgroupv2 is safe for delegation, which tells me that the kernel is giving us a guarantee that this is safe.

So we depend upon adding a new feature to runc to allow to create sub-cgroup than?

Any updates? I see podman "solves" this by introducing a new option: --security-opt unmask=/sys/fs/cgroup

containers/podman#9536

In re-reading this thread with fresh eyes, it occurs to me that we're discussing two separate things that I think could be disconnected in order to make some progress?

1. in order to enable this by default, we need more functionality/UX design in runc (harder)

2. in order to make it possible for users to opt into this (without the full "hammer" that is --privileged), I think we just need to add a new security opt and check for it in the following block of code?
   

   moby/daemon/oci_linux.go

   Lines 667 to 675 in f0cec02

   	// TODO: until a kernel/mount solution exists for handling remount in a user namespace,
   	// we must clear the readonly flag for the cgroups mount (@mrunalp concurs)
   	if uidMap := daemon.idMapping.UIDMaps; uidMap != nil || c.HostConfig.Privileged {
   	for i, m := range s.Mounts {
   	if m.Type == "cgroup" {
   	clearReadOnly(&s.Mounts[i])
   	}
   	}
   	}

(the argument for the latter without necessarily having the former is that there are use cases where the downsides listed above are not as bad, such as containers that are already mostly unconfined from a cgroups perspective -- certainly better than going all the way to privileged!)

In podman, the relevant CLI option is --security-opt=unmask=/sys/fs/cgroup, described here (look for `unmask).

If at all possible, can the option being added be made compatible with podman?

(I've replied to that same question over in #48828 (comment) instead where we're reviewing a explicit proposal adding an option 👀)