selinux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stephen Smalley <sds@tycho.nsa.gov>
To: xunchang <xunchang@google.com>,
	selinux@vger.kernel.org,
	Richard Haines <richard_c_haines@btinternet.com>
Subject: Re: [PATCH] Restorecon: factor out a lookup helper for context matches
Date: Wed, 19 Jun 2019 10:44:45 -0400	[thread overview]
Message-ID: <3a5038d7-cb5b-de62-23ee-e22324a8a989@tycho.nsa.gov> (raw)
In-Reply-To: <20190311222442.49824-1-xunchang@google.com>

On 3/11/19 6:24 PM, xunchang wrote:
> We used to hash the file_context and skip the restorecon on the top
> level directory if the hash doesn't change. But the file_context might
> change after an update; and some users experienced long restorecon
> time as they have lots of files under directories like /data/media.
> Therefore, we try to skip unnecessary restores if the file context
> relates to the given directory doesn't change.
> 
> This CL is the first step that factors out a lookup helper function
> and returns an array of matched pointers instead of a single one.
> The old loopup_common function is then modified to take the first
> element in the array.
> 
> This change has already been submitted in android selinux branch. And
> porting it upstream will make these two branches more consistent and
> save some work for the future merges.

There were some changes to this patch before it landed in AOSP, so they 
aren't quite consistent.  Do you want to submit the final patch?

> 
> Signed-off-by: Tianjie Xu <xunchang@google.com>
> ---
>   libselinux/include/selinux/label.h |   4 ++
>   libselinux/src/label.c             |   9 +++
>   libselinux/src/label_file.c        | 111 +++++++++++++++++++++++------
>   libselinux/src/label_internal.h    |   2 +
>   4 files changed, 106 insertions(+), 20 deletions(-)
> 
> diff --git a/libselinux/include/selinux/label.h b/libselinux/include/selinux/label.h
> index 277287ed..e537aa11 100644
> --- a/libselinux/include/selinux/label.h
> +++ b/libselinux/include/selinux/label.h
> @@ -7,6 +7,7 @@
>   #define _SELABEL_H_
>   
>   #include <stdbool.h>
> +#include <stdint.h>
>   #include <sys/types.h>
>   #include <selinux/selinux.h>
>   
> @@ -105,6 +106,9 @@ int selabel_lookup_raw(struct selabel_handle *handle, char **con,
>   
>   bool selabel_partial_match(struct selabel_handle *handle, const char *key);
>   
> +bool selabel_hash_all_partial_matches(struct selabel_handle *rec,
> +                                      const char *key, uint8_t* digest);
> +
>   int selabel_lookup_best_match(struct selabel_handle *rec, char **con,
>   			      const char *key, const char **aliases, int type);
>   int selabel_lookup_best_match_raw(struct selabel_handle *rec, char **con,
> diff --git a/libselinux/src/label.c b/libselinux/src/label.c
> index 8d586bda..1d16f685 100644
> --- a/libselinux/src/label.c
> +++ b/libselinux/src/label.c
> @@ -274,6 +274,15 @@ bool selabel_partial_match(struct selabel_handle *rec, const char *key)
>   	return rec->func_partial_match(rec, key);
>   }
>   
> +bool selabel_hash_all_partial_matches(struct selabel_handle *rec,
> +                                      const char *key, uint8_t *digest) {
> +	if (!rec->func_hash_all_partial_matches) {
> +		return false;
> +	}
> +
> +	return rec->func_hash_all_partial_matches(rec, key, digest);
> +}
> +
>   int selabel_lookup_best_match(struct selabel_handle *rec, char **con,
>   			      const char *key, const char **aliases, int type)
>   {
> diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
> index b81fd552..90cbd666 100644
> --- a/libselinux/src/label_file.c
> +++ b/libselinux/src/label_file.c
> @@ -843,22 +843,38 @@ static void closef(struct selabel_handle *rec)
>   	free(data);
>   }
>   
> -static struct spec *lookup_common(struct selabel_handle *rec,
> -					     const char *key,
> -					     int type,
> -					     bool partial)
> +// Finds all the matches of |key| in the given context. Returns the result in
> +// the allocated array and updates the match count. If match_count is NULL,
> +// stops early once the 1st match is found.
> +static const struct spec **lookup_all(struct selabel_handle *rec,
> +                                      const char *key,
> +                                      int type,
> +                                      bool partial,
> +                                      size_t *match_count)
>   {
>   	struct saved_data *data = (struct saved_data *)rec->data;
>   	struct spec *spec_arr = data->spec_arr;
>   	int i, rc, file_stem;
>   	mode_t mode = (mode_t)type;
>   	const char *buf;
> -	struct spec *ret = NULL;
>   	char *clean_key = NULL;
>   	const char *prev_slash, *next_slash;
>   	unsigned int sofar = 0;
>   	char *sub = NULL;
>   
> +	const struct spec **result = NULL;
> +	if (match_count) {
> +		*match_count = 0;
> +		result = calloc(data->nspec, sizeof(struct spec*));
> +	} else {
> +		result = calloc(1, sizeof(struct spec*));
> +	}
> +	if (!result) {
> +		selinux_log(SELINUX_ERROR, "Failed to allocate %zu bytes of data\n",
> +			    data->nspec * sizeof(struct spec*));
> +		goto finish;
> +	}
> +
>   	if (!data->nspec) {
>   		errno = ENOENT;
>   		goto finish;
> @@ -899,18 +915,33 @@ static struct spec *lookup_common(struct selabel_handle *rec,
>   		 * specified or if the mode matches the file mode then we do
>   		 * a regex check        */
>   		if ((spec->stem_id == -1 || spec->stem_id == file_stem) &&
> -		    (!mode || !spec->mode || mode == spec->mode)) {
> +				(!mode || !spec->mode || mode == spec->mode)) {
>   			if (compile_regex(data, spec, NULL) < 0)
>   				goto finish;
>   			if (spec->stem_id == -1)
>   				rc = regex_match(spec->regex, key, partial);
>   			else
>   				rc = regex_match(spec->regex, buf, partial);
> -			if (rc == REGEX_MATCH) {
> -				spec->matches++;
> -				break;
> -			} else if (partial && rc == REGEX_MATCH_PARTIAL)
> +
> +			if (rc == REGEX_MATCH || (partial && rc == REGEX_MATCH_PARTIAL)) {
> +				if (rc == REGEX_MATCH) {
> +					spec->matches++;
> +				}
> +
> +				if (strcmp(spec_arr[i].lr.ctx_raw, "<<none>>") == 0) {
> +					errno = ENOENT;
> +					goto finish;
> +				}
> +
> +				if (match_count) {
> +					result[*match_count] = spec;
> +					*match_count += 1;
> +					// Continue to find all the matches.
> +					continue;
> +				}
> +				result[0] = spec;
>   				break;
> +			}
>   
>   			if (rc == REGEX_NO_MATCH)
>   				continue;
> @@ -921,19 +952,58 @@ static struct spec *lookup_common(struct selabel_handle *rec,
>   		}
>   	}
>   
> -	if (i < 0 || strcmp(spec_arr[i].lr.ctx_raw, "<<none>>") == 0) {
> -		/* No matching specification. */
> -		errno = ENOENT;
> -		goto finish;
> -	}
> -
> -	errno = 0;
> -	ret = &spec_arr[i];
> -
>   finish:
>   	free(clean_key);
>   	free(sub);
> -	return ret;
> +	if (result && !result[0]) {
> +		free(result);
> +		result = NULL;
> +	}
> +	return result;
> +}
> +
> +static struct spec *lookup_common(struct selabel_handle *rec,
> +                                  const char *key,
> +                                  int type,
> +                                  bool partial) {
> +	const struct spec **matches = lookup_all(rec, key, type, partial, NULL);
> +	if (!matches) {
> +		return NULL;
> +	}
> +	struct spec *result = (struct spec*)matches[0];
> +	free(matches);
> +	return result;
> +}
> +
> +static bool hash_all_partial_matches(struct selabel_handle *rec, const char *key, uint8_t *digest)
> +{
> +	assert(digest);
> +
> +	size_t total_matches;
> +	const struct spec **matches = lookup_all(rec, key, 0, true, &total_matches);
> +	if (!matches) {
> +		return false;
> +	}
> +
> +	Sha1Context context;
> +	Sha1Initialise(&context);
> +	size_t i;
> +	for (i = 0; i < total_matches; i++) {
> +		char* regex_str = matches[i]->regex_str;
> +		mode_t mode = matches[i]->mode;
> +		char* ctx_raw = matches[i]->lr.ctx_raw;
> +
> +		Sha1Update(&context, regex_str, strlen(regex_str) + 1);
> +		Sha1Update(&context, &mode, sizeof(mode_t));
> +		Sha1Update(&context, ctx_raw, strlen(ctx_raw) + 1);
> +	}
> +
> +	SHA1_HASH sha1_hash;
> +	Sha1Finalise(&context, &sha1_hash);
> +	memcpy(digest, sha1_hash.bytes, SHA1_HASH_SIZE);
> +
> +	free(matches);
> +	return true;
>   }
>   
>   static struct selabel_lookup_rec *lookup(struct selabel_handle *rec,
> @@ -1133,6 +1203,7 @@ int selabel_file_init(struct selabel_handle *rec,
>   	rec->func_stats = &stats;
>   	rec->func_lookup = &lookup;
>   	rec->func_partial_match = &partial_match;
> +	rec->func_hash_all_partial_matches = &hash_all_partial_matches;
>   	rec->func_lookup_best_match = &lookup_best_match;
>   	rec->func_cmp = &cmp;
>   
> diff --git a/libselinux/src/label_internal.h b/libselinux/src/label_internal.h
> index 0e020557..1fa5ade6 100644
> --- a/libselinux/src/label_internal.h
> +++ b/libselinux/src/label_internal.h
> @@ -87,6 +87,8 @@ struct selabel_handle {
>   	void (*func_close) (struct selabel_handle *h);
>   	void (*func_stats) (struct selabel_handle *h);
>   	bool (*func_partial_match) (struct selabel_handle *h, const char *key);
> +	bool (*func_hash_all_partial_matches) (struct selabel_handle *h,
> +	                                       const char *key, uint8_t *digest);
>   	struct selabel_lookup_rec *(*func_lookup_best_match)
>   						    (struct selabel_handle *h,
>   						    const char *key,
> 


  parent reply	other threads:[~2019-06-19 14:45 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-11 22:24 [PATCH] Restorecon: factor out a lookup helper for context matches xunchang
2019-03-13 18:06 ` Stephen Smalley
2019-06-19 14:44 ` Stephen Smalley [this message]
2019-07-23 20:06   ` Nicolas Iooss
2019-07-23 20:21     ` Stephen Smalley
     [not found]     ` <32330df5cf5c0daf1de03c049637694856aa69c9.camel@btinternet.com>
2019-07-28 18:30       ` Nicolas Iooss
2019-07-29 21:35         ` Nicolas Iooss

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3a5038d7-cb5b-de62-23ee-e22324a8a989@tycho.nsa.gov \
    --to=sds@tycho.nsa.gov \
    --cc=richard_c_haines@btinternet.com \
    --cc=selinux@vger.kernel.org \
    --cc=xunchang@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).