matchpathcon_init overhead

matchpathcon_init overhead

[Stephen Smalley]

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

Hi,

As has been pointed out recently on fedora-selinux-list and
fedora-devel-list, it seems that udev and install are much slower now
than they were previously, and the culprit seems to be the fact that
matchpathcon/matchpathcon_init process the entire file_contexts
configuration rather than just relevant entries, and these particular
programs are bearing that cost for every file (unlike
setfiles/restorecon, where we process it once and then apply the result
to potentially many files).  The cost has increased recently due to the
context canonicalization, although there was some cost there already for
context validation.  We may be able to optimize that somewhat, but it
seems like the real problem is selecting a subset of file_contexts to
process.

The patch below introduces a matchpathcon_init_prefix() function that
allows a caller to optionally specify one or both of the path to the
file contexts configuration (already an argument to matchpathcon_init,
can be left NULL to use the default as before) and a prefix to use in
selecting which entries to load from that configuration.  Given a
non-NULL prefix, it will then only load entries whose regex either has
no identifiable stem or that have a stem that is a prefix of the
"prefix".  Thus, udev can call it with a prefix of "/dev" to exclude
most non-/dev entries, and install can just call it with the destination
path.   Since it includes prefixes of the prefix, we still get fallback
matching on entries like /.* to provide higher level defaults.  We end
up with some entries that aren't actually needed still due to certain
regexes that have no identifiable stem, e.g. /opt(/.*)?, since the stem
logic looks for a terminal / prior to regex characters, but it still
trims the number of entries considerably.

udev and install would then need to be modified to call
matchpathcon_init_prefix() once at startup prior to calling
matchpathcon() as usual.  

Does this seem reasonable?
  
-- 
Stephen Smalley
National Security Agency

[-- Attachment #2: matchpathcon.diff --]
[-- Type: text/x-patch, Size: 6381 bytes --]

Index: libselinux/include/selinux/selinux.h
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libselinux/include/selinux/selinux.h,v
retrieving revision 1.51
diff -u -p -r1.51 selinux.h
--- libselinux/include/selinux/selinux.h	8 Nov 2005 19:26:33 -0000	1.51
+++ libselinux/include/selinux/selinux.h	30 Nov 2005 20:28:14 -0000
@@ -305,6 +305,10 @@ extern void set_matchpathcon_flags(unsig
    from them if present. */
 extern int matchpathcon_init(const char *path);
 
+/* Same as matchpathcon_init, but only load entries with
+   regexes that have stems that are prefixes of 'prefix'. */
+extern int matchpathcon_init_prefix(const char *path, const char *prefix);
+
 /* Match the specified pathname and mode against the file contexts
    configuration and set *con to refer to the resulting context.
    'mode' can be 0 to disable mode matching.
Index: libselinux/src/matchpathcon.c
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libselinux/src/matchpathcon.c,v
retrieving revision 1.34
diff -u -p -r1.34 matchpathcon.c
--- libselinux/src/matchpathcon.c	29 Nov 2005 16:51:08 -0000	1.34
+++ libselinux/src/matchpathcon.c	30 Nov 2005 20:25:58 -0000
@@ -463,10 +463,11 @@ static void spec_hasMetaChars(struct spe
 	}
 	return;
 }
-static int process_line( const char *path, char *line_buf, int pass, unsigned lineno, int mls_enabled) {
+static int process_line( const char *path, const char *prefix, char *line_buf, int pass, unsigned lineno, int mls_enabled) {
 	int items, len, regerr;
 	char *buf_p;
 	char *regex, *type, *context;
+	const char *reg_buf;
 	char *anchored_regex;
 	len = strlen(line_buf);
 	if (line_buf[len - 1] == '\n')
@@ -489,10 +490,19 @@ static int process_line( const char *pat
 		context = type;
 		type = 0;
 	}
-	
+
+	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;
+	}
+
 	if (pass == 1) {
 		/* On the second pass, compile and store the specification in spec. */
-		const char *reg_buf = regex;
 		char *cp;
 		spec_arr[nspec].stem_id = find_stem_from_spec(&reg_buf);
 		spec_arr[nspec].regex_str = regex;
@@ -617,7 +627,7 @@ skip_trans:
 	return 0;
 }
 
-int matchpathcon_init(const char *path)
+int matchpathcon_init_prefix(const char *path, const char *prefix)
 {
 	FILE *fp;
 	FILE *localfp = NULL;
@@ -663,20 +673,20 @@ int matchpathcon_init(const char *path)
 		lineno = 0;
 		nspec = 0;
 		while (getline(&line_buf, &line_len, fp) > 0 && nspec < maxnspec) {
-			if (process_line(path, line_buf, pass, ++lineno, mls_enabled) != 0)
+			if (process_line(path, prefix, line_buf, pass, ++lineno, mls_enabled) != 0)
 				goto finish;
 		}
 		lineno = 0;
 		if (homedirfp) 
 			while (getline(&line_buf, &line_len, homedirfp) > 0 && nspec < maxnspec) {
-				if (process_line(homedir_path, line_buf, pass, ++lineno, mls_enabled) != 0)
+				if (process_line(homedir_path, prefix, line_buf, pass, ++lineno, mls_enabled) != 0)
 					goto finish;
 			}
 
 		lineno = 0;
 		if (localfp) 
 			while (getline(&line_buf, &line_len, localfp) > 0 && nspec < maxnspec) {
-				if (process_line(local_path, line_buf, pass, ++lineno, mls_enabled) != 0)
+				if (process_line(local_path, prefix, line_buf, pass, ++lineno, mls_enabled) != 0)
 					goto finish;
 			}
 
@@ -723,7 +733,12 @@ int matchpathcon_init(const char *path)
 	if (localfp) fclose(localfp);
 	return status;
 }
-hidden_def(matchpathcon_init)
+hidden_def(matchpathcon_init_prefix)
+
+int matchpathcon_init(const char *path)
+{
+	return matchpathcon_init_prefix(path, NULL);
+}
 
 static int matchpathcon_common(const char *name, 
 			       mode_t mode)
@@ -732,7 +747,7 @@ static int matchpathcon_common(const cha
 	const char *buf = name;
 
 	if (!nspec) {
-		ret = matchpathcon_init(NULL);
+		ret = matchpathcon_init_prefix(NULL, NULL);
 		if (ret < 0)
 			return ret;
 		if (!nspec) {
Index: libselinux/src/selinux_internal.h
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libselinux/src/selinux_internal.h,v
retrieving revision 1.16
diff -u -p -r1.16 selinux_internal.h
--- libselinux/src/selinux_internal.h	7 Nov 2005 19:30:37 -0000	1.16
+++ libselinux/src/selinux_internal.h	30 Nov 2005 20:27:05 -0000
@@ -59,7 +59,7 @@ hidden_proto(selinux_customizable_types_
 hidden_proto(selinux_media_context_path)
 hidden_proto(selinux_path)
 hidden_proto(selinux_check_passwd_access)
-hidden_proto(matchpathcon_init)
+hidden_proto(matchpathcon_init_prefix)
 hidden_proto(selinux_users_path)
 hidden_proto(selinux_usersconf_path);
 hidden_proto(selinux_translations_path);
Index: libselinux/utils/matchpathcon.c
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libselinux/utils/matchpathcon.c,v
retrieving revision 1.5
diff -u -p -r1.5 matchpathcon.c
--- libselinux/utils/matchpathcon.c	29 Nov 2005 16:50:15 -0000	1.5
+++ libselinux/utils/matchpathcon.c	30 Nov 2005 20:31:40 -0000
@@ -8,30 +8,47 @@
 
 void usage(const char *progname) 
 {
-	fprintf(stderr, "usage:  %s [-n] [-f file_contexts] path...\n", progname);
+	fprintf(stderr, "usage:  %s [-n] [-f file_contexts] [-p prefix] path...\n", progname);
 	exit(1);
 }
 
 int main(int argc, char **argv) 
 {
 	char *buf;
-	int rc, i;
+	int rc, i, init = 0;
 	int header=1, opt;
 
 	if (argc < 2) usage(argv[0]);
 
-	while ((opt = getopt(argc, argv, "nf:")) > 0) {
+	while ((opt = getopt(argc, argv, "nf:p:")) > 0) {
 		switch (opt) {
 		case 'n':
 			header=0;
 			break;
 		case 'f':
+			if (init) {
+				fprintf(stderr, "%s:  -f and -p are exclusive\n", argv[0]);
+				exit(1);
+			}
+			init = 1;
 			if (matchpathcon_init(optarg)) {
 				fprintf(stderr, "Error while processing %s:  %s\n",
 					optarg, errno ? strerror(errno) : "invalid");
 				exit(1);
 			}
 			break;
+		case 'p':
+			if (init) {
+				fprintf(stderr, "%s:  -f and -p are exclusive\n", argv[0]);
+				exit(1);
+			}
+			init = 1;
+			if (matchpathcon_init_prefix(NULL, optarg)) {
+				fprintf(stderr, "Error while processing %s:  %s\n",
+					optarg, errno ? strerror(errno) : "invalid");
+				exit(1);
+			}
+			break;
 		default:
 			usage(argv[0]);
 		}

Re: matchpathcon_init overhead

[Daniel J Walsh]

Stephen Smalley wrote:
> Hi,
>
> As has been pointed out recently on fedora-selinux-list and
> fedora-devel-list, it seems that udev and install are much slower now
> than they were previously, and the culprit seems to be the fact that
> matchpathcon/matchpathcon_init process the entire file_contexts
> configuration rather than just relevant entries, and these particular
> programs are bearing that cost for every file (unlike
> setfiles/restorecon, where we process it once and then apply the result
> to potentially many files).  The cost has increased recently due to the
> context canonicalization, although there was some cost there already for
> context validation.  We may be able to optimize that somewhat, but it
> seems like the real problem is selecting a subset of file_contexts to
> process.
>
> The patch below introduces a matchpathcon_init_prefix() function that
> allows a caller to optionally specify one or both of the path to the
> file contexts configuration (already an argument to matchpathcon_init,
> can be left NULL to use the default as before) and a prefix to use in
> selecting which entries to load from that configuration.  Given a
> non-NULL prefix, it will then only load entries whose regex either has
> no identifiable stem or that have a stem that is a prefix of the
> "prefix".  Thus, udev can call it with a prefix of "/dev" to exclude
> most non-/dev entries, and install can just call it with the destination
> path.   Since it includes prefixes of the prefix, we still get fallback
> matching on entries like /.* to provide higher level defaults.  We end
> up with some entries that aren't actually needed still due to certain
> regexes that have no identifiable stem, e.g. /opt(/.*)?, since the stem
> logic looks for a terminal / prior to regex characters, but it still
> trims the number of entries considerably.
>
> udev and install would then need to be modified to call
> matchpathcon_init_prefix() once at startup prior to calling
> matchpathcon() as usual.  
>
> Does this seem reasonable?
>   
>   
> ------------------------------------------------------------------------
>
> Index: libselinux/include/selinux/selinux.h
> ===================================================================
> RCS file: /nfshome/pal/CVS/selinux-usr/libselinux/include/selinux/selinux.h,v
> retrieving revision 1.51
> diff -u -p -r1.51 selinux.h
> --- libselinux/include/selinux/selinux.h	8 Nov 2005 19:26:33 -0000	1.51
> +++ libselinux/include/selinux/selinux.h	30 Nov 2005 20:28:14 -0000
> @@ -305,6 +305,10 @@ extern void set_matchpathcon_flags(unsig
>     from them if present. */
>  extern int matchpathcon_init(const char *path);
>  
> +/* Same as matchpathcon_init, but only load entries with
> +   regexes that have stems that are prefixes of 'prefix'. */
> +extern int matchpathcon_init_prefix(const char *path, const char *prefix);
> +
>  /* Match the specified pathname and mode against the file contexts
>     configuration and set *con to refer to the resulting context.
>     'mode' can be 0 to disable mode matching.
> Index: libselinux/src/matchpathcon.c
> ===================================================================
> RCS file: /nfshome/pal/CVS/selinux-usr/libselinux/src/matchpathcon.c,v
> retrieving revision 1.34
> diff -u -p -r1.34 matchpathcon.c
> --- libselinux/src/matchpathcon.c	29 Nov 2005 16:51:08 -0000	1.34
> +++ libselinux/src/matchpathcon.c	30 Nov 2005 20:25:58 -0000
> @@ -463,10 +463,11 @@ static void spec_hasMetaChars(struct spe
>  	}
>  	return;
>  }
> -static int process_line( const char *path, char *line_buf, int pass, unsigned lineno, int mls_enabled) {
> +static int process_line( const char *path, const char *prefix, char *line_buf, int pass, unsigned lineno, int mls_enabled) {
>  	int items, len, regerr;
>  	char *buf_p;
>  	char *regex, *type, *context;
> +	const char *reg_buf;
>  	char *anchored_regex;
>  	len = strlen(line_buf);
>  	if (line_buf[len - 1] == '\n')
> @@ -489,10 +490,19 @@ static int process_line( const char *pat
>  		context = type;
>  		type = 0;
>  	}
> -	
> +
> +	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;
> +	}
> +
>  	if (pass == 1) {
>  		/* On the second pass, compile and store the specification in spec. */
> -		const char *reg_buf = regex;
>  		char *cp;
>  		spec_arr[nspec].stem_id = find_stem_from_spec(&reg_buf);
>  		spec_arr[nspec].regex_str = regex;
> @@ -617,7 +627,7 @@ skip_trans:
>  	return 0;
>  }
>  
> -int matchpathcon_init(const char *path)
> +int matchpathcon_init_prefix(const char *path, const char *prefix)
>  {
>  	FILE *fp;
>  	FILE *localfp = NULL;
> @@ -663,20 +673,20 @@ int matchpathcon_init(const char *path)
>  		lineno = 0;
>  		nspec = 0;
>  		while (getline(&line_buf, &line_len, fp) > 0 && nspec < maxnspec) {
> -			if (process_line(path, line_buf, pass, ++lineno, mls_enabled) != 0)
> +			if (process_line(path, prefix, line_buf, pass, ++lineno, mls_enabled) != 0)
>  				goto finish;
>  		}
>  		lineno = 0;
>  		if (homedirfp) 
>  			while (getline(&line_buf, &line_len, homedirfp) > 0 && nspec < maxnspec) {
> -				if (process_line(homedir_path, line_buf, pass, ++lineno, mls_enabled) != 0)
> +				if (process_line(homedir_path, prefix, line_buf, pass, ++lineno, mls_enabled) != 0)
>  					goto finish;
>  			}
>  
>  		lineno = 0;
>  		if (localfp) 
>  			while (getline(&line_buf, &line_len, localfp) > 0 && nspec < maxnspec) {
> -				if (process_line(local_path, line_buf, pass, ++lineno, mls_enabled) != 0)
> +				if (process_line(local_path, prefix, line_buf, pass, ++lineno, mls_enabled) != 0)
>  					goto finish;
>  			}
>  
> @@ -723,7 +733,12 @@ int matchpathcon_init(const char *path)
>  	if (localfp) fclose(localfp);
>  	return status;
>  }
> -hidden_def(matchpathcon_init)
> +hidden_def(matchpathcon_init_prefix)
> +
> +int matchpathcon_init(const char *path)
> +{
> +	return matchpathcon_init_prefix(path, NULL);
> +}
>  
>  static int matchpathcon_common(const char *name, 
>  			       mode_t mode)
> @@ -732,7 +747,7 @@ static int matchpathcon_common(const cha
>  	const char *buf = name;
>  
>  	if (!nspec) {
> -		ret = matchpathcon_init(NULL);
> +		ret = matchpathcon_init_prefix(NULL, NULL);
>  		if (ret < 0)
>  			return ret;
>  		if (!nspec) {
> Index: libselinux/src/selinux_internal.h
> ===================================================================
> RCS file: /nfshome/pal/CVS/selinux-usr/libselinux/src/selinux_internal.h,v
> retrieving revision 1.16
> diff -u -p -r1.16 selinux_internal.h
> --- libselinux/src/selinux_internal.h	7 Nov 2005 19:30:37 -0000	1.16
> +++ libselinux/src/selinux_internal.h	30 Nov 2005 20:27:05 -0000
> @@ -59,7 +59,7 @@ hidden_proto(selinux_customizable_types_
>  hidden_proto(selinux_media_context_path)
>  hidden_proto(selinux_path)
>  hidden_proto(selinux_check_passwd_access)
> -hidden_proto(matchpathcon_init)
> +hidden_proto(matchpathcon_init_prefix)
>  hidden_proto(selinux_users_path)
>  hidden_proto(selinux_usersconf_path);
>  hidden_proto(selinux_translations_path);
> Index: libselinux/utils/matchpathcon.c
> ===================================================================
> RCS file: /nfshome/pal/CVS/selinux-usr/libselinux/utils/matchpathcon.c,v
> retrieving revision 1.5
> diff -u -p -r1.5 matchpathcon.c
> --- libselinux/utils/matchpathcon.c	29 Nov 2005 16:50:15 -0000	1.5
> +++ libselinux/utils/matchpathcon.c	30 Nov 2005 20:31:40 -0000
> @@ -8,30 +8,47 @@
>  
>  void usage(const char *progname) 
>  {
> -	fprintf(stderr, "usage:  %s [-n] [-f file_contexts] path...\n", progname);
> +	fprintf(stderr, "usage:  %s [-n] [-f file_contexts] [-p prefix] path...\n", progname);
>  	exit(1);
>  }
>  
>  int main(int argc, char **argv) 
>  {
>  	char *buf;
> -	int rc, i;
> +	int rc, i, init = 0;
>  	int header=1, opt;
>  
>  	if (argc < 2) usage(argv[0]);
>  
> -	while ((opt = getopt(argc, argv, "nf:")) > 0) {
> +	while ((opt = getopt(argc, argv, "nf:p:")) > 0) {
>  		switch (opt) {
>  		case 'n':
>  			header=0;
>  			break;
>  		case 'f':
> +			if (init) {
> +				fprintf(stderr, "%s:  -f and -p are exclusive\n", argv[0]);
> +				exit(1);
> +			}
> +			init = 1;
>  			if (matchpathcon_init(optarg)) {
>  				fprintf(stderr, "Error while processing %s:  %s\n",
>  					optarg, errno ? strerror(errno) : "invalid");
>  				exit(1);
>  			}
>  			break;
> +		case 'p':
> +			if (init) {
> +				fprintf(stderr, "%s:  -f and -p are exclusive\n", argv[0]);
> +				exit(1);
> +			}
> +			init = 1;
> +			if (matchpathcon_init_prefix(NULL, optarg)) {
> +				fprintf(stderr, "Error while processing %s:  %s\n",
> +					optarg, errno ? strerror(errno) : "invalid");
> +				exit(1);
> +			}
> +			break;
>  		default:
>  			usage(argv[0]);
>  		}
>   
Why can't we confirm after the match.

IE Match /dev/mydevice, now verify you have a good context.

Then allow restorcon/fixfiles ... to do it the old way.

-- 



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

Re: matchpathcon_init overhead

[Stephen Smalley]

On Wed, 2005-11-30 at 17:12 -0500, Daniel J Walsh wrote:
> Why can't we confirm after the match.
> 
> IE Match /dev/mydevice, now verify you have a good context.
> 
> Then allow restorcon/fixfiles ... to do it the old way.

That would avoid the overhead of context canonicalization/validation,
but wouldn't avoid the overhead of the regex compilation on all of
file_contexts.  We can try that first if you want, but allowing
applications to select a subset of file_contexts entries for matching
seems useful.

-- 
Stephen Smalley
National Security Agency


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

Re: matchpathcon_init overhead

[Daniel J Walsh]

Stephen Smalley wrote:
> On Wed, 2005-11-30 at 17:12 -0500, Daniel J Walsh wrote:
>   
>> Why can't we confirm after the match.
>>
>> IE Match /dev/mydevice, now verify you have a good context.
>>
>> Then allow restorcon/fixfiles ... to do it the old way.
>>     
>
> That would avoid the overhead of context canonicalization/validation,
> but wouldn't avoid the overhead of the regex compilation on all of
> file_contexts.  We can try that first if you want, but allowing
> applications to select a subset of file_contexts entries for matching
> seems useful.
>
>   
I was just thinking of the case of install.  If you using install for 
installing /usr/share/mypackage/myfile,
I guess you could submit /usr as the device, but you are still going to 
get a hell of a lot of matches.  Then
there is the case of you installing into /mydir/mypackage, and /mydir 
would be submitted and probably match nothing, so
what happens then.  May be we want both options.  One for udev and one 
for apps apps that use matchpathcon like install.


-- 



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

Re: matchpathcon_init overhead

[Stephen Smalley]

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

On Thu, 2005-12-01 at 08:42 -0500, Daniel J Walsh wrote:
> I was just thinking of the case of install.  If you using install for 
> installing /usr/share/mypackage/myfile,
> I guess you could submit /usr as the device, but you are still going to 
> get a hell of a lot of matches.

I expected install to pass the full destination path as the "prefix",
i.e. /usr/share/mypackage/myfile or at least /usr/share/mypackage.  The
prefix can be any path, even a complete one.

>   Then
> there is the case of you installing into /mydir/mypackage, and /mydir 
> would be submitted and probably match nothing, so
> what happens then.  May be we want both options.  One for udev and one 
> for apps apps that use matchpathcon like install.

Ok.  Attached is a patch relative to the prior one for libselinux that
also makes the context validation/canonicalization optional at
matchpathcon_init time, deferring it to a successful matchpathcon by
default unless a new flag is set via set_matchpathcon_flags.  This
avoids any need to change the majority of users (rpm, udev,
install, ...) to pick up the new behavior.  Also attached is a patch for
setfiles that sets the flag so that it retains the original behavior of
validating/canonicalizing during init.  Seems like restorecon should
have the same behavior as install, since it is often used selectively on
a few files, so I didn't patch it.  Seem sane?

-- 
Stephen Smalley
National Security Agency

[-- Attachment #2: matchpathcon.diff2 --]
[-- Type: text/x-patch, Size: 3412 bytes --]

Index: libselinux/include/selinux/selinux.h
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libselinux/include/selinux/selinux.h,v
retrieving revision 1.52
diff -u -p -r1.52 selinux.h
--- libselinux/include/selinux/selinux.h	1 Dec 2005 13:29:04 -0000	1.52
+++ libselinux/include/selinux/selinux.h	1 Dec 2005 13:33:44 -0000
@@ -293,6 +293,7 @@ extern void set_matchpathcon_canoncon(in
 /* Set flags controlling operation of matchpathcon_init or matchpathcon. */
 #define MATCHPATHCON_BASEONLY 1 /* Only process the base file_contexts file. */
 #define MATCHPATHCON_NOTRANS  2 /* Do not perform any context translation. */
+#define MATCHPATHCON_VALIDATE 4 /* Validate/canonicalize contexts at init time. */
 extern void set_matchpathcon_flags(unsigned int flags);
 
 /* Load the file contexts configuration specified by 'path'
Index: libselinux/src/matchpathcon.c
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/libselinux/src/matchpathcon.c,v
retrieving revision 1.35
diff -u -p -r1.35 matchpathcon.c
--- libselinux/src/matchpathcon.c	1 Dec 2005 13:29:04 -0000	1.35
+++ libselinux/src/matchpathcon.c	1 Dec 2005 14:01:36 -0000
@@ -84,7 +84,10 @@ static int default_canoncon(const char *
 	if (security_canonicalize_context(*context, &tmpcon) < 0) {
 		if (errno == ENOENT)
 			return 0;
-		myprintf("%s:  line %u has invalid context %s\n", path, lineno, *context);
+		if (lineno)
+			myprintf("%s:  line %u has invalid context %s\n", path, lineno, *context);
+		else
+			myprintf("%s:  invalid context %s\n", path, *context);
 		return 1;
 	}
 	free(*context);
@@ -116,6 +119,7 @@ typedef struct spec {
 	char *regex_str;	/* regular expession string for diagnostic messages */
 	char *type_str;		/* type string for diagnostic messages */
 	char *context;		/* context string */
+	int context_valid;      /* context string has been validated/canonicalized */
 	regex_t regex;		/* compiled regular expression */
 	mode_t mode;		/* mode format value */
 	int matches;		/* number of matching pathnames */
@@ -599,14 +603,17 @@ static int process_line( const char *pat
 			}
 
 skip_trans:
-			if (myinvalidcon) {
-				/* Old-style validation of context. */
-				if (myinvalidcon(path, lineno, context)) 
-					return 0;
-			} else {
-				/* New canonicalization of context. */
-				if (mycanoncon(path, lineno, &context))
-					return 0;
+			if (myflags & MATCHPATHCON_VALIDATE) {
+				if (myinvalidcon) {
+					/* Old-style validation of context. */
+					if (myinvalidcon(path, lineno, context)) 
+						return 0;
+				} else {
+					/* New canonicalization of context. */
+					if (mycanoncon(path, lineno, &context))
+						return 0;
+				}
+				spec_arr[nspec].context_valid = 1;
 			}
 		}
 
@@ -813,11 +820,28 @@ int matchpathcon(const char *name, 
 		return -1;
 	}
 
+	if (!spec_arr[i].context_valid) {
+		if (myinvalidcon) {
+			/* Old-style validation of context. */
+			if (myinvalidcon("file_contexts", 0, spec_arr[i].context))
+				goto bad;
+		} else {
+			/* New canonicalization of context. */
+			if (mycanoncon("file_contexts", 0, &spec_arr[i].context))
+				goto bad;
+		}
+		spec_arr[i].context_valid = 1;
+	}
+
 	*con = strdup(spec_arr[i].context);
 	if (!(*con))
 		return -1;
 
 	return 0;
+
+bad:
+	errno = EINVAL;
+	return -1;
 }
 
 int matchpathcon_index(const char *name, 

[-- Attachment #3: setfiles.diff --]
[-- Type: text/x-patch, Size: 1090 bytes --]

Index: policycoreutils/setfiles/setfiles.c
===================================================================
RCS file: /nfshome/pal/CVS/selinux-usr/policycoreutils/setfiles/setfiles.c,v
retrieving revision 1.39
diff -u -p -r1.39 setfiles.c
--- policycoreutils/setfiles/setfiles.c	8 Nov 2005 19:31:26 -0000	1.39
+++ policycoreutils/setfiles/setfiles.c	1 Dec 2005 13:45:24 -0000
@@ -417,6 +417,9 @@ int main(int argc, char **argv)
 
 	memset(excludeArray,0, sizeof(excludeArray));
 
+	/* Validate all file contexts during matchpathcon_init. */
+	set_matchpathcon_flags(MATCHPATHCON_VALIDATE);
+
 	/* Process any options. */
 	while ((opt = getopt(argc, argv, "Fc:dlnqrsvWe:o:")) > 0) {
 		switch (opt) {
@@ -443,7 +446,7 @@ int main(int argc, char **argv)
 			/* Only process the specified file_contexts file, not
 			   any .homedirs or .local files, and do not perform
 			   context translations. */
-			set_matchpathcon_flags(MATCHPATHCON_BASEONLY|MATCHPATHCON_NOTRANS);
+			set_matchpathcon_flags(MATCHPATHCON_BASEONLY|MATCHPATHCON_NOTRANS|MATCHPATHCON_VALIDATE);
 			
 			break;
 		}

Re: matchpathcon_init overhead

[Stephen Smalley]

On Thu, 2005-12-01 at 09:24 -0500, Stephen Smalley wrote:
> Ok.  Attached is a patch relative to the prior one for libselinux that
> also makes the context validation/canonicalization optional at
> matchpathcon_init time, deferring it to a successful matchpathcon by
> default unless a new flag is set via set_matchpathcon_flags.  This
> avoids any need to change the majority of users (rpm, udev,
> install, ...) to pick up the new behavior.  Also attached is a patch for
> setfiles that sets the flag so that it retains the original behavior of
> validating/canonicalizing during init.  Seems like restorecon should
> have the same behavior as install, since it is often used selectively on
> a few files, so I didn't patch it.  Seem sane?

These patches are included in libselinux 1.27.28 and policycoreutils
1.27.33.  Thus, you should see improved performance in udev and install
due to the lazy context canonicalization immediately without needing to
modify them, but you can further optimize them by having them also call
matchpathcon_init_prefix() with a suitable prefix once during startup
prior to any matchpathcon() calls.

-- 
Stephen Smalley
National Security Agency


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