[lxc-devel] [PATCH] coverity: rundir: Fix memory leaks

[Serge Hallyn]

Quoting Stéphane Graber (stgraber at ubuntu.com):
> Since we're no longer always returning a getenv result or some defined
> string, the callers should cleanup the variable after use.
> 
> As a result, change from const char* to char*, add the needed free()
> everywhere and use strdup() on strings coming from getenv.
> 
> Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>

Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>

> ---
>  src/lxc/lxclock.c | 11 +++++++++--
>  src/lxc/monitor.c |  6 +++++-
>  src/lxc/utils.c   | 23 +++++++++++++----------
>  src/lxc/utils.h   |  2 +-
>  4 files changed, 28 insertions(+), 14 deletions(-)
> 
> diff --git a/src/lxc/lxclock.c b/src/lxc/lxclock.c
> index 598d6c0..b7a0078 100644
> --- a/src/lxc/lxclock.c
> +++ b/src/lxc/lxclock.c
> @@ -98,7 +98,7 @@ static char *lxclock_name(const char *p, const char *n)
>  	int ret;
>  	int len;
>  	char *dest;
> -	const char *rundir;
> +	char *rundir;
>  
>  	/* lockfile will be:
>  	 * "/run" + "/lock/lxc/$lxcpath/$lxcname + '\0' if root
> @@ -113,12 +113,15 @@ static char *lxclock_name(const char *p, const char *n)
>  		return NULL;
>  	len += strlen(rundir);
>  
> -	if ((dest = malloc(len)) == NULL)
> +	if ((dest = malloc(len)) == NULL) {
> +		free(rundir);
>  		return NULL;
> +	}
>  
>  	ret = snprintf(dest, len, "%s/lock/lxc/%s", rundir, p);
>  	if (ret < 0 || ret >= len) {
>  		free(dest);
> +		free(rundir);
>  		return NULL;
>  	}
>  	ret = mkdir_p(dest, 0755);
> @@ -130,6 +133,7 @@ static char *lxclock_name(const char *p, const char *n)
>  			d = realloc(dest, l2);
>  			if (!d) {
>  				free(dest);
> +				free(rundir);
>  				return NULL;
>  			}
>  			len = l2;
> @@ -138,12 +142,15 @@ static char *lxclock_name(const char *p, const char *n)
>  		ret = snprintf(dest, len, "/tmp/%d/lxc/%s", geteuid(), p);
>  		if (ret < 0 || ret >= len) {
>  			free(dest);
> +			free(rundir);
>  			return NULL;
>  		}
>  		ret = snprintf(dest, len, "/tmp/%d/lxc/%s/%s", geteuid(), p, n);
>  	} else
>  		ret = snprintf(dest, len, "%s/lock/lxc/%s/%s", rundir, p, n);
>  
> +	free(rundir);
> +
>  	if (ret < 0 || ret >= len) {
>  		free(dest);
>  		return NULL;
> diff --git a/src/lxc/monitor.c b/src/lxc/monitor.c
> index 704cc22..e45b5cf 100644
> --- a/src/lxc/monitor.c
> +++ b/src/lxc/monitor.c
> @@ -54,7 +54,7 @@ int lxc_monitor_fifo_name(const char *lxcpath, char *fifo_path, size_t fifo_path
>  			  int do_mkdirp)
>  {
>  	int ret;
> -	const char *rundir;
> +	char *rundir;
>  
>  	rundir = get_rundir();
>  	if (!rundir)
> @@ -64,19 +64,23 @@ int lxc_monitor_fifo_name(const char *lxcpath, char *fifo_path, size_t fifo_path
>  		ret = snprintf(fifo_path, fifo_path_sz, "%s/lxc/%s", rundir, lxcpath);
>  		if (ret < 0 || ret >= fifo_path_sz) {
>  			ERROR("rundir/lxcpath (%s/%s) too long for monitor fifo", rundir, lxcpath);
> +			free(rundir);
>  			return -1;
>  		}
>  		ret = mkdir_p(fifo_path, 0755);
>  		if (ret < 0) {
>  			ERROR("unable to create monitor fifo dir %s", fifo_path);
> +			free(rundir);
>  			return ret;
>  		}
>  	}
>  	ret = snprintf(fifo_path, fifo_path_sz, "%s/lxc/%s/monitor-fifo", rundir, lxcpath);
>  	if (ret < 0 || ret >= fifo_path_sz) {
>  		ERROR("rundir/lxcpath (%s/%s) too long for monitor fifo", rundir, lxcpath);
> +		free(rundir);
>  		return -1;
>  	}
> +	free(rundir);
>  	return 0;
>  }
>  
> diff --git a/src/lxc/utils.c b/src/lxc/utils.c
> index db5cc19..c5c450d 100644
> --- a/src/lxc/utils.c
> +++ b/src/lxc/utils.c
> @@ -376,7 +376,7 @@ out:
>  	return values[i];
>  }
>  
> -const char *get_rundir()
> +char *get_rundir()
>  {
>  	char *rundir;
>  	const char *homedir;
> @@ -387,18 +387,21 @@ const char *get_rundir()
>  	}
>  
>  	rundir = getenv("XDG_RUNTIME_DIR");
> -	if (!rundir) {
> -		INFO("XDG_RUNTIME_DIR isn't set in the environment.");
> -		homedir = getenv("HOME");
> -		if (!homedir) {
> -			ERROR("HOME isn't set in the environment.");
> -			return NULL;
> -		}
> +	if (rundir) {
> +		rundir = strdup(rundir);
> +		return rundir;
> +	}
>  
> -		rundir = malloc(sizeof(char) * (17 + strlen(homedir)));
> -		sprintf(rundir, "%s/.cache/lxc/run/", homedir);
> +	INFO("XDG_RUNTIME_DIR isn't set in the environment.");
> +	homedir = getenv("HOME");
> +	if (!homedir) {
> +		ERROR("HOME isn't set in the environment.");
> +		return NULL;
>  	}
>  
> +	rundir = malloc(sizeof(char) * (17 + strlen(homedir)));
> +	sprintf(rundir, "%s/.cache/lxc/run/", homedir);
> +
>  	return rundir;
>  }
>  
> diff --git a/src/lxc/utils.h b/src/lxc/utils.h
> index f541253..f6f3373 100644
> --- a/src/lxc/utils.h
> +++ b/src/lxc/utils.h
> @@ -39,7 +39,7 @@ extern void lxc_setup_fs(void);
>  extern int get_u16(unsigned short *val, const char *arg, int base);
>  extern int mkdir_p(const char *dir, mode_t mode);
>  extern void remove_trailing_slashes(char *p);
> -extern const char *get_rundir(void);
> +extern char *get_rundir(void);
>  
>  extern const char *lxc_global_config_value(const char *option_name);
>  
> -- 
> 1.9.rc1
> 
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel