[lxc-devel] [PATCH 1/1] attach: mount a sane prox for LSM setup

Stéphane Graber stgraber at ubuntu.com
Tue May 26 17:05:50 UTC 2015


On Sun, May 17, 2015 at 01:04:47PM +0000, Serge Hallyn wrote:
> To set lsm labels, a namespace-local proc mount is needed.
> 
> If a container does not have a lxc.mount.auto = proc set, then
> tasks in the container do not have a correct /proc mount until
> init feels like doing the mount.  At startup we handlie this
> by mounting a temporary /proc if needed.  We weren't doing this
> at attach, though, so that
> 
> lxc-start -n $container
> lxc-wait -t 5 -s RUNNING -n $container
> lxc-attach -n $container -- uname -a
> 
> could in a racy way fail with something like
> 
> lxc-attach: lsm/apparmor.c: apparmor_process_label_set: 183 No such file or directory - failed to change apparmor profile to lxc-container-default
> 
> Thanks to Chris Townsend for finding this bug at
> https://bugs.launchpad.net/ubuntu/+source/lxc/+bug/1452451
> 
> Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>

Acked-by: Stéphane Graber <stgraber at ubuntu.com>

> ---
>  src/lxc/attach.c |  8 ++++++++
>  src/lxc/conf.c   | 44 +-------------------------------------------
>  src/lxc/utils.c  | 43 +++++++++++++++++++++++++++++++++++++++++++
>  src/lxc/utils.h  |  1 +
>  4 files changed, 53 insertions(+), 43 deletions(-)
> 
> diff --git a/src/lxc/attach.c b/src/lxc/attach.c
> index 69dafd4..731d7a6 100644
> --- a/src/lxc/attach.c
> +++ b/src/lxc/attach.c
> @@ -1040,10 +1040,18 @@ static int attach_child_main(void* data)
>  	/* set new apparmor profile/selinux context */
>  	if ((options->namespaces & CLONE_NEWNS) && (options->attach_flags & LXC_ATTACH_LSM)) {
>  		int on_exec;
> +		int proc_mounted;
>  
>  		on_exec = options->attach_flags & LXC_ATTACH_LSM_EXEC ? 1 : 0;
> +		proc_mounted = mount_proc_if_needed("/");
> +		if (proc_mounted == -1) {
> +			ERROR("Error mounting a sane /proc");
> +			rexit(-1);
> +		}
>  		ret = lsm_process_label_set(init_ctx->lsm_label,
>  				init_ctx->container->lxc_conf, 0, on_exec);
> +		if (proc_mounted)
> +			umount("/proc");
>  		if (ret < 0) {
>  			rexit(-1);
>  		}
> diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> index f0d2008..1b2ff05 100644
> --- a/src/lxc/conf.c
> +++ b/src/lxc/conf.c
> @@ -3551,48 +3551,6 @@ int ttys_shift_ids(struct lxc_conf *c)
>  	return 0;
>  }
>  
> -/*
> - * _do_tmp_proc_mount: Mount /proc inside container if not already
> - * mounted
> - *
> - * @rootfs : the rootfs where proc should be mounted
> - *
> - * Returns < 0 on failure, 0 if the correct proc was already mounted
> - * and 1 if a new proc was mounted.
> - */
> -static int do_tmp_proc_mount(const char *rootfs)
> -{
> -	char path[MAXPATHLEN];
> -	char link[20];
> -	int linklen, ret;
> -
> -	ret = snprintf(path, MAXPATHLEN, "%s/proc/self", rootfs);
> -	if (ret < 0 || ret >= MAXPATHLEN) {
> -		SYSERROR("proc path name too long");
> -		return -1;
> -	}
> -	memset(link, 0, 20);
> -	linklen = readlink(path, link, 20);
> -	INFO("I am %d, /proc/self points to '%s'", getpid(), link);
> -	ret = snprintf(path, MAXPATHLEN, "%s/proc", rootfs);
> -	if (linklen < 0) /* /proc not mounted */
> -		goto domount;
> -	/* can't be longer than rootfs/proc/1 */
> -	if (strncmp(link, "1", linklen) != 0) {
> -		/* wrong /procs mounted */
> -		umount2(path, MNT_DETACH); /* ignore failure */
> -		goto domount;
> -	}
> -	/* the right proc is already mounted */
> -	return 0;
> -
> -domount:
> -	if (mount("proc", path, "proc", 0, NULL))
> -		return -1;
> -	INFO("Mounted /proc in container for security transition");
> -	return 1;
> -}
> -
>  int tmp_proc_mount(struct lxc_conf *lxc_conf)
>  {
>  	int mounted;
> @@ -3604,7 +3562,7 @@ int tmp_proc_mount(struct lxc_conf *lxc_conf)
>  		} else
>  			mounted = 1;
>  	} else
> -		mounted = do_tmp_proc_mount(lxc_conf->rootfs.mount);
> +		mounted = mount_proc_if_needed(lxc_conf->rootfs.mount);
>  	if (mounted == -1) {
>  		SYSERROR("failed to mount /proc in the container.");
>  		return -1;
> diff --git a/src/lxc/utils.c b/src/lxc/utils.c
> index fe71e9a..0588fdd 100644
> --- a/src/lxc/utils.c
> +++ b/src/lxc/utils.c
> @@ -1663,3 +1663,46 @@ int setproctitle(char *title)
>  
>  	return ret;
>  }
> +
> +/*
> + * Mount a proc under @rootfs if proc self points to a pid other than
> + * my own.  This is needed to have a known-good proc mount for setting
> + * up LSMs both at container startup and attach.
> + *
> + * @rootfs : the rootfs where proc should be mounted
> + *
> + * Returns < 0 on failure, 0 if the correct proc was already mounted
> + * and 1 if a new proc was mounted.
> + */
> +int mount_proc_if_needed(const char *rootfs)
> +{
> +	char path[MAXPATHLEN];
> +	char link[20];
> +	int linklen, ret;
> +
> +	ret = snprintf(path, MAXPATHLEN, "%s/proc/self", rootfs);
> +	if (ret < 0 || ret >= MAXPATHLEN) {
> +		SYSERROR("proc path name too long");
> +		return -1;
> +	}
> +	memset(link, 0, 20);
> +	linklen = readlink(path, link, 20);
> +	INFO("I am %d, /proc/self points to '%s'", getpid(), link);
> +	ret = snprintf(path, MAXPATHLEN, "%s/proc", rootfs);
> +	if (linklen < 0) /* /proc not mounted */
> +		goto domount;
> +	/* can't be longer than rootfs/proc/1 */
> +	if (strncmp(link, "1", linklen) != 0) {
> +		/* wrong /procs mounted */
> +		umount2(path, MNT_DETACH); /* ignore failure */
> +		goto domount;
> +	}
> +	/* the right proc is already mounted */
> +	return 0;
> +
> +domount:
> +	if (mount("proc", path, "proc", 0, NULL))
> +		return -1;
> +	INFO("Mounted /proc in container for security transition");
> +	return 1;
> +}
> diff --git a/src/lxc/utils.h b/src/lxc/utils.h
> index cc18906..95ea2af 100644
> --- a/src/lxc/utils.h
> +++ b/src/lxc/utils.h
> @@ -287,3 +287,4 @@ bool switch_to_ns(pid_t pid, const char *ns);
>  int is_dir(const char *path);
>  char *get_template_path(const char *t);
>  int setproctitle(char *title);
> +int mount_proc_if_needed(const char *rootfs);
> -- 
> 2.1.4
> 
> _______________________________________________
> 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/20150526/52b84dce/attachment-0001.sig>


More information about the lxc-devel mailing list