[lxc-devel] [PATCH 1/1] cgmanager: use absolute cgroup path to switch cgroups at attach

Stéphane Graber stgraber at ubuntu.com
Wed May 7 03:42:27 UTC 2014


On Thu, May 01, 2014 at 03:27:55PM -0500, Serge Hallyn wrote:
> If an unprivileged user does 'lxc-start -n u1' in one
> login session, followed by 'lxc-attach -n u1' in another
> session, the attach will fail if the sessions are in different
> cgroups.  The same is true of lxc-cgroup commands.
> 
> Address this by using the GetPidCgroupAbs and MovePidAbs
> which work with the containers' cgroup path relative to
> the cgproxy.
> 
> Since GetPidCgroupAbs is new to api version 3 in cgmanager,
> use the old method if we are on an older cgmanager.
> 
> Signed-off-by: Serge Hallyn <serge.hallyn at ubuntu.com>

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

> ---
>  configure.ac        |   8 ++++
>  src/lxc/cgmanager.c | 132 ++++++++++++++++++++++++++++++++++++----------------
>  2 files changed, 99 insertions(+), 41 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index f3125f1..8865bc8 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -256,6 +256,14 @@ AM_COND_IF([ENABLE_CGMANAGER],
>  	PKG_CHECK_MODULES([DBUS], [dbus-1 >= 1.2.16])
>  	])
>  
> +AC_MSG_CHECKING(for get_pid_cgroup_abs_sync)
> +AC_SEARCH_LIBS([cgmanager_get_pid_cgroup_abs_sync], [cgmanager], [have_abs_cgroups=yes], [have_abs_cgroups=no], [-lnih -lnih-dbus -ldbus-1])
> +if test "x$have_abs_cgroups" = "xyes"; then
> +	AC_DEFINE([HAVE_CGMANAGER_GET_PID_CGROUP_ABS_SYNC], 1, [Have cgmanager_get_pid_cgroup_abs_sync])
> +	AC_MSG_RESULT([yes])
> +else
> +	AC_MSG_RESULT([no])
> +fi
>  # Linux capabilities
>  AC_ARG_ENABLE([capabilities],
>  	[AC_HELP_STRING([--enable-capabilities], [enable kernel capabilities support [default=auto]])],
> diff --git a/src/lxc/cgmanager.c b/src/lxc/cgmanager.c
> index fc959ec..5fe3a36 100644
> --- a/src/lxc/cgmanager.c
> +++ b/src/lxc/cgmanager.c
> @@ -107,6 +107,7 @@ static void process_lock_setup_atfork(void)
>  #endif
>  
>  static NihDBusProxy *cgroup_manager = NULL;
> +static int32_t api_version;
>  
>  static struct cgroup_ops cgmanager_ops;
>  static int nr_subsystems;
> @@ -162,11 +163,11 @@ static bool cgm_dbus_connect(void)
>  		return false;
>  	}
>  
> -	// force fd passing negotiation
> -	if (cgmanager_ping_sync(NULL, cgroup_manager, 0) != 0) {
> +	// get the api version
> +	if (cgmanager_get_api_version_sync(NULL, cgroup_manager, &api_version) != 0) {
>  		NihError *nerr;
>  		nerr = nih_error_get();
> -		ERROR("Error pinging cgroup manager: %s", nerr->message);
> +		ERROR("Error cgroup manager api version: %s", nerr->message);
>  		nih_free(nerr);
>  		cgm_dbus_disconnect();
>  		return false;
> @@ -554,13 +555,21 @@ bad:
>   * Internal helper, must be called with cgmanager dbus socket open
>   */
>  static bool lxc_cgmanager_enter(pid_t pid, const char *controller,
> -		const char *cgroup_path)
> +		const char *cgroup_path, bool abs)
>  {
> -	if (cgmanager_move_pid_sync(NULL, cgroup_manager, controller,
> -			cgroup_path, pid) != 0) {
> +	int ret;
> +
> +	if (abs)
> +		ret = cgmanager_move_pid_abs_sync(NULL, cgroup_manager,
> +			controller, cgroup_path, pid);
> +	else
> +		ret = cgmanager_move_pid_sync(NULL, cgroup_manager,
> +			controller, cgroup_path, pid);
> +	if (ret != 0) {
>  		NihError *nerr;
>  		nerr = nih_error_get();
> -		ERROR("call to cgmanager_move_pid_sync failed: %s", nerr->message);
> +		ERROR("call to cgmanager_move_pid_%ssync failed: %s",
> +			abs ? "abs_" : "", nerr->message);
>  		nih_free(nerr);
>  		return false;
>  	}
> @@ -568,12 +577,12 @@ static bool lxc_cgmanager_enter(pid_t pid, const char *controller,
>  }
>  
>  /* Internal helper, must be called with cgmanager dbus socket open */
> -static bool do_cgm_enter(pid_t pid, const char *cgroup_path)
> +static bool do_cgm_enter(pid_t pid, const char *cgroup_path, bool abs)
>  {
>  	int i;
>  
>  	for (i = 0; i < nr_subsystems; i++) {
> -		if (!lxc_cgmanager_enter(pid, subsystems[i], cgroup_path))
> +		if (!lxc_cgmanager_enter(pid, subsystems[i], cgroup_path, abs))
>  			return false;
>  	}
>  	return true;
> @@ -590,7 +599,7 @@ static inline bool cgm_enter(void *hdata, pid_t pid)
>  	}
>  	if (!d || !d->cgroup_path)
>  		goto out;
> -	if (do_cgm_enter(pid, d->cgroup_path))
> +	if (do_cgm_enter(pid, d->cgroup_path, false))
>  		ret = true;
>  out:
>  	cgm_dbus_disconnect();
> @@ -606,6 +615,41 @@ static const char *cgm_get_cgroup(void *hdata, const char *subsystem)
>  	return d->cgroup_path;
>  }
>  
> +#if HAVE_CGMANAGER_GET_PID_CGROUP_ABS_SYNC
> +static inline bool abs_cgroup_supported(void) {
> +	return api_version >= 3;
> +}
> +#else
> +static inline bool abs_cgroup_supported(void) {
> +	return false;
> +}
> +#define cgmanager_get_pid_cgroup_abs_sync(...) -1
> +#endif
> +
> +static char *try_get_abs_cgroup(const char *name, const char *lxcpath,
> +		const char *controller)
> +{
> +	char *cgroup = NULL;
> +
> +	if (abs_cgroup_supported()) {
> +		/* get the container init pid and ask for its abs cgroup */
> +		pid_t pid = lxc_cmd_get_init_pid(name, lxcpath);
> +		if (pid < 0)
> +			return NULL;
> +		if (cgmanager_get_pid_cgroup_abs_sync(NULL, cgroup_manager,
> +				controller, pid, &cgroup) != 0) {
> +			cgroup = NULL;
> +			NihError *nerr;
> +			nerr = nih_error_get();
> +			nih_free(nerr);
> +		}
> +		return cgroup;
> +	}
> +
> +	/* use the command interface to look for the cgroup */
> +	return lxc_cmd_get_cgroup_path(name, lxcpath, controller);
> +}
> +
>  /*
>   * nrtasks is called by the utmp helper by the container monitor.
>   * cgmanager socket was closed after cgroup setup was complete, so we need
> @@ -641,10 +685,20 @@ out:
>  	return pids_len;
>  }
>  
> +static inline void free_abs_cgroup(char *cgroup)
> +{
> +	if (!cgroup)
> +		return;
> +	if (abs_cgroup_supported())
> +		nih_free(cgroup);
> +	else
> +		free(cgroup);
> +}
> +
>  /* cgm_get is called to get container cgroup settings, not during startup */
>  static int cgm_get(const char *filename, char *value, size_t len, const char *name, const char *lxcpath)
>  {
> -	char *result, *controller, *key, *cgroup;
> +	char *result, *controller, *key, *cgroup = NULL;
>  	size_t newlen;
>  
>  	controller = alloca(strlen(filename)+1);
> @@ -654,14 +708,17 @@ static int cgm_get(const char *filename, char *value, size_t len, const char *na
>  		return -1;
>  	*key = '\0';
>  
> -	/* use the command interface to look for the cgroup */
> -	cgroup = lxc_cmd_get_cgroup_path(name, lxcpath, controller);
> -	if (!cgroup)
> -		return -1;
>  	if (!cgm_dbus_connect()) {
>  		ERROR("Error connecting to cgroup manager");
>  		return -1;
>  	}
> +
> +	cgroup = try_get_abs_cgroup(name, lxcpath, controller);
> +	if (!cgroup) {
> +		cgm_dbus_disconnect();
> +		return -1; 
> +	}
> +
>  	if (cgmanager_get_value_sync(NULL, cgroup_manager, controller, cgroup, filename, &result) != 0) {
>  		/*
>  		 * must consume the nih error
> @@ -671,12 +728,12 @@ static int cgm_get(const char *filename, char *value, size_t len, const char *na
>  		NihError *nerr;
>  		nerr = nih_error_get();
>  		nih_free(nerr);
> -		free(cgroup);
> +		free_abs_cgroup(cgroup);
>  		cgm_dbus_disconnect();
>  		return -1;
>  	}
>  	cgm_dbus_disconnect();
> -	free(cgroup);
> +	free_abs_cgroup(cgroup);
>  	newlen = strlen(result);
>  	if (!len || !value) {
>  		// user queries the size
> @@ -717,7 +774,7 @@ static int cgm_do_set(const char *controller, const char *file,
>  /* cgm_set is called to change cgroup settings, not during startup */
>  static int cgm_set(const char *filename, const char *value, const char *name, const char *lxcpath)
>  {
> -	char *controller, *key, *cgroup;
> +	char *controller, *key, *cgroup = NULL;
>  	int ret;
>  
>  	controller = alloca(strlen(filename)+1);
> @@ -727,21 +784,21 @@ static int cgm_set(const char *filename, const char *value, const char *name, co
>  		return -1;
>  	*key = '\0';
>  
> -	/* use the command interface to look for the cgroup */
> -	cgroup = lxc_cmd_get_cgroup_path(name, lxcpath, controller);
> -	if (!cgroup) {
> -		ERROR("Failed to get cgroup for controller %s for %s:%s",
> -			controller, lxcpath, name);
> -		return -1;
> -	}
>  	if (!cgm_dbus_connect()) {
>  		ERROR("Error connecting to cgroup manager");
>  		free(cgroup);
>  		return false;
>  	}
> +	cgroup = try_get_abs_cgroup(name, lxcpath, controller);
> +	if (!cgroup) {
> +		ERROR("Failed to get cgroup for controller %s for %s:%s",
> +			controller, lxcpath, name);
> +		cgm_dbus_disconnect();
> +		return -1;
> +	}
>  	ret = cgm_do_set(controller, filename, cgroup, value);
>  	cgm_dbus_disconnect();
> -	free(cgroup);
> +	free_abs_cgroup(cgroup);
>  	return ret;
>  }
>  
> @@ -932,36 +989,29 @@ static bool cgm_chown(void *hdata, struct lxc_conf *conf)
>   */
>  static bool cgm_attach(const char *name, const char *lxcpath, pid_t pid)
>  {
> -	bool pass = false;
> +	bool pass;
>  	char *cgroup = NULL;
> -	struct lxc_container *c;
>  
> -	c = lxc_container_new(name, lxcpath);
> -	if (!c) {
> -		ERROR("Could not load container %s:%s", lxcpath, name);
> +	if (!cgm_dbus_connect()) {
> +		ERROR("Error connecting to cgroup manager");
>  		return false;
>  	}
>  	// cgm_create makes sure that we have the same cgroup name for all
>  	// subsystems, so since this is a slow command over the cmd socket,
>  	// just get the cgroup name for the first one.
> -	cgroup = lxc_cmd_get_cgroup_path(name, lxcpath, subsystems[0]);
> +	cgroup = try_get_abs_cgroup(name, lxcpath, subsystems[0]);
>  	if (!cgroup) {
>  		ERROR("Failed to get cgroup for controller %s", subsystems[0]);
> -		goto out;
> +		cgm_dbus_disconnect();
> +		return false;
>  	}
>  
> -	if (!cgm_dbus_connect()) {
> -		ERROR("Error connecting to cgroup manager");
> -		goto out;
> -	}
> -	pass = do_cgm_enter(pid, cgroup);
> +	pass = do_cgm_enter(pid, cgroup, abs_cgroup_supported());
>  	cgm_dbus_disconnect();
>  	if (!pass)
>  		ERROR("Failed to enter group %s", cgroup);
>  
> -out:
> -	free(cgroup);
> -	lxc_container_put(c);
> +	free_abs_cgroup(cgroup);
>  	return pass;
>  }
>  
> -- 
> 1.9.1
> 
> _______________________________________________
> 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/20140506/c0c80399/attachment.sig>


More information about the lxc-devel mailing list