PERFORCE change 107568 for review

[Todd Miller]

http://perforce.freebsd.org/chv.cgi?CH=107568

Change 107568 by millert at millert_g5tower on 2006/10/09 19:34:16

	Clean up buffer freeing by doing it once at the end of
	process_line().  Use memory more efficiently by only
	allocating as much as we need for each regex/type/context
	tuple instead of 3 * length(buffer).  While here, use strok()
	instead of sscanf() since it is OK to modify to line_buf.

Affected files ...

.. //depot/projects/trustedbsd/sedarwin8/policies/sedarwin/libselinux/src/matchpathcon.c#3 edit

Differences ...

==== //depot/projects/trustedbsd/sedarwin8/policies/sedarwin/libselinux/src/matchpathcon.c#3 (text+ko) ====

@@ -48,7 +48,6 @@
 
 static int default_canoncon(const char *path, unsigned lineno, char **context)
 {
-#ifdef notyet
 	char *tmpcon;
 	if (security_canonicalize_context_raw(*context, &tmpcon) < 0) {
 		if (errno == ENOENT)
@@ -62,7 +61,6 @@
 	}
 	free(*context);
 	*context = tmpcon;
-#endif
 	return 0;
 }
 
@@ -444,11 +442,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;
@@ -458,55 +458,35 @@
 	/* Skip comment lines and empty lines. */
 	if (*buf_p == '#' || *buf_p == 0)
 		return 0;
-	/* XXXSEBSD
-           Allocate space for regex, type, and context. We do this only to
-	   minimize diffs with stock SELinux code which uses %as in the 
-           sscanf() format string rather than using something reasonable like
-           strtok() or strsep(). We additionally just assume that no substring
-           of line_buf can be longer than line_buf itself for this allocation.
-	 */
-	regex = (char *)malloc(strlen(line_buf) + 1);
-	if (regex == NULL)
-	 return(-1); 
 
-	type = (char *)malloc(strlen(line_buf) + 1);
-	if (type == NULL)
-	{
-	  free(regex);
-	  return(-1);
-	}
-
-	context = (char *)malloc(strlen(line_buf) + 1);
-	if (context == NULL)
-	{
-	  free(regex);
-	  free(type);
-	  return(-1);
-	}
-	
-	items = sscanf(line_buf, "%s %s %s", regex, type, context);
+	regex = strtok(buf_p, " \t");
+	type = strtok(NULL, " \t");
+	context = strtok(NULL, " \t");
+	items = !!regex + !!type + !!context;
 	if (items < 2) {
 		myprintf("%s:  line %d is missing fields, skipping\n", path,
 			 lineno);
-		free(regex);
-		free(type);
-		free(context);
 		return 0;
 	} else if (items == 2) {
 		/* The type field is optional. */
-		free(context);
 		context = type;
-		type = 0;
+		type = NULL;
+	}
+
+	regex = strdup(regex);
+	if (type != NULL)
+		type = strdup(type);
+	context = strdup(context);
+	if (!!regex + !!type + !!context != items) {
+		ret = -1;
+		goto finish;
 	}
 
 	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) {
@@ -519,10 +499,8 @@
 		len = strlen(reg_buf);
 		cp = anchored_regex = malloc(len + 3);
 		if (!anchored_regex) {
-			free(regex);
-			free(type);
-			free(context);
-			return -1;
+			ret = -1;
+			goto finish;
 		}
 		/* Create ^...$ regexp.  */
 		*cp++ = '^';
@@ -550,10 +528,7 @@
 				 path, lineno, anchored_regex,
 				 (errbuf ? errbuf : "out of memory"));
 			free(anchored_regex);
-			free(regex);
-			free(type);
-			free(context);
-			return 0;
+			goto finish;
 		}
 		free(anchored_regex);
 
@@ -566,10 +541,7 @@
 		if (type[0] != '-' || len != 2) {
 			myprintf("%s:  line %d has invalid file type %s\n",
 				 path, lineno, type);
-			free(regex);
-			free(type);
-			free(context);
-			return 0;
+			goto finish;
 		}
 		switch (type[1]) {
 		case 'b':
@@ -596,10 +568,7 @@
 		default:
 			myprintf("%s:  line %d has invalid file type %s\n",
 				 path, lineno, type);
-			free(regex);
-			free(type);
-			free(context);
-			return 0;
+			goto finish;
 		}
 
 	      skip_type:
@@ -607,20 +576,12 @@
 			if (myflags & MATCHPATHCON_VALIDATE) {
 				if (myinvalidcon) {
 					/* Old-style validation of context. */
-					if (myinvalidcon(path, lineno, context)) {
-						free(regex);
-						free(type);
-						free(context);
-						return 0;
-					}
+					if (myinvalidcon(path, lineno, context))
+						goto finish;
 				} else {
 					/* New canonicalization of context. */
-					if (mycanoncon(path, lineno, &context)) {
-						free(regex);
-						free(type);
-						free(context);
-						return 0;
-					}
+					if (mycanoncon(path, lineno, &context))
+						goto finish;
 				}
 				spec_arr[nspec].context_valid = 1;
 			}
@@ -631,16 +592,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)