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

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

[xunchang]

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>
---
 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,
-- 
2.21.0.360.g471c308f928-goog

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

[Stephen Smalley]

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

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

[Stephen Smalley]

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

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

[Nicolas Iooss]

On Wed, Jun 19, 2019 at 4:45 PM Stephen Smalley <sds@tycho.nsa.gov> wrote:
>
> 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?

Hello,

What are the states of this patch and the one which has been posted in
April (https://lore.kernel.org/selinux/20190417180955.136942-1-xunchang@google.com/)?
I do not follow what happens in Android but if the patches have been
modified there, it seems a good idea to post the modified patches to
selinux@vger.kernel.org.

Thanks,
Nicolas

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

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

[Stephen Smalley]

On 7/23/19 4:06 PM, Nicolas Iooss wrote:
> On Wed, Jun 19, 2019 at 4:45 PM Stephen Smalley <sds@tycho.nsa.gov> wrote:
>>
>> 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?
> 
> Hello,
> 
> What are the states of this patch and the one which has been posted in
> April (https://lore.kernel.org/selinux/20190417180955.136942-1-xunchang@google.com/)?
> I do not follow what happens in Android but if the patches have been
> modified there, it seems a good idea to post the modified patches to
> selinux@vger.kernel.org.

FWIW, the Android patch that was ultimately merged can be seen and 
downloaded from 
https://android-review.googlesource.com/c/platform/external/selinux/+/918713. 
  The posted patch was the version 1 patch.


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

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

[Nicolas Iooss]

On Wed, Jul 24, 2019 at 5:39 PM Richard Haines
<richard_c_haines@btinternet.com> wrote:
>
> On Tue, 2019-07-23 at 22:06 +0200, Nicolas Iooss wrote:
> > On Wed, Jun 19, 2019 at 4:45 PM Stephen Smalley <sds@tycho.nsa.gov>
> > wrote:
> > > 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?
> >
> > Hello,
> >
> > What are the states of this patch and the one which has been posted
> > in
> > April (
> > https://lore.kernel.org/selinux/20190417180955.136942-1-xunchang@google.com/
> > )?
> > I do not follow what happens in Android but if the patches have been
> > modified there, it seems a good idea to post the modified patches to
> > selinux@vger.kernel.org.
> >
> > Thanks,
> > Nicolas
>
> Once upon a time Android changed the way restorecon(8) works by
> replacing the per-mountpoint security.restorecon_last attribute with a
> per-directory security.sehash attribute computed from only those file
> contexts entries that partially match the directory.
>
> To achieve this Android produced the first three patches that are
> mentioned in Tianjie Xu reply to this thread (One specific to Android
> (for their version of restorecon), and two that are common to upstream
> SELinux if implementing per-directory attributes).
>
> The V4 patches I've sent [1], will implement the upstream version of
> restorecon(3) supporting per-directory attributes. Plus it also
> resolves "the requirement for caller to have CAP_SYS_ADMIN to call
> setxattr" problem mentioned by Tianjie Xu.
>
> However, to implement my patches [1], you need first to install the two
> common patches [2] and [3] that Android have already installed and sent
> to selinux@vger.kernel.org (read my cover letter patch for details).
>
> I think what Stephen is eluding to in his initial email, is that one of
> the patches submitted to Android and the corresponding patch to this
> list [2] are slightly different, and that the Android team should
> resolve this before any merging can take place. The differences I've
> detected are listed at the end of this email.
>
> Hope this clarifies the situation.
>
> Richard
>
> [1]
> https://lore.kernel.org/selinux/20190706152115.8490-1-richard_c_haines@btinternet.com/T/#u
> [2]
> https://lore.kernel.org/selinux/20190311222442.49824-1-xunchang@google.com/
> [3]
> https://lore.kernel.org/selinux/20190417180955.136942-1-xunchang@google.com/

Thanks for your explanation. This indeed clarified the understanding I
have of these patches and I agree with merging the 2 patches you used
as a base of your 2 patches. I have created a Pull Request for this,
https://github.com/SELinuxProject/selinux/pull/172 , and will merge it
tomorrow if nobody disagrees.

Thanks,
Nicolas

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

[Nicolas Iooss]

On Sun, Jul 28, 2019 at 8:30 PM Nicolas Iooss <nicolas.iooss@m4x.org> wrote:
>
> On Wed, Jul 24, 2019 at 5:39 PM Richard Haines
> <richard_c_haines@btinternet.com> wrote:
> >
> > On Tue, 2019-07-23 at 22:06 +0200, Nicolas Iooss wrote:
> > > On Wed, Jun 19, 2019 at 4:45 PM Stephen Smalley <sds@tycho.nsa.gov>
> > > wrote:
> > > > 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?
> > >
> > > Hello,
> > >
> > > What are the states of this patch and the one which has been posted
> > > in
> > > April (
> > > https://lore.kernel.org/selinux/20190417180955.136942-1-xunchang@google.com/
> > > )?
> > > I do not follow what happens in Android but if the patches have been
> > > modified there, it seems a good idea to post the modified patches to
> > > selinux@vger.kernel.org.
> > >
> > > Thanks,
> > > Nicolas
> >
> > Once upon a time Android changed the way restorecon(8) works by
> > replacing the per-mountpoint security.restorecon_last attribute with a
> > per-directory security.sehash attribute computed from only those file
> > contexts entries that partially match the directory.
> >
> > To achieve this Android produced the first three patches that are
> > mentioned in Tianjie Xu reply to this thread (One specific to Android
> > (for their version of restorecon), and two that are common to upstream
> > SELinux if implementing per-directory attributes).
> >
> > The V4 patches I've sent [1], will implement the upstream version of
> > restorecon(3) supporting per-directory attributes. Plus it also
> > resolves "the requirement for caller to have CAP_SYS_ADMIN to call
> > setxattr" problem mentioned by Tianjie Xu.
> >
> > However, to implement my patches [1], you need first to install the two
> > common patches [2] and [3] that Android have already installed and sent
> > to selinux@vger.kernel.org (read my cover letter patch for details).
> >
> > I think what Stephen is eluding to in his initial email, is that one of
> > the patches submitted to Android and the corresponding patch to this
> > list [2] are slightly different, and that the Android team should
> > resolve this before any merging can take place. The differences I've
> > detected are listed at the end of this email.
> >
> > Hope this clarifies the situation.
> >
> > Richard
> >
> > [1]
> > https://lore.kernel.org/selinux/20190706152115.8490-1-richard_c_haines@btinternet.com/T/#u
> > [2]
> > https://lore.kernel.org/selinux/20190311222442.49824-1-xunchang@google.com/
> > [3]
> > https://lore.kernel.org/selinux/20190417180955.136942-1-xunchang@google.com/
>
> Thanks for your explanation. This indeed clarified the understanding I
> have of these patches and I agree with merging the 2 patches you used
> as a base of your 2 patches. I have created a Pull Request for this,
> https://github.com/SELinuxProject/selinux/pull/172 , and will merge it
> tomorrow if nobody disagrees.
>
> Thanks,
> Nicolas

... Merged.
Thanks,
Nicolas