selinux: remove the sidtab context conversion indirect calls

The sidtab conversion code has support for multiple context conversion routines through the use of function pointers and indirect calls. However, the reality is that all current users rely on the same conversion routine: convert_context(). This patch does away with this extra complexity and replaces the indirect calls with direct function calls; allowing us to remove a layer of obfuscation and create cleaner, more maintainable code. Reviewed-by: Ondrej Mosnacek <[email protected]> Signed-off-by: Paul Moore <[email protected]>
majek · Nov 9, 2022 · 048be15 · 048be15
1 parent e0d8259
commit 048be15
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 44 deletions.
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
@@ -68,12 +68,6 @@
 #include "policycap_names.h"
 #include "ima.h"
 
-struct convert_context_args {
-	struct selinux_state *state;
-	struct policydb *oldp;
-	struct policydb *newp;
-};
-
 struct selinux_policy_convert_data {
 	struct convert_context_args args;
 	struct sidtab_convert_params sidtab_params;
@@ -2014,17 +2008,20 @@ static inline int convert_context_handle_invalid_context(
 	return 0;
 }
 
-/*
- * Convert the values in the security context
- * structure `oldc' from the values specified
- * in the policy `p->oldp' to the values specified
- * in the policy `p->newp', storing the new context
- * in `newc'.  Verify that the context is valid
- * under the new policy.
+/**
+ * services_convert_context - Convert a security context across policies.
+ * @args: populated convert_context_args struct
+ * @oldc: original context
+ * @newc: converted context
+ *
+ * Convert the values in the security context structure @oldc from the values
+ * specified in the policy @args->oldp to the values specified in the policy
+ * @args->newp, storing the new context in @newc, and verifying that the
+ * context is valid under the new policy.
  */
-static int convert_context(struct context *oldc, struct context *newc, void *p)
+int services_convert_context(struct convert_context_args *args,
+			     struct context *oldc, struct context *newc)
 {
-	struct convert_context_args *args;
 	struct ocontext *oc;
 	struct role_datum *role;
 	struct type_datum *typdatum;
@@ -2033,15 +2030,12 @@ static int convert_context(struct context *oldc, struct context *newc, void *p)
 	u32 len;
 	int rc;
 
-	args = p;
-
 	if (oldc->str) {
 		s = kstrdup(oldc->str, GFP_KERNEL);
 		if (!s)
 			return -ENOMEM;
 
-		rc = string_to_context_struct(args->newp, NULL, s,
-					      newc, SECSID_NULL);
+		rc = string_to_context_struct(args->newp, NULL, s, newc, SECSID_NULL);
 		if (rc == -EINVAL) {
 			/*
 			 * Retain string representation for later mapping.
@@ -2072,8 +2066,7 @@ static int convert_context(struct context *oldc, struct context *newc, void *p)
 
 	/* Convert the user. */
 	usrdatum = symtab_search(&args->newp->p_users,
-				 sym_name(args->oldp,
-					  SYM_USERS, oldc->user - 1));
+				 sym_name(args->oldp, SYM_USERS, oldc->user - 1));
 	if (!usrdatum)
 		goto bad;
 	newc->user = usrdatum->value;
@@ -2087,8 +2080,7 @@ static int convert_context(struct context *oldc, struct context *newc, void *p)
 
 	/* Convert the type. */
 	typdatum = symtab_search(&args->newp->p_types,
-				 sym_name(args->oldp,
-					  SYM_TYPES, oldc->type - 1));
+				 sym_name(args->oldp, SYM_TYPES, oldc->type - 1));
 	if (!typdatum)
 		goto bad;
 	newc->type = typdatum->value;
@@ -2122,8 +2114,7 @@ static int convert_context(struct context *oldc, struct context *newc, void *p)
 	/* Check the validity of the new context. */
 	if (!policydb_context_isvalid(args->newp, newc)) {
 		rc = convert_context_handle_invalid_context(args->state,
-							args->oldp,
-							oldc);
+							    args->oldp, oldc);
 		if (rc)
 			goto bad;
 	}
@@ -2332,21 +2323,21 @@ int security_load_policy(struct selinux_state *state, void *data, size_t len,
 		goto err_free_isids;
 	}
 
+	/*
+	 * Convert the internal representations of contexts
+	 * in the new SID table.
+	 */
+
 	convert_data = kmalloc(sizeof(*convert_data), GFP_KERNEL);
 	if (!convert_data) {
 		rc = -ENOMEM;
 		goto err_free_isids;
 	}
 
-	/*
-	 * Convert the internal representations of contexts
-	 * in the new SID table.
-	 */
 	convert_data->args.state = state;
 	convert_data->args.oldp = &oldpolicy->policydb;
 	convert_data->args.newp = &newpolicy->policydb;
 
-	convert_data->sidtab_params.func = convert_context;
 	convert_data->sidtab_params.args = &convert_data->args;
 	convert_data->sidtab_params.target = newpolicy->sidtab;
 

diff --git a/security/selinux/ss/services.h b/security/selinux/ss/services.h
@@ -29,10 +29,18 @@ struct selinux_policy {
 	u32 latest_granting;
 } __randomize_layout;
 
-void services_compute_xperms_drivers(struct extended_perms *xperms,
-				struct avtab_node *node);
+struct convert_context_args {
+	struct selinux_state *state;
+	struct policydb *oldp;
+	struct policydb *newp;
+};
 
+void services_compute_xperms_drivers(struct extended_perms *xperms,
+				     struct avtab_node *node);
 void services_compute_xperms_decision(struct extended_perms_decision *xpermd,
-					struct avtab_node *node);
+				      struct avtab_node *node);
+
+int services_convert_context(struct convert_context_args *args,
+			     struct context *oldc, struct context *newc);
 
 #endif	/* _SS_SERVICES_H_ */
diff --git a/security/selinux/ss/sidtab.c b/security/selinux/ss/sidtab.c
@@ -18,6 +18,7 @@
 #include "flask.h"
 #include "security.h"
 #include "sidtab.h"
+#include "services.h"
 
 struct sidtab_str_cache {
 	struct rcu_head rcu_member;
@@ -292,7 +293,6 @@ int sidtab_context_to_sid(struct sidtab *s, struct context *context,
 	}
 
 	count = s->count;
-	convert = s->convert;
 
 	/* bail out if we already reached max entries */
 	rc = -EOVERFLOW;
@@ -316,25 +316,28 @@ int sidtab_context_to_sid(struct sidtab *s, struct context *context,
 	 * if we are building a new sidtab, we need to convert the context
 	 * and insert it there as well
 	 */
+	convert = s->convert;
 	if (convert) {
+		struct sidtab *target = convert->target;
+
 		rc = -ENOMEM;
-		dst_convert = sidtab_do_lookup(convert->target, count, 1);
+		dst_convert = sidtab_do_lookup(target, count, 1);
 		if (!dst_convert) {
 			context_destroy(&dst->context);
 			goto out_unlock;
 		}
 
-		rc = convert->func(context, &dst_convert->context,
-				   convert->args);
+		rc = services_convert_context(convert->args,
+					      context, &dst_convert->context);
 		if (rc) {
 			context_destroy(&dst->context);
 			goto out_unlock;
 		}
 		dst_convert->sid = index_to_sid(count);
 		dst_convert->hash = context_compute_hash(&dst_convert->context);
-		convert->target->count = count + 1;
+		target->count = count + 1;
 
-		hash_add_rcu(convert->target->context_to_sid,
+		hash_add_rcu(target->context_to_sid,
 			     &dst_convert->list, dst_convert->hash);
 	}
 
@@ -402,9 +405,9 @@ static int sidtab_convert_tree(union sidtab_entry_inner *edst,
 		}
 		i = 0;
 		while (i < SIDTAB_LEAF_ENTRIES && *pos < count) {
-			rc = convert->func(&esrc->ptr_leaf->entries[i].context,
-					   &edst->ptr_leaf->entries[i].context,
-					   convert->args);
+			rc = services_convert_context(convert->args,
+					&esrc->ptr_leaf->entries[i].context,
+					&edst->ptr_leaf->entries[i].context);
 			if (rc)
 				return rc;
 			(*pos)++;

diff --git a/security/selinux/ss/sidtab.h b/security/selinux/ss/sidtab.h
@@ -65,8 +65,7 @@ struct sidtab_isid_entry {
 };
 
 struct sidtab_convert_params {
-	int (*func)(struct context *oldc, struct context *newc, void *args);
-	void *args;
+	struct convert_context_args *args;
 	struct sidtab *target;
 };