Merge pull request #1612 from brauner/2017-06-03/bugfixes

src/lxc/bdev/bdev.c

@@ -437,7 +437,7 @@ struct bdev *bdev_copy(struct lxc_container *c0, const char *cname,
 	data.orig = orig;
 	data.new = new;
 	if (am_unpriv())
-		ret = userns_exec_1(c0->lxc_conf, rsync_rootfs_wrapper, &data);
+		ret = userns_exec_1(c0->lxc_conf, rsync_rootfs_wrapper, &data, "rsync_rootfs_wrapper");
 	else
 		ret = rsync_rootfs(&data);
 

src/lxc/bdev/lxcaufs.c

@@ -133,7 +133,8 @@ int aufs_clonepaths(struct bdev *orig, struct bdev *new, const char *oldname,
 		rdata.src = odelta;
 		rdata.dest = ndelta;
 		if (am_unpriv())
-			ret = userns_exec_1(conf, rsync_delta_wrapper, &rdata);
+			ret = userns_exec_1(conf, rsync_delta_wrapper, &rdata,
+					    "rsync_delta_wrapper");
 		else
 			ret = rsync_delta(&rdata);
 		if (ret) {

src/lxc/bdev/lxcbtrfs.c

@@ -398,7 +398,8 @@ int btrfs_clonepaths(struct bdev *orig, struct bdev *new, const char *oldname,
 			return btrfs_snapshot(orig->dest, new->dest);
 		sdata.dest = new->dest;
 		sdata.src = orig->dest;
-		return userns_exec_1(conf, btrfs_snapshot_wrapper, &sdata);
+		return userns_exec_1(conf, btrfs_snapshot_wrapper, &sdata,
+				     "btrfs_snapshot_wrapper");
 	}
 
 	if (rmdir(new->dest) < 0 && errno != ENOENT) {

src/lxc/bdev/lxcoverlay.c

@@ -750,7 +750,8 @@ static int ovl_do_rsync(struct bdev *orig, struct bdev *new, struct lxc_conf *co
 	rdata.orig = orig;
 	rdata.new = new;
 	if (am_unpriv())
-		ret = userns_exec_1(conf, ovl_rsync_wrapper, &rdata);
+		ret = userns_exec_1(conf, ovl_rsync_wrapper, &rdata,
+				    "ovl_rsync_wrapper");
 	else
 		ret = ovl_rsync(&rdata);
 	if (ret)

src/lxc/cgroups/cgfs.c

@@ -1874,7 +1874,8 @@ static int create_or_remove_cgroup(bool do_remove,
 			return 0;
 		if (recurse) {
 			if (conf && !lxc_list_empty(&conf->id_map))
-				r = userns_exec_1(conf, rmdir_wrapper, buf);
+				r = userns_exec_1(conf, rmdir_wrapper, buf,
+						  "rmdir_wrapper");
 			else
 				r = cgroup_rmdir(buf);
 		} else
@@ -2616,7 +2617,8 @@ static bool do_cgfs_chown(char *cgroup_path, struct lxc_conf *conf)
 	/* 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) {
+	if (userns_exec_1(conf, chown_cgroup_wrapper, &data,
+			  "chown_cgroup_wrapper") < 0) {
 		ERROR("Error requesting cgroup chown in new namespace");
 		return false;
 	}

src/lxc/cgroups/cgfsng.c

@@ -1284,7 +1284,7 @@ void recursive_destroy(char *path, struct lxc_conf *conf)
 {
 	int r;
 	if (conf && !lxc_list_empty(&conf->id_map))
-		r = userns_exec_1(conf, rmdir_wrapper, path);
+		r = userns_exec_1(conf, rmdir_wrapper, path, "rmdir_wrapper");
 	else
 		r = cgroup_rmdir(path);
 
@@ -1507,7 +1507,8 @@ static bool cgfsns_chown(void *hdata, struct lxc_conf *conf)
 	wrap.d = d;
 	wrap.origuid = geteuid();
 
-	if (userns_exec_1(conf, chown_cgroup_wrapper, &wrap) < 0) {
+	if (userns_exec_1(conf, chown_cgroup_wrapper, &wrap,
+			  "chown_cgroup_wrapper") < 0) {
 		ERROR("Error requesting cgroup chown in new namespace");
 		return false;
 	}

src/lxc/cgroups/cgmanager.c

@@ -497,7 +497,8 @@ static bool chown_cgroup(const char *cgroup_path, struct lxc_conf *conf)
 	/* 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) {
+	if (userns_exec_1(conf, chown_cgroup_wrapper, &data,
+			  "chown_cgroup_wrapper") < 0) {
 		ERROR("Error requesting cgroup chown in new namespace");
 		return false;
 	}

src/lxc/conf.c

@@ -815,17 +815,17 @@ static int lxc_mount_auto_mounts(struct lxc_conf *conf, int flags, struct lxc_ha
 		 * :mixed, because then the container can't remount it read-write. */
 		if (cg_flags == LXC_AUTO_CGROUP_NOSPEC || cg_flags == LXC_AUTO_CGROUP_FULL_NOSPEC) {
 			int has_sys_admin = 0;
-			if (!lxc_list_empty(&conf->keepcaps)) {
+
+			if (!lxc_list_empty(&conf->keepcaps))
 				has_sys_admin = in_caplist(CAP_SYS_ADMIN, &conf->keepcaps);
-			} else {
+			else
 				has_sys_admin = !in_caplist(CAP_SYS_ADMIN, &conf->caps);
-			}
-			if (cg_flags == LXC_AUTO_CGROUP_NOSPEC) {
+
+			if (cg_flags == LXC_AUTO_CGROUP_NOSPEC)
 				cg_flags = has_sys_admin ? LXC_AUTO_CGROUP_RW : LXC_AUTO_CGROUP_MIXED;
-			} else {
+			else
 				cg_flags = has_sys_admin ? LXC_AUTO_CGROUP_FULL_RW : LXC_AUTO_CGROUP_FULL_MIXED;
 		}
-		}
 
 		if (!cgroup_mount(conf->rootfs.path ? conf->rootfs.mount : "", handler, cg_flags)) {
 			SYSERROR("error mounting /sys/fs/cgroup");
@@ -2764,8 +2764,8 @@ struct lxc_conf *lxc_conf_init(void)
 
 static int instantiate_veth(struct lxc_handler *handler, struct lxc_netdev *netdev)
 {
-	char veth1buf[IFNAMSIZ], *veth1;
-	char veth2buf[IFNAMSIZ], *veth2;
+	char *veth1, *veth2;
+	char veth1buf[IFNAMSIZ], veth2buf[IFNAMSIZ];
 	int bridge_index, err;
 	unsigned int mtu = 0;
 
@@ -2797,8 +2797,8 @@ static int instantiate_veth(struct lxc_handler *handler, struct lxc_netdev *netd
 
 	err = lxc_veth_create(veth1, veth2);
 	if (err) {
-		ERROR("failed to create veth pair (%s and %s): %s", veth1, veth2,
-		      strerror(-err));
+		ERROR("failed to create veth pair \"%s\" and \"%s\": %s", veth1,
+		      veth2, strerror(-err));
 		goto out_delete;
 	}
 
@@ -2807,30 +2807,30 @@ static int instantiate_veth(struct lxc_handler *handler, struct lxc_netdev *netd
 	 * of a container */
 	err = setup_private_host_hw_addr(veth1);
 	if (err) {
-		ERROR("failed to change mac address of host interface '%s': %s",
+		ERROR("failed to change mac address of host interface \"%s\": %s",
 		      veth1, strerror(-err));
 		goto out_delete;
 	}
 
 	netdev->ifindex = if_nametoindex(veth2);
 	if (!netdev->ifindex) {
-		ERROR("failed to retrieve the index for %s", veth2);
+		ERROR("failed to retrieve the index for \"%s\"", veth2);
 		goto out_delete;
 	}
 
 	if (netdev->mtu) {
 		if (lxc_safe_uint(netdev->mtu, &mtu) < 0)
-			WARN("Failed to parse mtu from.");
+			WARN("failed to parse mtu from");
 		else
-			INFO("Retrieved mtu %d", mtu);
+			INFO("retrieved mtu %d", mtu);
 	} else if (netdev->link) {
 		bridge_index = if_nametoindex(netdev->link);
 		if (bridge_index) {
 			mtu = netdev_get_mtu(bridge_index);
-			INFO("Retrieved mtu %d from %s", mtu, netdev->link);
+			INFO("retrieved mtu %d from %s", mtu, netdev->link);
 		} else {
 			mtu = netdev_get_mtu(netdev->ifindex);
-			INFO("Retrieved mtu %d from %s", mtu, veth2);
+			INFO("retrieved mtu %d from %s", mtu, veth2);
 		}
 	}
 
@@ -2839,7 +2839,8 @@ static int instantiate_veth(struct lxc_handler *handler, struct lxc_netdev *netd
 		if (!err)
 			err = lxc_netdev_set_mtu(veth2, mtu);
 		if (err) {
-			ERROR("failed to set mtu '%i' for veth pair (%s and %s): %s",
+			ERROR("failed to set mtu \"%d\" for veth pair \"%s\" "
+			      "and \"%s\": %s",
 			      mtu, veth1, veth2, strerror(-err));
 			goto out_delete;
 		}
@@ -2848,16 +2849,16 @@ static int instantiate_veth(struct lxc_handler *handler, struct lxc_netdev *netd
 	if (netdev->link) {
 		err = lxc_bridge_attach(handler->lxcpath, handler->name, netdev->link, veth1);
 		if (err) {
-			ERROR("failed to attach '%s' to the bridge '%s': %s",
+			ERROR("failed to attach \"%s\" to bridge \"%s\": %s",
 			      veth1, netdev->link, strerror(-err));
 			goto out_delete;
 		}
-		INFO("Attached '%s': to the bridge '%s': ", veth1, netdev->link);
+		INFO("attached \"%s\" to bridge \"%s\"", veth1, netdev->link);
 	}
 
 	err = lxc_netdev_up(veth1);
 	if (err) {
-		ERROR("failed to set %s up : %s", veth1, strerror(-err));
+		ERROR("failed to set \"%s\" up: %s", veth1, strerror(-err));
 		goto out_delete;
 	}
 
@@ -2868,8 +2869,8 @@ static int instantiate_veth(struct lxc_handler *handler, struct lxc_netdev *netd
 			goto out_delete;
 	}
 
-	DEBUG("instantiated veth '%s/%s', index is '%d'",
-	      veth1, veth2, netdev->ifindex);
+	DEBUG("instantiated veth \"%s/%s\", index is \"%d\"", veth1, veth2,
+	      netdev->ifindex);
 
 	return 0;
 
@@ -4107,21 +4108,30 @@ static int send_fd(int sock, int fd)
 
 static int send_ttys_to_parent(struct lxc_handler *handler)
 {
+	int i, ret;
 	struct lxc_conf *conf = handler->conf;
 	const struct lxc_tty_info *tty_info = &conf->tty_info;
-	int i;
 	int sock = handler->ttysock[0];
 
 	for (i = 0; i < tty_info->nbtty; i++) {
 		struct lxc_pty_info *pty_info = &tty_info->pty_info[i];
-		if (send_fd(sock, pty_info->slave) < 0)
-			goto bad;
+		ret = send_fd(sock, pty_info->slave);
+		if (ret >= 0)
+			send_fd(sock, pty_info->master);
+		TRACE("sending pty \"%s\" with master fd %d and slave fd %d to "
+		      "parent",
+		      pty_info->name, pty_info->master, pty_info->slave);
 		close(pty_info->slave);
 		pty_info->slave = -1;
-		if (send_fd(sock, pty_info->master) < 0)
-			goto bad;
 		close(pty_info->master);
 		pty_info->master = -1;
+		if (ret < 0) {
+			ERROR("failed to send pty \"%s\" with master fd %d and "
+			      "slave fd %d to parent : %s",
+			      pty_info->name, pty_info->master, pty_info->slave,
+			      strerror(errno));
+			goto bad;
+		}
 	}
 
 	close(handler->ttysock[0]);
@@ -4659,6 +4669,7 @@ void lxc_conf_free(struct lxc_conf *conf)
 
 struct userns_fn_data {
 	int (*fn)(void *);
+	const char *fn_name;
 	void *arg;
 	int p[2];
 };
@@ -4680,6 +4691,8 @@ static int run_userns_fn(void *data)
 	/* Close read end of the pipe. */
 	close(d->p[0]);
 
+	if (d->fn_name)
+		TRACE("calling function \"%s\"", d->fn_name);
 	/* Call function to run. */
 	return d->fn(d->arg);
 }
@@ -4757,7 +4770,8 @@ static struct id_map *idmap_add(struct lxc_conf *conf, uid_t id, enum idtype typ
  * retrieve from the ontainer's configured {g,u}id mappings as it must have been
  * there to start the container in the first place.
  */
-int userns_exec_1(struct lxc_conf *conf, int (*fn)(void *), void *data)
+int userns_exec_1(struct lxc_conf *conf, int (*fn)(void *), void *data,
+		  const char *fn_name)
 {
 	pid_t pid;
 	uid_t euid, egid;
@@ -4777,6 +4791,7 @@ int userns_exec_1(struct lxc_conf *conf, int (*fn)(void *), void *data)
 		return -1;
 	}
 	d.fn = fn;
+	d.fn_name = fn_name;
 	d.arg = data;
 	d.p[0] = p[0];
 	d.p[1] = p[1];
@@ -4827,17 +4842,16 @@ int userns_exec_1(struct lxc_conf *conf, int (*fn)(void *), void *data)
 		goto on_error;
 	}
 
+	host_uid_map = container_root_uid;
+	host_gid_map = container_root_gid;
+
 	/* Check whether the {g,u}id of the user has a mapping. */
 	euid = geteuid();
 	egid = getegid();
-	if (euid == container_root_uid->hostid)
-		host_uid_map = container_root_uid;
-	else
+	if (euid != container_root_uid->hostid)
 		host_uid_map = idmap_add(conf, euid, ID_TYPE_UID);
 
-	if (egid == container_root_gid->hostid)
-		host_gid_map = container_root_gid;
-	else
+	if (egid != container_root_gid->hostid)
 		host_gid_map = idmap_add(conf, egid, ID_TYPE_GID);
 
 	if (!host_uid_map) {
@@ -4863,7 +4877,7 @@ int userns_exec_1(struct lxc_conf *conf, int (*fn)(void *), void *data)
 	lxc_list_add_elem(tmplist, container_root_uid);
 	lxc_list_add_tail(idmap, tmplist);
 
-	if (host_uid_map != container_root_uid) {
+	if (host_uid_map && (host_uid_map != container_root_uid)) {
 		/* idmap will now keep track of that memory. */
 		container_root_uid = NULL;
 
@@ -4873,9 +4887,11 @@ int userns_exec_1(struct lxc_conf *conf, int (*fn)(void *), void *data)
 			goto on_error;
 		lxc_list_add_elem(tmplist, host_uid_map);
 		lxc_list_add_tail(idmap, tmplist);
+	}
+	/* idmap will now keep track of that memory. */
+	container_root_uid = NULL;
 	/* idmap will now keep track of that memory. */
 	host_uid_map = NULL;
-	}
 
 	tmplist = malloc(sizeof(*tmplist));
 	if (!tmplist)
@@ -4883,7 +4899,7 @@ int userns_exec_1(struct lxc_conf *conf, int (*fn)(void *), void *data)
 	lxc_list_add_elem(tmplist, container_root_gid);
 	lxc_list_add_tail(idmap, tmplist);
 
-	if (host_gid_map != container_root_gid) {
+	if (host_gid_map && (host_gid_map != container_root_gid)) {
 		/* idmap will now keep track of that memory. */
 		container_root_gid = NULL;
 
@@ -4892,9 +4908,11 @@ int userns_exec_1(struct lxc_conf *conf, int (*fn)(void *), void *data)
 			goto on_error;
 		lxc_list_add_elem(tmplist, host_gid_map);
 		lxc_list_add_tail(idmap, tmplist);
+	}
+	/* idmap will now keep track of that memory. */
+	container_root_gid = NULL;
 	/* idmap will now keep track of that memory. */
 	host_gid_map = NULL;
-	}
 
 	if (lxc_log_get_level() == LXC_LOG_PRIORITY_TRACE ||
 	    conf->loglevel == LXC_LOG_PRIORITY_TRACE) {
@@ -4927,10 +4945,15 @@ int userns_exec_1(struct lxc_conf *conf, int (*fn)(void *), void *data)
 	ret = wait_for_pid(pid);
 
 on_error:
+	if (idmap)
 		lxc_free_idmap(idmap);
+	if (container_root_uid)
 		free(container_root_uid);
+	if (container_root_gid)
 		free(container_root_gid);
+	if (host_uid_map && (host_uid_map != container_root_uid))
 		free(host_uid_map);
+	if (host_gid_map && (host_gid_map != container_root_gid))
 		free(host_gid_map);
 
 	if (p[0] != -1)

src/lxc/conf.h

@@ -473,7 +473,8 @@ extern int find_unmapped_nsid(struct lxc_conf *conf, enum idtype idtype);
 extern int mapped_hostid(unsigned id, struct lxc_conf *conf, enum idtype idtype);
 extern int chown_mapped_root(char *path, struct lxc_conf *conf);
 extern int ttys_shift_ids(struct lxc_conf *c);
-extern int userns_exec_1(struct lxc_conf *conf, int (*fn)(void *), void *data);
+extern int userns_exec_1(struct lxc_conf *conf, int (*fn)(void *), void *data,
+			 const char *fn_name);
 extern int parse_mntopts(const char *mntopts, unsigned long *mntflags,
 			 char **mntdata);
 extern void tmp_proc_unmount(struct lxc_conf *lxc_conf);

src/lxc/lxccontainer.c

@@ -2339,7 +2339,8 @@ static bool has_snapshots(struct lxc_container *c)
 
 static bool do_destroy_container(struct lxc_conf *conf) {
 	if (am_unpriv()) {
-		if (userns_exec_1(conf, bdev_destroy_wrapper, conf) < 0)
+		if (userns_exec_1(conf, bdev_destroy_wrapper, conf,
+				  "bdev_destroy_wrapper") < 0)
 			return false;
 		return true;
 	}
@@ -2421,7 +2422,8 @@ static bool container_destroy(struct lxc_container *c)
 	char *path = alloca(strlen(p1) + strlen(c->name) + 2);
 	sprintf(path, "%s/%s", p1, c->name);
 	if (am_unpriv())
-		ret = userns_exec_1(conf, lxc_rmdir_onedev_wrapper, path);
+		ret = userns_exec_1(conf, lxc_rmdir_onedev_wrapper, path,
+				    "lxc_rmdir_onedev_wrapper");
 	else
 		ret = lxc_rmdir_onedev(path, "snaps");
 	if (ret < 0) {
@@ -3230,7 +3232,7 @@ static struct lxc_container *do_lxcapi_clone(struct lxc_container *c, const char
 	data.hookargs = hookargs;
 	if (am_unpriv())
 		ret = userns_exec_1(c->lxc_conf, clone_update_rootfs_wrapper,
-				&data);
+				    &data, "clone_update_rootfs_wrapper");
 	else
 		ret = clone_update_rootfs(&data);
 	if (ret < 0)

src/lxc/start.c

@@ -1021,8 +1021,9 @@ static int recv_fd(int sock, int *fd)
 
 static int recv_ttys_from_child(struct lxc_handler *handler)
 {
+	int i, ret;
+	int sock = handler->ttysock[1];
 	struct lxc_conf *conf = handler->conf;
-	int i, sock = handler->ttysock[1];
 	struct lxc_tty_info *tty_info = &conf->tty_info;
 
 	if (!conf->tty)
@@ -1035,11 +1036,18 @@ static int recv_ttys_from_child(struct lxc_handler *handler)
 	for (i = 0; i < conf->tty; i++) {
 		struct lxc_pty_info *pty_info = &tty_info->pty_info[i];
 		pty_info->busy = 0;
-		if (recv_fd(sock, &pty_info->slave) < 0 ||
-		    recv_fd(sock, &pty_info->master) < 0) {
-			ERROR("Error receiving tty info from child process.");
+		ret = recv_fd(sock, &pty_info->slave);
+		if (ret >= 0)
+			recv_fd(sock, &pty_info->master);
+		if (ret < 0) {
+			ERROR("failed to receive pty with master fd %d and "
+			      "slave fd %d from child: %s",
+			      pty_info->master, pty_info->slave,
+			      strerror(errno));
 			return -1;
 		}
+		TRACE("received pty with master fd %d and slave fd %d from child",
+		      pty_info->master, pty_info->slave);
 	}
 	tty_info->nbtty = conf->tty;
 
@@ -1533,7 +1541,8 @@ static void lxc_destroy_container_on_signal(struct lxc_handler *handler,
 	}
 
 	if (am_unpriv())
-		ret = userns_exec_1(handler->conf, lxc_rmdir_onedev_wrapper, destroy);
+		ret = userns_exec_1(handler->conf, lxc_rmdir_onedev_wrapper,
+				    destroy, "lxc_rmdir_onedev_wrapper");
 	else
 		ret = lxc_rmdir_onedev(destroy, NULL);
 
@@ -1552,7 +1561,8 @@ static int lxc_rmdir_onedev_wrapper(void *data)
 
 static bool do_destroy_container(struct lxc_conf *conf) {
 	if (am_unpriv()) {
-		if (userns_exec_1(conf, bdev_destroy_wrapper, conf) < 0)
+		if (userns_exec_1(conf, bdev_destroy_wrapper, conf,
+				  "bdev_destroy_wrapper") < 0)
 			return false;
 		return true;
 	}