Re: [PATCH] Restorecon: factor out a lookup helper for context matches

[Stephen Smalley <sds@tycho.nsa.gov>]

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.
> 
> Signed-off-by: Tianjie Xu <xunchang@google.com>

This looks good to me but we might defer merging it until after the 
selinux userspace 2.9 release.

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