Re: PATCH: libselinux matchpathcon() memory leak

[Karl MacMillan <kmacmillan@mentalrootkit.com>]

Todd C. Miller wrote:
> This is a patch I sent in last year but forgot to split up as
> requested; it still applies to the recently-released selinux-1.34.0.
> 
> This patch adds a finish: label and ret variable that holds the
> function return value.  Instead of returning early we just goto
> finish and let it clean things up as needed.  This does assume that
> free(NULL) is valid but that as been the case since C89.
> 
>  - todd
> 

Acked-by: Karl MacMillan <kmacmillan@mentalrootkit.com> and merged into 
trunk and stable.

> --- matchpathcon.c	2006-06-29 14:21:04.000000000 -0400
> +++ matchpathcon.c	2007-01-25 14:19:39.000000000 -0500
> @@ -443,11 +443,13 @@
>  static int process_line(const char *path, const char *prefix, char *line_buf,
>  			int pass, unsigned lineno)
>  {
> -	int items, len, regerr;
> +	int items, len, regerr, ret;
>  	char *buf_p;
>  	char *regex, *type, *context;
>  	const char *reg_buf;
>  	char *anchored_regex;
> +
> +	ret = 0;
>  	len = strlen(line_buf);
>  	if (line_buf[len - 1] == '\n')
>  		line_buf[len - 1] = 0;
> @@ -464,19 +466,15 @@
>  		return 0;
>  	} else if (items == 2) {
>  		/* The type field is optional. */
> -		free(context);
>  		context = type;
> -		type = 0;
> +		type = NULL;
>  	}
>  
>  	reg_buf = regex;
>  	len = get_stem_from_spec(reg_buf);
>  	if (len && prefix && strncmp(prefix, regex, len)) {
>  		/* Stem of regex does not match requested prefix, discard. */
> -		free(regex);
> -		free(type);
> -		free(context);
> -		return 0;
> +		goto finish;
>  	}
>  
>  	if (pass == 1) {
> @@ -488,8 +486,10 @@
>  		/* Anchor the regular expression. */
>  		len = strlen(reg_buf);
>  		cp = anchored_regex = malloc(len + 3);
> -		if (!anchored_regex)
> -			return -1;
> +		if (!anchored_regex) {
> +			ret = -1;
> +			goto finish;
> +		}
>  		/* Create ^...$ regexp.  */
>  		*cp++ = '^';
>  		cp = mempcpy(cp, reg_buf, len);
> @@ -515,7 +515,7 @@
>  				 path, lineno, anchored_regex,
>  				 (errbuf ? errbuf : "out of memory"));
>  			free(anchored_regex);
> -			return 0;
> +			goto finish;
>  		}
>  		free(anchored_regex);
>  
> @@ -528,7 +528,7 @@
>  		if (type[0] != '-' || len != 2) {
>  			myprintf("%s:  line %d has invalid file type %s\n",
>  				 path, lineno, type);
> -			return 0;
> +			goto finish;
>  		}
>  		switch (type[1]) {
>  		case 'b':
> @@ -555,7 +555,7 @@
>  		default:
>  			myprintf("%s:  line %d has invalid file type %s\n",
>  				 path, lineno, type);
> -			return 0;
> +			goto finish;
>  		}
>  
>  	      skip_type:
> @@ -564,11 +564,11 @@
>  				if (myinvalidcon) {
>  					/* Old-style validation of context. */
>  					if (myinvalidcon(path, lineno, context))
> -						return 0;
> +						goto finish;
>  				} else {
>  					/* New canonicalization of context. */
>  					if (mycanoncon(path, lineno, &context))
> -						return 0;
> +						goto finish;
>  				}
>  				spec_arr[nspec].context_valid = 1;
>  			}
> @@ -579,16 +579,19 @@
>  		/* Determine if specification has 
>  		 * any meta characters in the RE */
>  		spec_hasMetaChars(&spec_arr[nspec]);
> +
> +		/* Prevent stored strings from being freed. */
> +		regex = NULL;
> +		type = NULL;
> +		context = NULL;
>  	}
>  
>  	nspec++;
> -	if (pass == 0) {
> -		free(regex);
> -		if (type)
> -			free(type);
> -		free(context);
> -	}
> -	return 0;
> +finish:
> +	free(regex);
> +	free(type);
> +	free(context);
> +	return ret;
>  }
>  
>  int matchpathcon_init_prefix(const char *path, const char *prefix)
> 
> --
> This message was distributed to subscribers of the selinux mailing list.
> If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
> the words "unsubscribe selinux" without quotes as the message.


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.