[lxc-devel] [PATCH] uidmap: fix writing multiple ranges

[Serge Hallyn]

Quoting Dwight Engen (dwight.engen at oracle.com):
> The kernel requires a single atomic write for setting the /proc
> idmap files. We were calling write(2) more than once when multiple
> ranges were configured so instead build a buffer to pass in one write(2)
> call.
> 
> Change id types to unsigned long to handle large id mappings gracefully.
> 
> Fix max id in example comment.
> 
> Signed-off-by: Dwight Engen <dwight.engen at oracle.com>

I'm really not sure that was intended, but I see it is in fact the case.
Thanks Dwight :)

Acked-by: Serge Hallyn <serge.hallyn at ubuntu.com>

> ---
>  src/lxc/conf.c    | 34 ++++++++++++++++++++++++++++------
>  src/lxc/conf.h    | 16 ++++++++--------
>  src/lxc/confile.c |  6 +++---
>  3 files changed, 39 insertions(+), 17 deletions(-)
> 
> diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> index e2abc72..9700c7a 100644
> --- a/src/lxc/conf.c
> +++ b/src/lxc/conf.c
> @@ -2447,7 +2447,8 @@ int lxc_assign_network(struct lxc_list *network, pid_t pid)
>  	return 0;
>  }
>  
> -static int add_id_mapping(enum idtype idtype, pid_t pid, uid_t ns_start, uid_t host_start, int range)
> +static int write_id_mapping(enum idtype idtype, pid_t pid, const char *buf,
> +			    size_t buf_size)
>  {
>  	char path[PATH_MAX];
>  	int ret, closeret;
> @@ -2463,7 +2464,7 @@ static int add_id_mapping(enum idtype idtype, pid_t pid, uid_t ns_start, uid_t h
>  		perror("open");
>  		return -EINVAL;
>  	}
> -	ret = fprintf(f, "%d %d %d", ns_start, host_start, range);
> +	ret = fwrite(buf, buf_size, 1, f);
>  	if (ret < 0)
>  		SYSERROR("writing id mapping");
>  	closeret = fclose(f);
> @@ -2477,13 +2478,34 @@ int lxc_map_ids(struct lxc_list *idmap, pid_t pid)
>  	struct lxc_list *iterator;
>  	struct id_map *map;
>  	int ret = 0;
> -
> -	lxc_list_for_each(iterator, idmap) {
> -		map = iterator->elem;
> -		ret = add_id_mapping(map->idtype, pid, map->nsid, map->hostid, map->range);
> +	char *buf,*pos;
> +	enum idtype type;
> +
> +	/* The kernel only takes <= 4k for writes to /proc/<nr>/[ug]id_map */
> +	buf = pos = malloc(4096);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	for(type = ID_TYPE_UID; type <= ID_TYPE_GID; type++) {
> +		int left,fill;
> +		lxc_list_for_each(iterator, idmap) {
> +			map = iterator->elem;
> +			if (map->idtype == type) {
> +				left = 4096 - (pos - buf);
> +				fill = snprintf(pos, left, "%lu %lu %lu\n",
> +					map->nsid, map->hostid, map->range);
> +				if (fill <= 0 || fill >= left)
> +					SYSERROR("snprintf failed, too many mappings");
> +				pos += fill;
> +			}
> +		}
> +		ret = write_id_mapping(type, pid, buf, pos-buf);
>  		if (ret)
>  			break;
> +		pos = buf;
>  	}
> +
> +	free(buf);
>  	return ret;
>  }
>  
> diff --git a/src/lxc/conf.h b/src/lxc/conf.h
> index f20fb2f..cc7961a 100644
> --- a/src/lxc/conf.h
> +++ b/src/lxc/conf.h
> @@ -149,17 +149,17 @@ enum idtype {
>  
>  /*
>   * id_map is an id map entry.  Form in confile is:
> - * lxc.id_map = U 9800 0 100
> - * lxc.id_map = U 9900 1000 100
> - * lxc.id_map = G 9800 0 100
> - * lxc.id_map = G 9900 1000 100
> - * meaning the container can use uids and gids 0-100 and 1000-1100,
> - * with uid 0 mapping to uid 9800 on the host, and gid 1000 to
> - * gid 9900 on the host.
> + * lxc.id_map = u 0    9800 100
> + * lxc.id_map = u 1000 9900 100
> + * lxc.id_map = g 0    9800 100
> + * lxc.id_map = g 1000 9900 100
> + * meaning the container can use uids and gids 0-99 and 1000-1099,
> + * with [ug]id 0 mapping to [ug]id 9800 on the host, and [ug]id 1000 to
> + * [ug]id 9900 on the host.
>   */
>  struct id_map {
>  	enum idtype idtype;
> -	int hostid, nsid, range;
> +	unsigned long hostid, nsid, range;
>  };
>  
>  /*
> diff --git a/src/lxc/confile.c b/src/lxc/confile.c
> index 59cedef..0b1fd51 100644
> --- a/src/lxc/confile.c
> +++ b/src/lxc/confile.c
> @@ -1024,7 +1024,7 @@ static int config_idmap(const char *key, const char *value, struct lxc_conf *lxc
>  	char *subkey;
>  	struct lxc_list *idmaplist = NULL;
>  	struct id_map *idmap = NULL;
> -	int hostid, nsid, range;
> +	unsigned long hostid, nsid, range;
>  	char type;
>  	int ret;
>  
> @@ -1049,10 +1049,10 @@ static int config_idmap(const char *key, const char *value, struct lxc_conf *lxc
>  
>  	lxc_list_add_tail(&lxc_conf->id_map, idmaplist);
>  
> -	ret = sscanf(value, "%c %d %d %d", &type, &nsid, &hostid, &range);
> +	ret = sscanf(value, "%c %lu %lu %lu", &type, &nsid, &hostid, &range);
>  	if (ret != 4)
>  		goto out;
> -	INFO("read uid map: type %c nsid %d hostid %d range %d", type, nsid, hostid, range);
> +	INFO("read uid map: type %c nsid %lu hostid %lu range %lu", type, nsid, hostid, range);
>  	if (type == 'u')
>  		idmap->idtype = ID_TYPE_UID;
>  	else if (type == 'g')
> -- 
> 1.7.12.3
> 
> 
> ------------------------------------------------------------------------------
> Symantec Endpoint Protection 12 positioned as A LEADER in The Forrester  
> Wave(TM): Endpoint Security, Q1 2013 and "remains a good choice" in the  
> endpoint security space. For insight on selecting the right partner to 
> tackle endpoint security challenges, access the full report. 
> http://p.sf.net/sfu/symantec-dev2dev
> _______________________________________________
> Lxc-devel mailing list
> Lxc-devel at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/lxc-devel