[lxc-devel] [PATCH] get rid of fancy proctitles

Tycho Andersen tycho.andersen at canonical.com
Mon Nov 16 21:38:13 UTC 2015


On Mon, Nov 16, 2015 at 09:00:24PM +0000, Serge Hallyn wrote:
> Hey Tycho,
> 
> I'm not sure what the best way to handle this is, but I dont' like
> having an ephemeral pastebin link in the changelog.  Can you just stick
> the test program in the commit msg?

Actually I think we can drop this one for now, there may be a better
way.

Thanks,

Tycho

> Quoting Tycho Andersen (tycho.andersen at canonical.com):
> > It turns out clobbering the environ kernel stack does actually affect the
> > kernel environment (viz [1]), regardless of what previous behavior or man
> > pages said. For the 1.1 series, we should simply not try to clobber
> > proctitles.
> > 
> > [1]: http://paste.ubuntu.com/13300018/
> > 
> > Signed-off-by: Tycho Andersen <tycho.andersen at canonical.com>
> > ---
> >  src/lxc/criu.c         |   9 ----
> >  src/lxc/lxccontainer.c |   9 ----
> >  src/lxc/utils.c        | 123 -------------------------------------------------
> >  src/lxc/utils.h        |   1 -
> >  4 files changed, 142 deletions(-)
> > 
> > diff --git a/src/lxc/criu.c b/src/lxc/criu.c
> > index 7ee6cbe..a76f02d 100644
> > --- a/src/lxc/criu.c
> > +++ b/src/lxc/criu.c
> > @@ -465,7 +465,6 @@ void do_restore(struct lxc_container *c, int pipe, char *directory, bool verbose
> >  		goto out_fini_handler;
> >  	} else {
> >  		int ret;
> > -		char title[2048];
> >  
> >  		pid_t w = waitpid(pid, &status, 0);
> >  		if (w == -1) {
> > @@ -511,14 +510,6 @@ void do_restore(struct lxc_container *c, int pipe, char *directory, bool verbose
> >  			goto out_fini_handler;
> >  		}
> >  
> > -		/*
> > -		 * See comment in lxcapi_start; we don't care if these
> > -		 * fail because it's just a beauty thing. We just
> > -		 * assign the return here to silence potential.
> > -		 */
> > -		ret = snprintf(title, sizeof(title), "[lxc monitor] %s %s", c->config_path, c->name);
> > -		ret = setproctitle(title);
> > -
> >  		ret = lxc_poll(c->name, handler);
> >  		if (ret)
> >  			lxc_abort(c->name, handler);
> > diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> > index 11d1822..503925e 100644
> > --- a/src/lxc/lxccontainer.c
> > +++ b/src/lxc/lxccontainer.c
> > @@ -689,7 +689,6 @@ static bool do_lxcapi_start(struct lxc_container *c, int useinit, char * const a
> >  	* while container is running...
> >  	*/
> >  	if (daemonize) {
> > -		char title[2048];
> >  		lxc_monitord_spawn(c->config_path);
> >  
> >  		pid_t pid = fork();
> > @@ -704,14 +703,6 @@ static bool do_lxcapi_start(struct lxc_container *c, int useinit, char * const a
> >  			return wait_on_daemonized_start(c, pid);
> >  		}
> >  
> > -		/* We don't really care if this doesn't print all the
> > -		 * characters; all that it means is that the proctitle will be
> > -		 * ugly. Similarly, we also don't care if setproctitle()
> > -		 * fails. */
> > -		snprintf(title, sizeof(title), "[lxc monitor] %s %s", c->config_path, c->name);
> > -		INFO("Attempting to set proc title to %s", title);
> > -		setproctitle(title);
> > -
> >  		/* second fork to be reparented by init */
> >  		pid = fork();
> >  		if (pid < 0) {
> > diff --git a/src/lxc/utils.c b/src/lxc/utils.c
> > index d9e769d..3f83865 100644
> > --- a/src/lxc/utils.c
> > +++ b/src/lxc/utils.c
> > @@ -1341,129 +1341,6 @@ char *get_template_path(const char *t)
> >  }
> >  
> >  /*
> > - * Sets the process title to the specified title. Note:
> > - *   1. this function requires root to succeed
> > - *   2. it clears /proc/self/environ
> > - *   3. it may not succed (e.g. if title is longer than /proc/self/environ +
> > - *      the original title)
> > - */
> > -int setproctitle(char *title)
> > -{
> > -	char buf[2048], *tmp;
> > -	FILE *f;
> > -	int i, len, ret = 0;
> > -
> > -	/* We don't really need to know all of this stuff, but unfortunately
> > -	 * PR_SET_MM_MAP requires us to set it all at once, so we have to
> > -	 * figure it out anyway.
> > -	 */
> > -	unsigned long start_data, end_data, start_brk, start_code, end_code,
> > -			start_stack, arg_start, arg_end, env_start, env_end,
> > -			brk_val;
> > -	struct prctl_mm_map prctl_map;
> > -
> > -	f = fopen_cloexec("/proc/self/stat", "r");
> > -	if (!f) {
> > -		return -1;
> > -	}
> > -
> > -	tmp = fgets(buf, sizeof(buf), f);
> > -	fclose(f);
> > -	if (!tmp) {
> > -		return -1;
> > -	}
> > -
> > -	/* Skip the first 25 fields, column 26-28 are start_code, end_code,
> > -	 * and start_stack */
> > -	tmp = strchr(buf, ' ');
> > -	for (i = 0; i < 24; i++) {
> > -		if (!tmp)
> > -			return -1;
> > -		tmp = strchr(tmp+1, ' ');
> > -	}
> > -	if (!tmp)
> > -		return -1;
> > -
> > -	i = sscanf(tmp, "%lu %lu %lu", &start_code, &end_code, &start_stack);
> > -	if (i != 3)
> > -		return -1;
> > -
> > -	/* Skip the next 19 fields, column 45-51 are start_data to arg_end */
> > -	for (i = 0; i < 19; i++) {
> > -		if (!tmp)
> > -			return -1;
> > -		tmp = strchr(tmp+1, ' ');
> > -	}
> > -
> > -	if (!tmp)
> > -		return -1;
> > -
> > -	i = sscanf(tmp, "%lu %lu %lu %lu %lu %lu %lu",
> > -		&start_data,
> > -		&end_data,
> > -		&start_brk,
> > -		&arg_start,
> > -		&arg_end,
> > -		&env_start,
> > -		&env_end);
> > -	if (i != 7)
> > -		return -1;
> > -
> > -	/* Include the null byte here, because in the calculations below we
> > -	 * want to have room for it. */
> > -	len = strlen(title) + 1;
> > -
> > -	/* We're truncating the environment, so we should use at most the
> > -	 * length of the argument + environment for the title. */
> > -	if (len > env_end - arg_start) {
> > -		arg_end = env_end;
> > -		len = env_end - arg_start;
> > -		title[len-1] = '\0';
> > -	} else {
> > -		/* Only truncate the environment if we're actually going to
> > -		 * overwrite part of it. */
> > -		if (len >= arg_end - arg_start) {
> > -			env_start = env_end;
> > -		}
> > -
> > -		arg_end = arg_start + len;
> > -
> > -		/* check overflow */
> > -		if (arg_end < len || arg_end < arg_start) {
> > -			return -1;
> > -		}
> > -
> > -	}
> > -
> > -	brk_val = syscall(__NR_brk, 0);
> > -
> > -	prctl_map = (struct prctl_mm_map) {
> > -		.start_code = start_code,
> > -		.end_code = end_code,
> > -		.start_stack = start_stack,
> > -		.start_data = start_data,
> > -		.end_data = end_data,
> > -		.start_brk = start_brk,
> > -		.brk = brk_val,
> > -		.arg_start = arg_start,
> > -		.arg_end = arg_end,
> > -		.env_start = env_start,
> > -		.env_end = env_end,
> > -		.auxv = NULL,
> > -		.auxv_size = 0,
> > -		.exe_fd = -1,
> > -	};
> > -
> > -	ret = prctl(PR_SET_MM, PR_SET_MM_MAP, (long) &prctl_map, sizeof(prctl_map), 0);
> > -	if (ret == 0)
> > -		strcpy((char*)arg_start, title);
> > -	else
> > -		SYSERROR("setting cmdline failed");
> > -
> > -	return ret;
> > -}
> > -
> > -/*
> >   * @path:    a pathname where / replaced with '\0'.
> >   * @offsetp: pointer to int showing which path segment was last seen.
> >   *           Updated on return to reflect the next segment.
> > diff --git a/src/lxc/utils.h b/src/lxc/utils.h
> > index 059026f..5cd58da 100644
> > --- a/src/lxc/utils.h
> > +++ b/src/lxc/utils.h
> > @@ -278,7 +278,6 @@ int print_to_file(const char *file, const char *content);
> >  bool switch_to_ns(pid_t pid, const char *ns);
> >  int is_dir(const char *path);
> >  char *get_template_path(const char *t);
> > -int setproctitle(char *title);
> >  int safe_mount(const char *src, const char *dest, const char *fstype,
> >  		unsigned long flags, const void *data, const char *rootfs);
> >  int mount_proc_if_needed(const char *rootfs);
> > -- 
> > 2.5.0
> > 
> > _______________________________________________
> > 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


More information about the lxc-devel mailing list