[lxc-devel] [PATCH 1/1] lxc-cgm: fix issue with nested chowning
Stéphane Graber
stgraber at ubuntu.com
Fri Aug 29 14:27:31 UTC 2014
On Fri, Aug 29, 2014 at 02:20:44PM +0000, Serge Hallyn wrote:
> To ask cgmanager to chown files as an unpriv user, we must send the
> request from the container's namespace (with our own userid also
> mapped in). However when we create a new namespace then we must
> open a new dbus connection, so that our credential and the credential
> on the dbus socket match. Otherwise the proxy will refuse the request.
>
> Because we were warning about this failure but not exiting, the failure
> was not noticed until the unprivileged container went on to try to
> administer its cgroups, i.e. creating a container inside itself.
>
> Fix this by having the do_chown_cgroup create a new cgmanager connection.
> In order to reduce the number of connections, since the list of subsystems
> is global anyway, don't call do_chown_cgroup once for each controller,
> just call it once and have it run over all controllers.
>
> (This patch does not change the fact that we don't fail if the
> chown failed. I think we should change that, but let's do it in a
> later patch)
>
> Reported-by: Stéphane Graber <stgraber at ubuntu.com>
> Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>
Acked-by: Stéphane Graber <stgraber at ubuntu.com>
> ---
> src/lxc/cgmanager.c | 61 ++++++++++++++++++++++++++++++++++-------------------
> 1 file changed, 39 insertions(+), 22 deletions(-)
>
> diff --git a/src/lxc/cgmanager.c b/src/lxc/cgmanager.c
> index 4db61c1..97d19ca 100644
> --- a/src/lxc/cgmanager.c
> +++ b/src/lxc/cgmanager.c
> @@ -261,19 +261,16 @@ static bool lxc_cgmanager_escape(void)
> }
>
> struct chown_data {
> - const char *controller;
> const char *cgroup_path;
> uid_t origuid;
> };
>
> static int do_chown_cgroup(const char *controller, const char *cgroup_path,
> - uid_t origuid)
> + uid_t newuid)
> {
> int sv[2] = {-1, -1}, optval = 1, ret = -1;
> char buf[1];
>
> - uid_t caller_nsuid = get_ns_uid(origuid);
> -
> if (socketpair(AF_UNIX, SOCK_DGRAM, 0, sv) < 0) {
> SYSERROR("Error creating socketpair");
> goto out;
> @@ -321,7 +318,7 @@ static int do_chown_cgroup(const char *controller, const char *cgroup_path,
> ERROR("Error getting reply from server over socketpair");
> goto out;
> }
> - if (send_creds(sv[0], getpid(), caller_nsuid, 0)) {
> + if (send_creds(sv[0], getpid(), newuid, 0)) {
> SYSERROR("%s: Error sending pid over SCM_CREDENTIAL", __func__);
> goto out;
> }
> @@ -343,6 +340,8 @@ out:
> static int chown_cgroup_wrapper(void *data)
> {
> struct chown_data *arg = data;
> + int i;
> + uid_t destuid;
>
> if (setresgid(0,0,0) < 0)
> SYSERROR("Failed to setgid to 0");
> @@ -350,7 +349,21 @@ static int chown_cgroup_wrapper(void *data)
> SYSERROR("Failed to setuid to 0");
> if (setgroups(0, NULL) < 0)
> SYSERROR("Failed to clear groups");
> - return do_chown_cgroup(arg->controller, arg->cgroup_path, arg->origuid);
> + cgm_dbus_disconnect();
> + if (!cgm_dbus_connect()) {
> + ERROR("Error connecting to cgroup manager");
> + return -1;
> + }
> + destuid = get_ns_uid(arg->origuid);
> + for (i = 0; i < nr_subsystems; i++) {
> + if (do_chown_cgroup(subsystems[i], arg->cgroup_path, destuid) < 0) {
> + ERROR("Failed to chown %s:%s to container root",
> + subsystems[i], arg->cgroup_path);
> + return -1;
> + }
> + }
> + cgm_dbus_disconnect();
> + return 0;
> }
>
> /* Internal helper. Must be called with the cgmanager dbus socket open */
> @@ -369,31 +382,39 @@ static bool lxc_cgmanager_chmod(const char *controller,
> }
>
> /* Internal helper. Must be called with the cgmanager dbus socket open */
> -static bool chown_cgroup(const char *controller, const char *cgroup_path,
> - struct lxc_conf *conf)
> +static bool chown_cgroup(const char *cgroup_path, struct lxc_conf *conf)
> {
> struct chown_data data;
> + int i;
>
> if (lxc_list_empty(&conf->id_map))
> /* If there's no mapping then we don't need to chown */
> return true;
>
> - data.controller = controller;
> data.cgroup_path = cgroup_path;
> data.origuid = geteuid();
>
> + /* Unpriv users can't chown it themselves, so chown from
> + * a child namespace mapping both our own and the target uid
> + */
> if (userns_exec_1(conf, chown_cgroup_wrapper, &data) < 0) {
> ERROR("Error requesting cgroup chown in new namespace");
> return false;
> }
>
> - /* now chmod 775 the directory else the container cannot create cgroups */
> - if (!lxc_cgmanager_chmod(controller, cgroup_path, "", 0775))
> - return false;
> - if (!lxc_cgmanager_chmod(controller, cgroup_path, "tasks", 0775))
> - return false;
> - if (!lxc_cgmanager_chmod(controller, cgroup_path, "cgroup.procs", 0775))
> - return false;
> + /*
> + * Now chmod 775 the directory else the container cannot create cgroups.
> + * This can't be done in the child namespace because it only group-owns
> + * the cgroup
> + */
> + for (i = 0; i < nr_subsystems; i++) {
> + if (!lxc_cgmanager_chmod(subsystems[i], cgroup_path, "", 0775))
> + return false;
> + if (!lxc_cgmanager_chmod(subsystems[i], cgroup_path, "tasks", 0775))
> + return false;
> + if (!lxc_cgmanager_chmod(subsystems[i], cgroup_path, "cgroup.procs", 0775))
> + return false;
> + }
>
> return true;
> }
> @@ -1134,7 +1155,6 @@ out:
> static bool cgm_chown(void *hdata, struct lxc_conf *conf)
> {
> struct cgm_data *d = hdata;
> - int i;
>
> if (!d || !d->cgroup_path)
> return false;
> @@ -1142,11 +1162,8 @@ static bool cgm_chown(void *hdata, struct lxc_conf *conf)
> ERROR("Error connecting to cgroup manager");
> return false;
> }
> - for (i = 0; i < nr_subsystems; i++) {
> - if (!chown_cgroup(subsystems[i], d->cgroup_path, conf))
> - WARN("Failed to chown %s:%s to container root",
> - subsystems[i], d->cgroup_path);
> - }
> + if (!chown_cgroup(d->cgroup_path, conf))
> + WARN("Failed to chown %s to container root", d->cgroup_path);
> cgm_dbus_disconnect();
> return true;
> }
> --
> 2.1.0
>
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel
--
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20140829/4f91a405/attachment.sig>
More information about the lxc-devel
mailing list