Snapcraft adt failures with the new core release

A minimal sample, extracted from cgroup-freezer-support.c, that fails for me: https://paste.ubuntu.com/25975101/

Are you running it as root or setuid?

I actually tried this:

  1. build with gcc -DDO_WAIT..
  2. move the binary to the container’s /, chown root:root, chmod u+s
  3. run it, it will stop waiting for SIGCONT
  4. go back to host, find the test binary’s pid in global namespace
  5. while on the host trace-cmd -e syscalls -e cgroup -e fs -p function -P <global-pid>
  6. back in the container kill -CONT <pid>, see cannot create freezer cgroup hierarchy: Permission denied
  7. back to the host C-c, trace-cmd report … and get a segfault in trace-cmd :slight_smile:

I’m running with setuid

Ah ffs, I know what’s going on. So obvious that I didn’t even consider it. You’re running setuid but not setgid, right? By that I mean snap-confine.

Yes, we are not setgid

Yes, adding g+s makes the minimal sample succeed, and will probably fix snap-confine too (although I have not tried it with snap-confine, @zyga-snapd did you?)

Yes I did try that and it does fix the issue.

The question remains: should LXD set up cgroups to be root owned and root writable or should snap-confine have to be setgid.

The reason why we keep the owner as the container’s owning uid rather than uid 0 in the container is that without this, the owner of the container (especially if it’s an unprivileged user) would loose the ability to control the container’s cgroups.

So should we just make snap-confine g+s CC: @jdstrand

If that’s a possibility for you, yes. I just tested this on our side again for unprivileged container’s you’d effectively loose the ability to attach to the container as an unprivileged user because you can’t move yourself into the cgroup.

Hmm, can you expand on that please. Does that mean regular use of LXD will be broken?

Would it be any better if we didn’t chown? (Note that when we are g+s then the file will be root.root anyway, right?)

Having snap-confine be setgid is possible, though we’d need to verify we are dropping correctly. However, if snap-confine does the chown as setgid, won’t that break container’s ability to control those cgroups? (referring to @stgraber’s point)

I think at at time we can perhaps drop the chown (as all the new files will be root.root anyway and the point of the chown was to make that so (it will now effectively become a no-op).

For LXD it’s not as big a deal since LXD itself runs as root so will always be able to attach regardless of what the container does. This permission trick comes from liblxc which can also be used by entirely unprivileged users in which case, changing /sys/fs/cgroup/freezer or any of the cgroup owner to root:root in the container would indeed prevent attaching or reconfiguring the container.

Anyway, if you run with g+s for snap-confine, that should fix this problem and I don’t expect any side effects.

If we drop the chown, won’t the files be root.user in the non-sudo case?

With g+s they should be root.root, right? We won’t run as user gid;

@stgraber we are not changing /sys/fs/cgroup/freezer, just the particular cgroups inside, is that okay?

Yes, that’s perfectly fine, liblxc only ever attaches or reconfigures the root of the cgroup.

1 Like

Yes, of course. I thought you were saying don’t setgid and don’t chown.

I’m running a spread run with the setgid changes in place. I’ll polish the patch to ensure we don’t do unneeded chowns and propose it for review.

Thank you both @brauner and @stgraber for your assistance!