diff mbox series

[5/9] Revert "libselinux: avoid memory allocation in common file label lookup"

Message ID 20241211161417.126236-5-jwcart2@gmail.com (mailing list archive)
State Rejected
Delegated to: Petr Lautrbach
Headers show
Series [1/9] Revert "libselinux/utils: drop reachable assert in sefcontext_compile" | expand

Commit Message

James Carter Dec. 11, 2024, 4:14 p.m. UTC
This reverts commit 89dd0b234f041abeb53cd8a436f3998d4cd7eace.

Needed to revert commit 92306daf5219e73f6e8bc9fc7699399457999bcd
"libselinux: rework selabel_file(5) database", which broke Android
file_context matching.

Signed-off-by: James Carter <jwcart2@gmail.com>
---
 libselinux/src/label_file.c       | 103 +++++++++++++-----------------
 libselinux/src/selinux_internal.h |   8 ---
 2 files changed, 43 insertions(+), 68 deletions(-)
diff mbox series

Patch

diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
index be1ee11a..40bcb9ee 100644
--- a/libselinux/src/label_file.c
+++ b/libselinux/src/label_file.c
@@ -1535,30 +1535,12 @@  FUZZ_EXTERN void free_lookup_result(struct lookup_result *result)
 	}
 }
 
-/**
- * lookup_check_node() - Try to find a file context definition in the given node or parents.
- * @node:      The deepest specification node to match against. Parent nodes are successively
- *             searched on no match or when finding all matches.
- * @key:       The absolute file path to look up.
- * @file_kind: The kind of the file to look up (translated from file type into LABEL_FILE_KIND_*).
- * @partial:   Whether to partially match the given file path or completely.
- * @find_all:  Whether to find all file context definitions or just the most specific.
- * @buf:       A pre-allocated buffer for a potential result to avoid allocating it on the heap or
- *             NULL. Mututal exclusive with @find_all.
- *
- * Return: A pointer to a file context definition if a match was found. If @find_all was specified
- *         its a linked list of all results. If @buf was specified it is returned on a match found.
- *         NULL is returned in case of no match found.
- */
-static struct lookup_result *lookup_check_node(struct spec_node *node, const char *key, uint8_t file_kind,
-					       bool partial, bool find_all, struct lookup_result *buf)
+static struct lookup_result *lookup_check_node(struct spec_node *node, const char *key, uint8_t file_kind, bool partial, bool find_all)
 {
 	struct lookup_result *result = NULL;
 	struct lookup_result **next = &result;
 	size_t key_len = strlen(key);
 
-	assert(!(find_all && buf != NULL));
-
 	for (struct spec_node *n = node; n; n = n->parent) {
 
 		uint32_t literal_idx = search_literal_spec(n->literal_specs, n->literal_specs_num, key, key_len, partial);
@@ -1581,14 +1563,10 @@  static struct lookup_result *lookup_check_node(struct spec_node *node, const cha
 						return NULL;
 					}
 
-					if (likely(buf)) {
-						r = buf;
-					} else {
-						r = malloc(sizeof(*r));
-						if (!r) {
-							free_lookup_result(result);
-							return NULL;
-						}
+					r = malloc(sizeof(*r));
+					if (!r) {
+						free_lookup_result(result);
+						return NULL;
 					}
 
 					*r = (struct lookup_result) {
@@ -1600,11 +1578,11 @@  static struct lookup_result *lookup_check_node(struct spec_node *node, const cha
 						.next = NULL,
 					};
 
-					if (likely(!find_all))
-						return r;
-
 					*next = r;
 					next = &r->next;
+
+					if (!find_all)
+						return result;
 				}
 
 				literal_idx++;
@@ -1646,14 +1624,10 @@  static struct lookup_result *lookup_check_node(struct spec_node *node, const cha
 					return NULL;
 				}
 
-				if (likely(buf)) {
-					r = buf;
-				} else {
-					r = malloc(sizeof(*r));
-					if (!r) {
-						free_lookup_result(result);
-						return NULL;
-					}
+				r = malloc(sizeof(*r));
+				if (!r) {
+					free_lookup_result(result);
+					return NULL;
 				}
 
 				*r = (struct lookup_result) {
@@ -1665,12 +1639,12 @@  static struct lookup_result *lookup_check_node(struct spec_node *node, const cha
 					.next = NULL,
 				};
 
-				if (likely(!find_all))
-					return r;
-
 				*next = r;
 				next = &r->next;
 
+				if (!find_all)
+					return result;
+
 				continue;
 			}
 
@@ -1786,8 +1760,7 @@  FUZZ_EXTERN struct lookup_result *lookup_all(struct selabel_handle *rec,
 				 const char *key,
 				 int type,
 				 bool partial,
-				 bool find_all,
-				 struct lookup_result *buf)
+				 bool find_all)
 {
 	struct saved_data *data = (struct saved_data *)rec->data;
 	struct lookup_result *result = NULL;
@@ -1799,18 +1772,18 @@  FUZZ_EXTERN struct lookup_result *lookup_all(struct selabel_handle *rec,
 	unsigned int sofar = 0;
 	char *sub = NULL;
 
-	if (unlikely(!key)) {
+	if (!key) {
 		errno = EINVAL;
 		goto finish;
 	}
 
-	if (unlikely(!data->num_specs)) {
+	if (!data->num_specs) {
 		errno = ENOENT;
 		goto finish;
 	}
 
 	/* Remove duplicate slashes */
-	if (unlikely(next_slash = strstr(key, "//"))) {
+	if ((next_slash = strstr(key, "//"))) {
 		clean_key = (char *) malloc(strlen(key) + 1);
 		if (!clean_key)
 			goto finish;
@@ -1827,12 +1800,12 @@  FUZZ_EXTERN struct lookup_result *lookup_all(struct selabel_handle *rec,
 
 	/* remove trailing slash */
 	len = strlen(key);
-	if (unlikely(len == 0)) {
+	if (len == 0) {
 		errno = EINVAL;
 		goto finish;
 	}
 
-	if (unlikely(len > 1 && key[len - 1] == '/')) {
+	if (len > 1 && key[len - 1] == '/') {
 		/* reuse clean_key from above if available */
 		if (!clean_key) {
 			clean_key = (char *) malloc(len);
@@ -1852,7 +1825,7 @@  FUZZ_EXTERN struct lookup_result *lookup_all(struct selabel_handle *rec,
 
 	node = lookup_find_deepest_node(data->root, key);
 
-	result = lookup_check_node(node, key, file_kind, partial, find_all, buf);
+	result = lookup_check_node(node, key, file_kind, partial, find_all);
 
 finish:
 	free(clean_key);
@@ -1863,9 +1836,14 @@  finish:
 static struct lookup_result *lookup_common(struct selabel_handle *rec,
 					   const char *key,
 					   int type,
-					   bool partial,
-					   struct lookup_result *buf) {
-	return lookup_all(rec, key, type, partial, false, buf);
+					   bool partial) {
+	struct lookup_result *result = lookup_all(rec, key, type, partial, false);
+	if (!result)
+		return NULL;
+
+	free_lookup_result(result->next);
+	result->next = NULL;
+	return result;
 }
 
 /*
@@ -1925,7 +1903,7 @@  static bool hash_all_partial_matches(struct selabel_handle *rec, const char *key
 {
 	assert(digest);
 
-	struct lookup_result *matches = lookup_all(rec, key, 0, true, true, NULL);
+	struct lookup_result *matches = lookup_all(rec, key, 0, true, true);
 	if (!matches) {
 		return false;
 	}
@@ -1954,20 +1932,25 @@  static bool hash_all_partial_matches(struct selabel_handle *rec, const char *key
 static struct selabel_lookup_rec *lookup(struct selabel_handle *rec,
 					 const char *key, int type)
 {
-	struct lookup_result buf, *result;
+	struct lookup_result *result;
+	struct selabel_lookup_rec *lookup_result;
 
-	result = lookup_common(rec, key, type, false, &buf);
+	result = lookup_common(rec, key, type, false);
 	if (!result)
 		return NULL;
 
-	return result->lr;
+	lookup_result = result->lr;
+	free_lookup_result(result);
+	return lookup_result;
 }
 
 static bool partial_match(struct selabel_handle *rec, const char *key)
 {
-	struct lookup_result buf;
+	struct lookup_result *result = lookup_common(rec, key, 0, true);
+	bool ret = result;
 
-	return !!lookup_common(rec, key, 0, true, &buf);
+	free_lookup_result(result);
+	return ret;
 }
 
 static struct selabel_lookup_rec *lookup_best_match(struct selabel_handle *rec,
@@ -1989,7 +1972,7 @@  static struct selabel_lookup_rec *lookup_best_match(struct selabel_handle *rec,
 	results = calloc(n+1, sizeof(*results));
 	if (!results)
 		return NULL;
-	results[0] = lookup_common(rec, key, type, false, NULL);
+	results[0] = lookup_common(rec, key, type, false);
 	if (results[0]) {
 		if (!results[0]->has_meta_chars) {
 			/* exact match on key */
@@ -2000,7 +1983,7 @@  static struct selabel_lookup_rec *lookup_best_match(struct selabel_handle *rec,
 		prefix_len = results[0]->prefix_len;
 	}
 	for (i = 1; i <= n; i++) {
-		results[i] = lookup_common(rec, aliases[i-1], type, false, NULL);
+		results[i] = lookup_common(rec, aliases[i-1], type, false);
 		if (results[i]) {
 			if (!results[i]->has_meta_chars) {
 				/* exact match on alias */
diff --git a/libselinux/src/selinux_internal.h b/libselinux/src/selinux_internal.h
index 964b8418..372837dd 100644
--- a/libselinux/src/selinux_internal.h
+++ b/libselinux/src/selinux_internal.h
@@ -142,12 +142,4 @@  static inline void fclose_errno_safe(FILE *stream)
 	errno = saved_errno;
 }
 
-#ifdef __GNUC__
-# define likely(x)			__builtin_expect(!!(x), 1)
-# define unlikely(x)			__builtin_expect(!!(x), 0)
-#else
-# define likely(x)			(x)
-# define unlikely(x)			(x)
-#endif /* __GNUC__ */
-
 #endif /* SELINUX_INTERNAL_H_ */