[lxc-devel] [PATCH] Set default paths for unprivileged use

[Stéphane Graber]

On Thu, Dec 19, 2013 at 05:32:57PM -0600, Serge Hallyn wrote:
> Quoting Stéphane Graber (stgraber at ubuntu.com):
> > When running unprivileged (euid != 0), LXC will now use the following paths:
> >  - Default lxc path: ~/.local/share/lxc/
> >  - Default config path: ~/.config/lxc/lxc.conf
> > 
> > Those two paths are based on standard XDG paths (though ignoring all the
> > possible override paths for now at least) and so probably don't need to
> > be configurable at build time.
> > 
> > Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
> 
> Two comments on freeing user_lxc_path:
> 
> > ---
> >  src/lxc/utils.c | 37 ++++++++++++++++++++++++++++++++++---
> >  1 file changed, 34 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/lxc/utils.c b/src/lxc/utils.c
> > index 785f3e6..9fd91b7 100644
> > --- a/src/lxc/utils.c
> > +++ b/src/lxc/utils.c
> > @@ -39,6 +39,7 @@
> >  #include <sys/types.h>
> >  #include <sys/wait.h>
> >  #include <assert.h>
> > +#include <wordexp.h>
> >  
> >  #ifndef HAVE_GETLINE
> >  #ifdef HAVE_FGETLN
> > @@ -248,11 +249,30 @@ const char *lxc_global_config_value(const char *option_name)
> >  		{ "lvm_vg",          DEFAULT_VG      },
> >  		{ "lvm_thin_pool",   DEFAULT_THIN_POOL },
> >  		{ "zfsroot",         DEFAULT_ZFSROOT },
> > -		{ "lxcpath",         LXCPATH         },
> > +		{ "lxcpath",         NULL            },
> >  		{ "cgroup.pattern",  DEFAULT_CGROUP_PATTERN },
> >  		{ "cgroup.use",      NULL            },
> >  		{ NULL, NULL },
> >  	};
> > +
> > +	char *user_config_path;
> > +	char *user_lxc_path;
> > +	wordexp_t exp;
> > +
> > +	if (geteuid() > 0) {
> > +		wordexp("~/.config/lxc/lxc.conf", &exp, 0);
> > +		user_config_path = strdup(exp.we_wordv[0]);
> > +		wordfree(&exp);
> > +
> > +		wordexp("~/.local/share/lxc/", &exp, 0);
> > +		user_lxc_path = strdup(exp.we_wordv[0]);
> > +		wordfree(&exp);
> > +	}
> > +	else {
> > +		user_config_path = strdup(LXC_GLOBAL_CONF);
> > +		user_lxc_path = strdup(LXCPATH);
> > +	}
> > +
> >  	/* placed in the thread local storage pool */
> >  	static __thread const char *values[sizeof(options) / sizeof(options[0])] = { 0 };
> >  	const char *(*ptr)[2];
> > @@ -266,17 +286,23 @@ const char *lxc_global_config_value(const char *option_name)
> >  			break;
> >  	}
> >  	if (!(*ptr)[0]) {
> > +		free(user_config_path);
> > +		free(user_lxc_path);
> >  		errno = EINVAL;
> >  		return NULL;
> >  	}
> >  
> >  	if (values[i]) {
> > +		free(user_config_path);
> > +		free(user_lxc_path);
> >  		value = values[i];
> > +
> >  		return value;
> >  	}
> >  
> >  	process_lock();
> > -	fin = fopen_cloexec(LXC_GLOBAL_CONF, "r");
> > +	fin = fopen_cloexec(user_config_path, "r");
> > +	free(user_config_path);
> >  	process_unlock();
> >  	if (fin) {
> >  		while (fgets(buf, 1024, fin)) {
> > @@ -315,7 +341,12 @@ const char *lxc_global_config_value(const char *option_name)
> >  		}
> >  	}
> >  	/* could not find value, use default */
> > -	values[i] = (*ptr)[1];
> > +	if (strcmp(option_name, "lxcpath") == 0)
> > +		values[i] = user_lxc_path;
> > +	else {
> > +		free(user_config_path);
> 
> Did you mean to free user_lxc_path here?

I did indeed, user_config_path is always freed right after fopen.

> 
> > +		values[i] = (*ptr)[1];
> > +	}
> >  	/* special case: if default value is NULL,
> >  	 * and there is no config, don't view that
> >  	 * as an error... */
> 
> Below here is an out: path.  I don't think you're freeing
> user_lxc_path on that codepath.

So with the free above corrected to free user_config_path, it'll either
have been already freed or be returned so shouldn't need any further
freeing.

> > -- 
> > 1.8.5.2
> > 
> > _______________________________________________
> > lxc-devel mailing list
> > lxc-devel at lists.linuxcontainers.org
> > http://lists.linuxcontainers.org/listinfo/lxc-devel
> _______________________________________________
> 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: 836 bytes
Desc: Digital signature
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20131220/6675af6c/attachment.pgp>