[lxc-devel] [PATCH 1/1] cgmanager: chown cgroups to the container root

[Serge Hallyn]

Quoting Stéphane Graber (stgraber at ubuntu.com):
> On Thu, Jan 23, 2014 at 11:56:15PM -0600, Serge Hallyn wrote:
> > After this patch, starting an unprivileged container using
> > cgmanager gets the cgroup chown to the container root, so
> > that it can install the cgmanager (proxy) and make cgroup
> > requests.
> > 
> > (Still desirable and not in this patch is the automatic setup of
> > /sys/fs/cgroup/manager/sock, which you can currently do with
> > two lxc.mount.entries)
> > 
> > Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
> > ---
> >  src/lxc/cgmanager.c | 167 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  src/lxc/cgroup.c    |   8 +++
> >  src/lxc/cgroup.h    |   2 +
> >  src/lxc/start.c     |   3 +
> >  4 files changed, 178 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/lxc/cgmanager.c b/src/lxc/cgmanager.c
> > index e43e1f7..4f83272 100644
> > --- a/src/lxc/cgmanager.c
> > +++ b/src/lxc/cgmanager.c
> > @@ -75,6 +75,44 @@ static void cgmanager_disconnected(DBusConnection *connection)
> >  	}
> >  }
> >  
> > +static int send_creds(int sock, int rpid, int ruid, int rgid)
> > +{
> > +	struct msghdr msg = { 0 };
> > +	struct iovec iov;
> > +	struct cmsghdr *cmsg;
> > +	struct ucred cred = {
> > +		.pid = rpid,
> > +		.uid = ruid,
> > +		.gid = rgid,
> > +	};
> > +	char cmsgbuf[CMSG_SPACE(sizeof(cred))];
> > +	char buf[1];
> > +	buf[0] = 'p';
> > +
> > +	msg.msg_control = cmsgbuf;
> > +	msg.msg_controllen = sizeof(cmsgbuf);
> > +
> > +	cmsg = CMSG_FIRSTHDR(&msg);
> > +	cmsg->cmsg_len = CMSG_LEN(sizeof(struct ucred));
> > +	cmsg->cmsg_level = SOL_SOCKET;
> > +	cmsg->cmsg_type = SCM_CREDENTIALS;
> > +	memcpy(CMSG_DATA(cmsg), &cred, sizeof(cred));
> > +
> > +	msg.msg_name = NULL;
> > +	msg.msg_namelen = 0;
> > +
> > +	iov.iov_base = buf;
> > +	iov.iov_len = sizeof(buf);
> > +	msg.msg_iov = &iov;
> > +	msg.msg_iovlen = 1;
> > +
> > +	if (sendmsg(sock, &msg, 0) < 0) {
> > +		perror("sendmsg");
> > +		return -1;
> > +	}
> > +	return 0;
> > +}
> > +
> >  #define CGMANAGER_DBUS_SOCK "unix:path=/sys/fs/cgroup/cgmanager/sock"
> >  bool lxc_init_cgmanager(void)
> >  {
> > @@ -120,10 +158,121 @@ static bool lxc_cgmanager_create(const char *controller, const char *cgroup_path
> >  		return false;
> >  	}
> >  
> > -	// TODO - try to chown the cgroup to the container root
> >  	return true;
> >  }
> >  
> > +struct chown_data {
> > +	const char *controller;
> > +	const char *cgroup_path;
> > +};
> > +
> > +static int do_chown_cgroup(const char *controller, const char *cgroup_path)
> > +{
> > +	int sv[2] = {-1, -1}, optval = 1;
> > +	char buf[1];
> > +
> > +	if (setgid(0) < 0)
> > +		WARN("Failed to setgid to 0");
> > +	if (setuid(0) < 0)
> > +		WARN("Failed to setuid to 0");
> > +
> > +	if (socketpair(AF_UNIX, SOCK_DGRAM, 0, sv) < 0) {
> > +		SYSERROR("Error creating socketpair");
> > +		exit(1);
> > +	}
> > +	if (setsockopt(sv[1], SOL_SOCKET, SO_PASSCRED, &optval, sizeof(optval)) == -1) {
> > +		SYSERROR("setsockopt failed");
> > +		exit(1);
> > +	}
> > +	if (setsockopt(sv[0], SOL_SOCKET, SO_PASSCRED, &optval, sizeof(optval)) == -1) {
> > +		SYSERROR("setsockopt failed");
> > +		exit(1);
> > +	}
> 
> Shouldn't those "exit(1)" be "return -1" instead?

Hm, yeah.  They're highly unlikely to be hit which is why I didn't
notice it in testing :)  Thanks for catching that.

Please feel free to do so in-line, or else I'll send an updated
patch, uh, maybe tomorrow.

-serge