[lxc-devel] [PATCH RFC] do_mount_entry: add nexec, nosuid, nodev, rdonly flags if needed at remount

[Stéphane Graber]

On Mon, Aug 18, 2014 at 03:28:21AM +0000, Serge Hallyn wrote:
> See http://lkml.org/lkml/2014/8/13/746 and its history.  The kernel now refuses
> mounts if we don't add ro,nosuid,nodev,noexec flags if they were already there.
> 
> Also use the newly found info to skip remount if unneeded.  For background, if
> you want to create a read-only bind mount, then you must first mount(2) with
> MS_BIND to create the bind mount, then re-mount(2) again to get the new mount
> options to apply.  So if this wasn't a bind mount, or no new mount options were
> introduced, then we don't do the second mount(2).
> 
> null_endofword() and get_field() were not changed, only moved up in
> the file.
> 
> (Note, while I can start containers inside a privileged container with
> this patch, most of the lxc tests still fail with the kernel in question;
> Andy's patch seems to still be needed - a kernel with which is available
> at https://launchpad.net/~serge-hallyn/+archive/ubuntu/userns-natty
> ppa:serge-hallyn/userns-natty)
> 
> Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>

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

> ---
>  src/lxc/conf.c | 107 ++++++++++++++++++++++++++++++++++++++++++---------------
>  1 file changed, 80 insertions(+), 27 deletions(-)
> 
> diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> index 2b3aff1..01ace43 100644
> --- a/src/lxc/conf.c
> +++ b/src/lxc/conf.c
> @@ -1927,10 +1927,74 @@ int parse_mntopts(const char *mntopts, unsigned long *mntflags,
>  	return 0;
>  }
>  
> +static void null_endofword(char *word)
> +{
> +	while (*word && *word != ' ' && *word != '\t')
> +		word++;
> +	*word = '\0';
> +}
> +
> +/*
> + * skip @nfields spaces in @src
> + */
> +static char *get_field(char *src, int nfields)
> +{
> +	char *p = src;
> +	int i;
> +
> +	for (i = 0; i < nfields; i++) {
> +		while (*p && *p != ' ' && *p != '\t')
> +			p++;
> +		if (!*p)
> +			break;
> +		p++;
> +	}
> +	return p;
> +}
> +
> +static unsigned long get_mount_flags(const char *path)
> +{
> +	FILE *f = fopen("/proc/self/mountinfo", "r");
> +	char *line = NULL;
> +	size_t len = 0;
> +	unsigned long flags = 0;
> +
> +	if (!f) {
> +		WARN("Failed to open /proc/self/mountinfo");
> +		return 0;
> +	}
> +	while (getline(&line, &len, f) != -1) {
> +		char *target, *opts, *p, *saveptr = NULL;
> +		target = get_field(line, 4);
> +		if (!target)
> +			continue;
> +		opts = get_field(target, 2);
> +		if (!opts)
> +			continue;
> +		null_endofword(opts);
> +		for (p = strtok_r(opts, ",", &saveptr); p;
> +			p = strtok_r(NULL, ",", &saveptr)) {
> +			if (strcmp(p, "ro") == 0)
> +				flags |= MS_RDONLY;
> +			else if (strcmp(p, "nodev") == 0)
> +				flags |= MS_NODEV;
> +			else if (strcmp(p, "nosuid") == 0)
> +				flags |= MS_NOSUID;
> +			else if (strcmp(p, "noexec") == 0)
> +				flags |= MS_NOEXEC;
> +			/* XXX todo - we'll have to deal with atime? */
> +		}
> +	}
> +	free(line);
> +	fclose(f);
> +	return flags;
> +}
> +
>  static int mount_entry(const char *fsname, const char *target,
>  		       const char *fstype, unsigned long mountflags,
>  		       const char *data, int optional)
>  {
> +	unsigned long extraflags;
>  	if (mount(fsname, target, fstype, mountflags & ~MS_REMOUNT, data)) {
>  		if (optional) {
>  			INFO("failed to mount '%s' on '%s' (optional): %s", fsname,
> @@ -1944,9 +2008,22 @@ static int mount_entry(const char *fsname, const char *target,
>  	}
>  
>  	if ((mountflags & MS_REMOUNT) || (mountflags & MS_BIND)) {
> +		extraflags = get_mount_flags(target);
> +		DEBUG("flags was %lu, extraflags set to %lu\n", mountflags, extraflags);
> +		if (!(mountflags & MS_REMOUNT) && (mountflags & MS_BIND)) {
> +			if (!(extraflags & ~mountflags))
> +				DEBUG("Skipping remount");
> +				goto skipremount;
> +		}
> +		DEBUG("proceeding with remont");
> +		mountflags |= extraflags;
> +	}
>  
> -		DEBUG("remounting %s on %s to respect bind or remount options",
> -		      fsname, target);
> +
> +	if ((mountflags & MS_REMOUNT) || (mountflags & MS_BIND)) {
> +		DEBUG("remounting %s on %s to respect bind or remount options %lu (extra %lu)",
> +		      fsname ? fsname : "(none)",
> +		      target ? target : "(none)", mountflags, extraflags);
>  
>  		if (mount(fsname, target, fstype,
>  			  mountflags | MS_REMOUNT, data)) {
> @@ -1963,6 +2040,7 @@ static int mount_entry(const char *fsname, const char *target,
>  		}
>  	}
>  
> +skipremount:
>  	DEBUG("mounted '%s' on '%s', type '%s'", fsname, target, fstype);
>  
>  	return 0;
> @@ -3888,31 +3966,6 @@ void tmp_proc_unmount(struct lxc_conf *lxc_conf)
>  	}
>  }
>  
> -static void null_endofword(char *word)
> -{
> -	while (*word && *word != ' ' && *word != '\t')
> -		word++;
> -	*word = '\0';
> -}
> -
> -/*
> - * skip @nfields spaces in @src
> - */
> -static char *get_field(char *src, int nfields)
> -{
> -	char *p = src;
> -	int i;
> -
> -	for (i = 0; i < nfields; i++) {
> -		while (*p && *p != ' ' && *p != '\t')
> -			p++;
> -		if (!*p)
> -			break;
> -		p++;
> -	}
> -	return p;
> -}
> -
>  static void remount_all_slave(void)
>  {
>  	/* walk /proc/mounts and change any shared entries to slave */
> -- 
> 2.1.0.rc1
> 
> _______________________________________________
> 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/20140817/95338d4c/attachment.sig>