* Re: [PATCH 1/2] libsepol: Fix sid handling when writing out policy from binary
[not found] <20251104205236.60931-1-jwcart2@gmail.com>
@ 2025-11-05 18:58 ` Stephen Smalley
[not found] ` <20251104205236.60931-2-jwcart2@gmail.com>
1 sibling, 0 replies; 3+ messages in thread
From: Stephen Smalley @ 2025-11-05 18:58 UTC (permalink / raw)
To: James Carter; +Cc: selinux, russell, Chris PeBenito, selinux-refpolicy
On Tue, Nov 4, 2025 at 3:52 PM James Carter <jwcart2@gmail.com> wrote:
>
> 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>
With this applied, I confirmed that building refpolicy with the patch
in [1] did not disturb the initial SID index values.
I also saw no regressions wrt existing policy.
Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Tested-by: Stephen Smalley <stephen.smalley.work@gmail.com>
[1] https://lore.kernel.org/selinux-refpolicy/20251030200720.18719-2-stephen.smalley.work@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 [flat|nested] 3+ messages in thread