selinux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] libsepol: Fix sid handling when writing out policy from binary
@ 2025-11-04 20:52 James Carter
  2025-11-04 20:52 ` [PATCH 2/2] checkpolicy/tests: Modify tests to check handling of initial sids James Carter
  2025-11-05 18:58 ` [PATCH 1/2] libsepol: Fix sid handling when writing out policy from binary Stephen Smalley
  0 siblings, 2 replies; 5+ messages in thread
From: James Carter @ 2025-11-04 20:52 UTC (permalink / raw)
  To: selinux; +Cc: stephen.smalley.work, russell, James Carter

Initial sids are stored only as unsigned 32-bit numbers in a
binary policy. When a binary kernel policy is converted to CIL
or a policy.conf or a binary base module is converted to CIL, a
mapping in kernel_to_common.h is used to determine the name of
the initial sid.

A problem can occur when policy converted from binary to text is
once again compiled. The initial sids will not be the correct
number if there are gaps in the list of initial sids. This will
cause the effected initial sids to be interpreted by the kernel
as a different initial sid.

When writing out sid and sidorder statements in CIL, write out
all the initial sids from kernel (which is initial sid #1) to the
initial sid with the highest number associated with it. In the
same way, when writing out sid statements for a policy.conf, all
the initial sids from the first to the highest numbered must be
written out with no gaps.

No changes are needed when writing out statements associating
an initial sid with a security context. There can be gaps in
these statements. The numbering is taken from the declarations.

Signed-off-by: James Carter <jwcart2@gmail.com>
---
 libsepol/src/kernel_to_cil.c    |  47 +++-----------
 libsepol/src/kernel_to_common.c |  48 +++++++++++++++
 libsepol/src/kernel_to_common.h |   1 +
 libsepol/src/kernel_to_conf.c   |  41 +++---------
 libsepol/src/module_to_cil.c    | 106 ++++++++++++++++----------------
 5 files changed, 121 insertions(+), 122 deletions(-)

diff --git a/libsepol/src/kernel_to_cil.c b/libsepol/src/kernel_to_cil.c
index 4da63ba5..06cf4498 100644
--- a/libsepol/src/kernel_to_cil.c
+++ b/libsepol/src/kernel_to_cil.c
@@ -565,54 +565,31 @@ exit:
 static int write_sids_to_cil(FILE *out, const char *const *sid_to_str,
 			     unsigned num_sids, struct ocontext *isids)
 {
-	struct ocontext *isid;
 	struct strs *strs;
 	char *sid;
 	char *prev;
-	char unknown[18];
 	unsigned i;
-	int rc;
 
-	rc = strs_init(&strs, num_sids+1);
-	if (rc != 0) {
-		goto exit;
+	strs = isids_to_strs(sid_to_str, num_sids, isids);
+	if (!strs) {
+		ERR(NULL, "Error writing sid rules to CIL");
+		return -1;
 	}
 
-	for (isid = isids; isid != NULL; isid = isid->next) {
-		i = isid->sid[0];
-		if (i < num_sids && sid_to_str[i]) {
-			sid = strdup(sid_to_str[i]);
-		} else {
-			snprintf(unknown, 18, "%s%u", "UNKNOWN", i);
-			sid = strdup(unknown);
-		}
-		if (!sid) {
-			ERR(NULL, "Out of memory");
-			rc = -1;
-			goto exit;
-		}
-		rc = strs_add_at_index(strs, sid, i);
-		if (rc != 0) {
-			free(sid);
-			goto exit;
-		}
+	if (strs_num_items(strs) == 0) {
+		strs_destroy(&strs);
+		return 0;
 	}
 
-	for (i=0; i<strs_num_items(strs); i++) {
+	for (i=1; i < strs_num_items(strs); i++) {
 		sid = strs_read_at_index(strs, i);
-		if (!sid) {
-			continue;
-		}
 		sepol_printf(out, "(sid %s)\n", sid);
 	}
 
 	sepol_printf(out, "(sidorder (");
 	prev = NULL;
-	for (i=0; i<strs_num_items(strs); i++) {
+	for (i=1; i < strs_num_items(strs); i++) {
 		sid = strs_read_at_index(strs, i);
-		if (!sid) {
-			continue;
-		}
 		if (prev) {
 			sepol_printf(out, "%s ", prev);
 		}
@@ -623,14 +600,10 @@ static int write_sids_to_cil(FILE *out, const char *const *sid_to_str,
 	}
 	sepol_printf(out, "))\n");
 
-exit:
 	strs_free_all(strs);
 	strs_destroy(&strs);
-	if (rc != 0) {
-		ERR(NULL, "Error writing sid rules to CIL");
-	}
 
-	return rc;
+	return 0;
 }
 
 static int write_sid_decl_rules_to_cil(FILE *out, struct policydb *pdb)
diff --git a/libsepol/src/kernel_to_common.c b/libsepol/src/kernel_to_common.c
index e4338ec6..99e46865 100644
--- a/libsepol/src/kernel_to_common.c
+++ b/libsepol/src/kernel_to_common.c
@@ -382,6 +382,54 @@ int strs_stack_empty(const struct strs *stack)
 	return strs_num_items(stack) == 0;
 }
 
+struct strs *isids_to_strs(const char *const *sid_to_str, unsigned num_sids, struct ocontext *isids)
+{
+	struct ocontext *isid;
+	struct strs *strs;
+	char *sid;
+	char unknown[18];
+	unsigned i, max;
+	int rc;
+
+	rc = strs_init(&strs, num_sids+1);
+	if (rc != 0) {
+		goto exit;
+	}
+
+	max = 0;
+	for (isid = isids; isid != NULL; isid = isid->next) {
+		i = isid->sid[0];
+		if (i > max) {
+			max = i;
+		}
+	}
+
+	for (i=1; i <= max; i++) {
+		if (i < num_sids && sid_to_str[i]) {
+			sid = strdup(sid_to_str[i]);
+		} else {
+			snprintf(unknown, 18, "%s%u", "UNKNOWN", i);
+			sid = strdup(unknown);
+		}
+		if (!sid) {
+			ERR(NULL, "Out of memory");
+			goto exit;
+		}
+		rc = strs_add_at_index(strs, sid, i);
+		if (rc != 0) {
+			free(sid);
+			goto exit;
+		}
+	}
+
+	return strs;
+
+exit:
+	strs_free_all(strs);
+	strs_destroy(&strs);
+	return NULL;
+}
+
 static int compare_ranges(uint64_t l1, uint64_t h1, uint64_t l2, uint64_t h2)
 {
 	uint64_t d1, d2;
diff --git a/libsepol/src/kernel_to_common.h b/libsepol/src/kernel_to_common.h
index 3ba97dfc..d3283658 100644
--- a/libsepol/src/kernel_to_common.h
+++ b/libsepol/src/kernel_to_common.h
@@ -115,4 +115,5 @@ int strs_stack_push(struct strs *stack, char *s);
 char *strs_stack_pop(struct strs *stack);
 int strs_stack_empty(const struct strs *stack);
 
+struct strs *isids_to_strs(const char *const *sid_to_str, unsigned num_sids, struct ocontext *isids);
 int sort_ocontexts(struct policydb *pdb);
diff --git a/libsepol/src/kernel_to_conf.c b/libsepol/src/kernel_to_conf.c
index 6d608443..a8126d58 100644
--- a/libsepol/src/kernel_to_conf.c
+++ b/libsepol/src/kernel_to_conf.c
@@ -463,53 +463,30 @@ static int write_class_decl_rules_to_conf(FILE *out, struct policydb *pdb)
 static int write_sids_to_conf(FILE *out, const char *const *sid_to_str,
 			      unsigned num_sids, struct ocontext *isids)
 {
-	struct ocontext *isid;
 	struct strs *strs;
 	char *sid;
-	char unknown[18];
 	unsigned i;
-	int rc;
 
-	rc = strs_init(&strs, num_sids+1);
-	if (rc != 0) {
-		goto exit;
+	strs = isids_to_strs(sid_to_str, num_sids, isids);
+	if (!strs) {
+		ERR(NULL, "Error writing sid rules to policy.conf");
+		return -1;
 	}
 
-	for (isid = isids; isid != NULL; isid = isid->next) {
-		i = isid->sid[0];
-		if (i < num_sids && sid_to_str[i]) {
-			sid = strdup(sid_to_str[i]);
-		} else {
-			snprintf(unknown, sizeof(unknown), "%s%u", "UNKNOWN", i);
-			sid = strdup(unknown);
-		}
-		if (!sid) {
-			rc = -1;
-			goto exit;
-		}
-		rc = strs_add_at_index(strs, sid, i);
-		if (rc != 0) {
-			free(sid);
-			goto exit;
-		}
+	if (strs_num_items(strs) == 0) {
+		strs_destroy(&strs);
+		return 0;
 	}
 
-	for (i=0; i<strs_num_items(strs); i++) {
+	for (i=1; i < strs_num_items(strs); i++) {
 		sid = strs_read_at_index(strs, i);
-		if (!sid) {
-			continue;
-		}
 		sepol_printf(out, "sid %s\n", sid);
 	}
 
-exit:
 	strs_free_all(strs);
 	strs_destroy(&strs);
-	if (rc != 0) {
-		ERR(NULL, "Error writing sid rules to policy.conf");
-	}
 
-	return rc;
+	return 0;
 }
 
 static int write_sid_decl_rules_to_conf(FILE *out, struct policydb *pdb)
diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c
index 8647d928..1c4e80d1 100644
--- a/libsepol/src/module_to_cil.c
+++ b/libsepol/src/module_to_cil.c
@@ -2544,71 +2544,71 @@ static int context_to_cil(struct policydb *pdb, struct context_struct *con)
 static int ocontext_isid_to_cil(struct policydb *pdb, const char *const *sid_to_string,
 				unsigned num_sids, struct ocontext *isids)
 {
-	int rc = -1;
-
 	struct ocontext *isid;
-
-	struct sid_item {
-		char *sid_key;
-		struct sid_item *next;
-	};
-
-	struct sid_item *head = NULL;
-	struct sid_item *item = NULL;
+	struct ocontext **isid_array;
+	struct strs *strs;
 	char *sid;
-	char unknown[18];
+	char *prev;
 	unsigned i;
 
-	for (isid = isids; isid != NULL; isid = isid->next) {
-		i = isid->sid[0];
-		if (i < num_sids && sid_to_string[i]) {
-			sid = (char*)sid_to_string[i];
-		} else {
-			snprintf(unknown, 18, "%s%u", "UNKNOWN", i);
-			sid = unknown;
-		}
-		cil_println(0, "(sid %s)", sid);
-		cil_printf("(sidcontext %s ", sid);
-		context_to_cil(pdb, &isid->context[0]);
-		cil_printf(")\n");
+	strs = isids_to_strs(sid_to_string, num_sids, isids);
+	if (!strs) {
+		ERR(NULL, "Error writing sid rules to CIL");
+		return -1;
+	}
 
-		// get the sid names in the correct order (reverse from the isids
-		// ocontext) for sidorder statement
-		item = malloc(sizeof(*item));
-		if (item == NULL) {
-			ERR(NULL, "Out of memory");
-			rc = -1;
-			goto exit;
-		}
-		item->sid_key = strdup(sid);
-		if (!item->sid_key) {
-			ERR(NULL, "Out of memory");
-			free(item);
-			rc = -1;
-			goto exit;
+	if (strs_num_items(strs) == 0) {
+		strs_destroy(&strs);
+		return 0;
+	}
+
+	for (i=1; i < strs_num_items(strs); i++) {
+		sid = strs_read_at_index(strs, i);
+		cil_printf("(sid %s)\n", sid);
+	}
+
+	cil_printf("(sidorder (");
+	prev = NULL;
+	for (i=1; i < strs_num_items(strs); i++) {
+		sid = strs_read_at_index(strs, i);
+		if (prev) {
+			cil_printf("%s ", prev);
 		}
-		item->next = head;
-		head = item;
+		prev = sid;
+	}
+	if (prev) {
+		cil_printf("%s", prev);
 	}
+	cil_printf("))\n");
 
-	if (head != NULL) {
-		cil_printf("(sidorder (");
-		for (item = head; item != NULL; item = item->next) {
-			cil_printf("%s ", item->sid_key);
+	isid_array = calloc(strs_num_items(strs), sizeof(struct ocontext *));
+	if (!isid_array) {
+		ERR(NULL, "Out of memory");
+		strs_free_all(strs);
+		strs_destroy(&strs);
+		return -1;
+	}
+	for (isid = isids; isid != NULL; isid = isid->next) {
+		i = isid->sid[0];
+		if (i < strs_num_items(strs)) {
+			isid_array[i] = isid;
+		}
+	}
+	for (i=1; i < strs_num_items(strs); i++) {
+		if (isid_array[i]) {
+			sid = strs_read_at_index(strs, i);
+			cil_printf("(sidcontext %s ", sid);
+			isid = isid_array[i];
+			context_to_cil(pdb, &isid->context[0]);
+			cil_printf(")\n");
 		}
-		cil_printf("))\n");
 	}
+	free(isid_array);
 
-	rc = 0;
+	strs_free_all(strs);
+	strs_destroy(&strs);
 
-exit:
-	while(head) {
-		item = head;
-		head = item->next;
-		free(item->sid_key);
-		free(item);
-	}
-	return rc;
+	return 0;
 }
 
 static int ocontext_selinux_isid_to_cil(struct policydb *pdb, struct ocontext *isids)
-- 
2.50.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-11-07 16:04 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-04 20:52 [PATCH 1/2] libsepol: Fix sid handling when writing out policy from binary James Carter
2025-11-04 20:52 ` [PATCH 2/2] checkpolicy/tests: Modify tests to check handling of initial sids James Carter
2025-11-05 19:00   ` Stephen Smalley
2025-11-07 16:04     ` Stephen Smalley
2025-11-05 18:58 ` [PATCH 1/2] libsepol: Fix sid handling when writing out policy from binary Stephen Smalley

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).