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

[Alexey Gladkov <legion@altlinux.ru>]

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

On Thu, Sep 17, 2020 at 04:11:07PM +0300, Arseny Maslennikov wrote:
> 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.

I think this patch is simple enough.

> 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

I'm pretty sure the hasher-priv shouldn't be tied to systemd. I'm also
convinced that the server will not be tied. The hasher-privd must be able
to run on systems without systemd.

> 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.

Yeah, I didn't plan on supporting legacy version of cgroups. Docker
already can work with cgroupsv2.

> 
> > 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



-- 
Rgrds, legion


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