Re: [PATCH 1/2] libsepol: Fix sid handling when writing out policy from binary

On Tue, Nov 4, 2025 at 3:52 PM James Carter <[email protected]> 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 <[email protected]>

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 <[email protected]>
Tested-by: Stephen Smalley <[email protected]>

[1] https://lore.kernel.org/selinux-refpolicy/[email protected]/

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