Re: [devel] [PATCH hasher-priv v1 3/3] Add cgroup support

[Arseny Maslennikov <arseny@altlinux.org>]

[-- Attachment #1: Type: text/plain, Size: 7807 bytes --]

On Fri, Dec 13, 2019 at 12:42:05PM +0100, Alex Gladkov wrote:
> From: Alexey Gladkov <legion@altlinux.org>
> 

Could you please explain what you're trying to do with this patch?
Even if it's obvious from the source itself, we still must have an
opportunity to discuss, and a decent explanation should stay in the
project history.

Most likely, it'll turn out we _at least_ have to pass Delegate=yes to
the systemd service:

       Delegate=
           Turns on delegation of further resource control
           partitioning to processes of the unit. Units where
           this is enabled may create and manage their own
           private subhierarchy of control groups below the
           control group of the unit itself.
Manual page systemd.resource-control(5): lines 786-791

Do we only support cgroup2 and ignore cgroup1? If yes, great, but
perhaps then we might want to have a setting to not fiddle with cgroup
trees, to support the unfortunate users that have to run Docker and
other garbage.

> Signed-off-by: Alexey Gladkov <legion@altlinux.org>
> ---
>  hasher-priv/Makefile      |   2 +-
>  hasher-priv/caller_task.c |   3 +
>  hasher-priv/cgroup.c      | 119 ++++++++++++++++++++++++++++++++++++++
>  hasher-priv/config.c      |   5 ++
>  hasher-priv/priv.h        |   2 +
>  hasher-priv/server.conf   |   9 +++
>  6 files changed, 139 insertions(+), 1 deletion(-)
>  create mode 100644 hasher-priv/cgroup.c
> 
> diff --git a/hasher-priv/Makefile b/hasher-priv/Makefile
> index c73216f..e999972 100644
> --- a/hasher-priv/Makefile
> +++ b/hasher-priv/Makefile
> @@ -51,7 +51,7 @@ server_SRC = hasher-privd.c \
>  	chdir.c chdiruid.c chid.c child.c chrootuid.c cmdline.c \
>  	config.c fds.c getconf.c getugid.c ipc.c killuid.c io_log.c io_x11.c \
>  	makedev.c mount.c net.c parent.c pass.c pty.c signal.c tty.c \
> -	unshare.c xmalloc.c x11.c
> +	unshare.c xmalloc.c x11.c cgroup.c
>  server_OBJ = $(server_SRC:.c=.o)
>  
>  DEP = $(SRC:.c=.d) $(server_SRC:.c=.d)
> diff --git a/hasher-priv/caller_task.c b/hasher-priv/caller_task.c
> index d8f2dd5..722e0a6 100644
> --- a/hasher-priv/caller_task.c
> +++ b/hasher-priv/caller_task.c
> @@ -95,6 +95,9 @@ caller_task(struct task *task)
>  		return pid;
>  	}
>  
> +	if (join_cgroup() < 0)
> +		exit(rc);
> +
>  	if ((rc = reopen_iostreams(task->stdin, task->stdout, task->stderr)) < 0)
>  		exit(rc);
>  
> diff --git a/hasher-priv/cgroup.c b/hasher-priv/cgroup.c
> new file mode 100644
> index 0000000..ac14938
> --- /dev/null
> +++ b/hasher-priv/cgroup.c
> @@ -0,0 +1,119 @@
> +
> +/*
> +  Copyright (C) 2019  Alexey Gladkov <legion@altlinux.org>
> +
> +  The cgroup helper for hasher-privd program.
> +
> +  SPDX-License-Identifier: GPL-2.0-or-later
> +*/
> +
> +#include <sys/param.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +
> +#include <unistd.h>
> +#include <stdio.h>
> +#include <stdarg.h>
> +#include <string.h>
> +#include <fcntl.h>
> +#include <errno.h>
> +
> +#include "logging.h"
> +#include "priv.h"
> +
> +int
> +join_cgroup(void)
> +{
> +	int ret = 0;
> +
> +	if (!server_cgroup_template)
> +		return ret;
> +
> +	char cgroup_path[MAXPATHLEN];
> +
> +	size_t i, j, escape;
> +	size_t len = strlen(server_cgroup_template);
> +	int fd = -1;
> +
> +	i = j = escape = 0;
> +
> +	for (; i < len; i++) {
> +		if (j > sizeof(cgroup_path)) {
> +			err("path too long");
> +			ret = -1;
> +			goto fail;
> +		}
> +
> +		if (escape) {
> +			ssize_t n = 0;
> +			char *p = cgroup_path + j;
> +			size_t sz = (size_t) (p - cgroup_path);
> +
> +			switch (server_cgroup_template[i]) {
> +				case 'u':
> +					n = snprintf(p, sz, "%s", caller_user);
> +					break;
> +				case 'U':
> +					n = snprintf(p, sz, "%u", caller_uid);
> +					break;
> +				case 'G':
> +					n = snprintf(p, sz, "%u", caller_gid);
> +					break;
> +				case 'N':
> +					n = snprintf(p, sz, "%u", caller_num);
> +					break;
> +				case '%':
> +					n = snprintf(p, sz, "%%");
> +					break;
> +			}
> +
> +			if (n <= 0) {
> +				err("unable to expand escape sequence: %%%c",
> +				    server_cgroup_template[i]);
> +				ret = -1;
> +				goto fail;
> +			}
> +
> +			j += (size_t) n;
> +
> +			escape = 0;
> +			continue;
> +
> +		} else if (server_cgroup_template[i] == '%') {
> +			escape = 1;
> +			continue;
> +
> +		} else if (server_cgroup_template[i] == '/' && j > 0) {
> +			cgroup_path[j] = '\0';
> +
> +			errno = 0;
> +			if (mkdir(cgroup_path, 0755) < 0 && errno != EEXIST) {
> +				err("mkdir: %s: errno=%d: %m", cgroup_path, errno);
> +				ret = -1;
> +				goto fail;
> +			}
> +		}
> +
> +		cgroup_path[j++] = server_cgroup_template[i];
> +	}
> +
> +	cgroup_path[j] = '\0';
> +
> +	if ((fd = open(cgroup_path, O_CREAT | O_WRONLY | O_CLOEXEC, 0644)) < 0) {
> +		err("open: %s: %m", cgroup_path);
> +		ret = -1;
> +		goto fail;
> +	}
> +
> +	if (dprintf(fd, "%d\n", getpid()) < 0) {
> +		err("dprintf: %s: unable to write pid", cgroup_path);
> +		ret = -1;
> +	}
> +fail:
> +	if (fd >= 0 && close(fd) < 0) {
> +		err("close: %s: %m", cgroup_path);
> +		ret = -1;
> +	}
> +
> +	return ret;
> +}
> diff --git a/hasher-priv/config.c b/hasher-priv/config.c
> index 6b6bdb1..3faf936 100644
> --- a/hasher-priv/config.c
> +++ b/hasher-priv/config.c
> @@ -30,6 +30,7 @@ const char *const *chroot_prefix_list;
>  const char *chroot_prefix_path;
>  const char *change_user1, *change_user2;
>  char *server_control_group = NULL;
> +char *server_cgroup_template = NULL;
>  char *server_pidfile = NULL;
>  const char *term;
>  const char *x11_display, *x11_key;
> @@ -671,6 +672,9 @@ set_server_config(const char *name, const char *value, const char *filename)
>  	} else if (!strcasecmp("control_group", name)) {
>  		free(server_control_group);
>  		server_control_group = xstrdup(value);
> +	} else if (!strcasecmp("cgroup_template", name)) {
> +		free(server_cgroup_template);
> +		server_cgroup_template = xstrdup(value);
>  	} else {
>  		bad_option_name(name, filename);
>  	}
> @@ -771,4 +775,5 @@ free_server_configuration(void)
>  {
>  	free(server_pidfile);
>  	free(server_control_group);
> +	free(server_cgroup_template);
>  }
> diff --git a/hasher-priv/priv.h b/hasher-priv/priv.h
> index f0eb9f9..f29603a 100644
> --- a/hasher-priv/priv.h
> +++ b/hasher-priv/priv.h
> @@ -120,6 +120,7 @@ int     do_chrootuid2(void);
>  
>  int process_caller_task(int, struct task *);
>  pid_t fork_server(int, uid_t, gid_t, unsigned);
> +int join_cgroup(void);
>  
>  extern const char *chroot_path;
>  extern const char **chroot_argv;
> @@ -162,6 +163,7 @@ extern work_limit_t wlimit;
>  extern int server_log_priority;
>  extern unsigned long server_session_timeout;
>  extern char *server_control_group;
> +extern char *server_cgroup_template;
>  extern char *server_pidfile;
>  extern gid_t server_gid;
>  
> diff --git a/hasher-priv/server.conf b/hasher-priv/server.conf
> index 53ea5c3..9e70487 100644
> --- a/hasher-priv/server.conf
> +++ b/hasher-priv/server.conf
> @@ -11,3 +11,12 @@ session_timeout=3600
>  
>  # Allow users of this group to interact with hasher-privd via the control socket.
>  control_group=hashman
> +
> +# Template for cgroup path to which task handler should be added.
> +#
> +# %u -- Session's user name.
> +# %U -- Session's user numeric ID.
> +# %G -- Session's group numeric ID.
> +# %N -- Session's user number.
> +#
> +#cgroup_template=/sys/fs/cgroup2/hasher-priv/%u/cgroup.procs
> -- 
> 2.24.0
> 
> _______________________________________________
> Devel mailing list
> Devel@lists.altlinux.org
> https://lists.altlinux.org/mailman/listinfo/devel

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]