| Message ID | 20200328124550.199568-3-omosnace@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | userspace: Implement new format of filename trans rules | expand |
On Sat, Mar 28, 2020 at 8:46 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > Implement a new, more space-efficient form of storing filename > transitions in the binary policy. The internal structures have already > been converted to this new representation; this patch just implements > reading/writing an equivalent representation from/to the binary policy. > > This new format reduces the size of Fedora policy from 7.6 MB to only > 3.3 MB (with policy optimization enabled in both cases). With the > unconfined module disabled, the size is reduced from 3.3 MB to 2.4 MB. > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> Acked-by: James Carter <jwcart2@gmail.com> > --- > libsepol/include/sepol/policydb/policydb.h | 3 +- > libsepol/src/policydb.c | 218 +++++++++++++++++---- > libsepol/src/write.c | 72 ++++++- > 3 files changed, 242 insertions(+), 51 deletions(-) > > diff --git a/libsepol/include/sepol/policydb/policydb.h b/libsepol/include/sepol/policydb/policydb.h > index c3180c61..9ef43abc 100644 > --- a/libsepol/include/sepol/policydb/policydb.h > +++ b/libsepol/include/sepol/policydb/policydb.h > @@ -755,10 +755,11 @@ extern int policydb_set_target_platform(policydb_t *p, int platform); > #define POLICYDB_VERSION_XPERMS_IOCTL 30 /* Linux-specific */ > #define POLICYDB_VERSION_INFINIBAND 31 /* Linux-specific */ > #define POLICYDB_VERSION_GLBLUB 32 > +#define POLICYDB_VERSION_COMP_FTRANS 33 /* compressed filename transitions */ > > /* Range of policy versions we understand*/ > #define POLICYDB_VERSION_MIN POLICYDB_VERSION_BASE > -#define POLICYDB_VERSION_MAX POLICYDB_VERSION_GLBLUB > +#define POLICYDB_VERSION_MAX POLICYDB_VERSION_COMP_FTRANS > > /* Module versions and specific changes*/ > #define MOD_POLICYDB_VERSION_BASE 4 > diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c > index 6b121d66..18ce1912 100644 > --- a/libsepol/src/policydb.c > +++ b/libsepol/src/policydb.c > @@ -200,6 +200,13 @@ static struct policydb_compat_info policydb_compat[] = { > .ocon_num = OCON_IBENDPORT + 1, > .target_platform = SEPOL_TARGET_SELINUX, > }, > + { > + .type = POLICY_KERN, > + .version = POLICYDB_VERSION_COMP_FTRANS, > + .sym_num = SYM_NUM, > + .ocon_num = OCON_IBENDPORT + 1, > + .target_platform = SEPOL_TARGET_SELINUX, > + }, > { > .type = POLICY_BASE, > .version = MOD_POLICYDB_VERSION_BASE, > @@ -2661,73 +2668,202 @@ int policydb_filetrans_insert(policydb_t *p, uint32_t stype, uint32_t ttype, > return ebitmap_set_bit(&datum->stypes, stype - 1, 1); > } > > -int filename_trans_read(policydb_t *p, struct policy_file *fp) > +static int filename_trans_read_one_old(policydb_t *p, struct policy_file *fp) > { > - unsigned int i; > - uint32_t buf[4], nel, len, stype, ttype, tclass, otype; > + uint32_t buf[4], len, stype, ttype, tclass, otype; > + char *name = NULL; > int rc; > - char *name; > > rc = next_entry(buf, fp, sizeof(uint32_t)); > if (rc < 0) > return -1; > - nel = le32_to_cpu(buf[0]); > + len = le32_to_cpu(buf[0]); > + if (zero_or_saturated(len)) > + return -1; > > - for (i = 0; i < nel; i++) { > - name = NULL; > + name = calloc(len + 1, sizeof(*name)); > + if (!name) > + return -1; > > - rc = next_entry(buf, fp, sizeof(uint32_t)); > - if (rc < 0) > - goto err; > - len = le32_to_cpu(buf[0]); > - if (zero_or_saturated(len)) > + rc = next_entry(name, fp, len); > + if (rc < 0) > + goto err; > + > + rc = next_entry(buf, fp, sizeof(uint32_t) * 4); > + if (rc < 0) > + goto err; > + > + stype = le32_to_cpu(buf[0]); > + ttype = le32_to_cpu(buf[1]); > + tclass = le32_to_cpu(buf[2]); > + otype = le32_to_cpu(buf[3]); > + > + rc = policydb_filetrans_insert(p, stype, ttype, tclass, name, &name, > + otype, NULL); > + if (rc) { > + if (rc != SEPOL_EEXIST) > goto err; > + /* > + * Some old policies were wrongly generated with > + * duplicate filename transition rules. For backward > + * compatibility, do not reject such policies, just > + * issue a warning and ignore the duplicate. > + */ > + WARN(fp->handle, > + "Duplicate name-based type_transition %s %s:%s \"%s\": %s, ignoring", > + p->p_type_val_to_name[stype - 1], > + p->p_type_val_to_name[ttype - 1], > + p->p_class_val_to_name[tclass - 1], > + name, > + p->p_type_val_to_name[otype - 1]); > + /* continue, ignoring this one */ > + } > + free(name); > + return 0; > +err: > + free(name); > + return -1; > +} > + > +static int filename_trans_check_datum(filename_trans_datum_t *datum) > +{ > + ebitmap_t stypes, otypes; > + > + ebitmap_init(&stypes); > + ebitmap_init(&otypes); > + > + while (datum) { > + if (ebitmap_get_bit(&otypes, datum->otype)) > + return -1; > + > + if (ebitmap_set_bit(&otypes, datum->otype, 1)) > + return -1; > > - name = calloc(len + 1, sizeof(*name)); > - if (!name) > + if (ebitmap_match_any(&stypes, &datum->stypes)) > + return -1; > + > + if (ebitmap_union(&stypes, &datum->stypes)) > + return -1; > + } > + return 0; > +} > + > +static int filename_trans_read_one_new(policydb_t *p, struct policy_file *fp) > +{ > + filename_trans_key_t *ft = NULL; > + filename_trans_datum_t **dst, *datum, *first = NULL; > + unsigned int i; > + uint32_t buf[3], len, ttype, tclass, ndatum; > + char *name = NULL; > + int rc; > + > + rc = next_entry(buf, fp, sizeof(uint32_t)); > + if (rc < 0) > + return -1; > + len = le32_to_cpu(buf[0]); > + if (zero_or_saturated(len)) > + return -1; > + > + name = calloc(len + 1, sizeof(*name)); > + if (!name) > + return -1; > + > + rc = next_entry(name, fp, len); > + if (rc < 0) > + goto err; > + > + rc = next_entry(buf, fp, sizeof(uint32_t) * 3); > + if (rc < 0) > + goto err; > + > + ttype = le32_to_cpu(buf[0]); > + tclass = le32_to_cpu(buf[1]); > + ndatum = le32_to_cpu(buf[2]); > + if (ndatum == 0) > + goto err; > + > + dst = &first; > + for (i = 0; i < ndatum; i++) { > + datum = malloc(sizeof(*datum)); > + if (!datum) > goto err; > > - rc = next_entry(name, fp, len); > + *dst = datum; > + > + /* ebitmap_read() will at least init the bitmap */ > + rc = ebitmap_read(&datum->stypes, fp); > if (rc < 0) > goto err; > > - rc = next_entry(buf, fp, sizeof(uint32_t) * 4); > + rc = next_entry(buf, fp, sizeof(uint32_t)); > if (rc < 0) > goto err; > > - stype = le32_to_cpu(buf[0]); > - ttype = le32_to_cpu(buf[1]); > - tclass = le32_to_cpu(buf[2]); > - otype = le32_to_cpu(buf[3]); > + datum->otype = le32_to_cpu(buf[0]); > > - rc = policydb_filetrans_insert(p, stype, ttype, tclass, name, > - &name, otype, NULL); > - if (rc) { > - if (rc != SEPOL_EEXIST) > - goto err; > - /* > - * Some old policies were wrongly generated with > - * duplicate filename transition rules. For backward > - * compatibility, do not reject such policies, just > - * issue a warning and ignore the duplicate. > - */ > - WARN(fp->handle, > - "Duplicate name-based type_transition %s %s:%s \"%s\": %s, ignoring", > - p->p_type_val_to_name[stype - 1], > - p->p_type_val_to_name[ttype - 1], > - p->p_class_val_to_name[tclass - 1], > - name, > - p->p_type_val_to_name[otype - 1]); > - /* continue, ignoring this one */ > - } > - free(name); > + p->filename_trans_count += ebitmap_cardinality(&datum->stypes); > + > + dst = &datum->next; > } > + *dst = NULL; > + > + if (ndatum > 1 && filename_trans_check_datum(first)) > + goto err; > + > + ft = malloc(sizeof(*ft)); > + if (!ft) > + goto err; > + > + ft->ttype = ttype; > + ft->tclass = tclass; > + ft->name = name; > + > + rc = hashtab_insert(p->filename_trans, (hashtab_key_t)ft, > + (hashtab_datum_t)first); > + if (rc) > + goto err; > + > return 0; > err: > + free(ft); > free(name); > + while (first) { > + datum = first; > + first = first->next; > + > + ebitmap_destroy(&datum->stypes); > + free(datum); > + } > return -1; > } > > +int filename_trans_read(policydb_t *p, struct policy_file *fp) > +{ > + unsigned int i; > + uint32_t buf[1], nel; > + int rc; > + > + rc = next_entry(buf, fp, sizeof(uint32_t)); > + if (rc < 0) > + return -1; > + nel = le32_to_cpu(buf[0]); > + > + if (p->policyvers < POLICYDB_VERSION_COMP_FTRANS) { > + for (i = 0; i < nel; i++) { > + rc = filename_trans_read_one_old(p, fp); > + if (rc < 0) > + return -1; > + } > + } else { > + for (i = 0; i < nel; i++) { > + rc = filename_trans_read_one_new(p, fp); > + if (rc < 0) > + return -1; > + } > + } > + return 0; > +} > + > static int ocontext_read_xen(struct policydb_compat_info *info, > policydb_t *p, struct policy_file *fp) > { > diff --git a/libsepol/src/write.c b/libsepol/src/write.c > index d3aee8d5..9c74d9f5 100644 > --- a/libsepol/src/write.c > +++ b/libsepol/src/write.c > @@ -569,7 +569,7 @@ static int role_allow_write(role_allow_t * r, struct policy_file *fp) > return POLICYDB_SUCCESS; > } > > -static int filename_write_helper(hashtab_key_t key, void *data, void *ptr) > +static int filename_write_one_old(hashtab_key_t key, void *data, void *ptr) > { > uint32_t bit, buf[4]; > size_t items, len; > @@ -605,6 +605,54 @@ static int filename_write_helper(hashtab_key_t key, void *data, void *ptr) > return 0; > } > > +static int filename_write_one_new(hashtab_key_t key, void *data, void *ptr) > +{ > + uint32_t buf[3]; > + size_t items, len, ndatum; > + filename_trans_key_t *ft = (filename_trans_key_t *)key; > + filename_trans_datum_t *datum; > + void *fp = ptr; > + > + len = strlen(ft->name); > + buf[0] = cpu_to_le32(len); > + items = put_entry(buf, sizeof(uint32_t), 1, fp); > + if (items != 1) > + return POLICYDB_ERROR; > + > + items = put_entry(ft->name, sizeof(char), len, fp); > + if (items != len) > + return POLICYDB_ERROR; > + > + ndatum = 0; > + datum = data; > + do { > + ndatum++; > + datum = datum->next; > + } while (datum); > + > + buf[0] = cpu_to_le32(ft->ttype); > + buf[1] = cpu_to_le32(ft->tclass); > + buf[2] = cpu_to_le32(ndatum); > + items = put_entry(buf, sizeof(uint32_t), 3, fp); > + if (items != 3) > + return POLICYDB_ERROR; > + > + datum = data; > + do { > + if (ebitmap_write(&datum->stypes, fp)) > + return POLICYDB_ERROR; > + > + buf[0] = cpu_to_le32(datum->otype); > + items = put_entry(buf, sizeof(uint32_t), 1, fp); > + if (items != 1) > + return POLICYDB_ERROR; > + > + datum = datum->next; > + } while (datum); > + > + return 0; > +} > + > static int filename_trans_write(struct policydb *p, void *fp) > { > size_t items; > @@ -614,16 +662,22 @@ static int filename_trans_write(struct policydb *p, void *fp) > if (p->policyvers < POLICYDB_VERSION_FILENAME_TRANS) > return 0; > > - buf[0] = cpu_to_le32(p->filename_trans_count); > - items = put_entry(buf, sizeof(uint32_t), 1, fp); > - if (items != 1) > - return POLICYDB_ERROR; > + if (p->policyvers < POLICYDB_VERSION_COMP_FTRANS) { > + buf[0] = cpu_to_le32(p->filename_trans_count); > + items = put_entry(buf, sizeof(uint32_t), 1, fp); > + if (items != 1) > + return POLICYDB_ERROR; > > - rc = hashtab_map(p->filename_trans, filename_write_helper, fp); > - if (rc) > - return rc; > + rc = hashtab_map(p->filename_trans, filename_write_one_old, fp); > + } else { > + buf[0] = cpu_to_le32(p->filename_trans->nel); > + items = put_entry(buf, sizeof(uint32_t), 1, fp); > + if (items != 1) > + return POLICYDB_ERROR; > > - return 0; > + rc = hashtab_map(p->filename_trans, filename_write_one_new, fp); > + } > + return rc; > } > > static int role_set_write(role_set_t * x, struct policy_file *fp) > -- > 2.25.1 >
On Tue, Mar 31, 2020 at 4:30 PM James Carter <jwcart2@gmail.com> wrote: > > On Sat, Mar 28, 2020 at 8:46 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > > Implement a new, more space-efficient form of storing filename > > transitions in the binary policy. The internal structures have already > > been converted to this new representation; this patch just implements > > reading/writing an equivalent representation from/to the binary policy. > > > > This new format reduces the size of Fedora policy from 7.6 MB to only > > 3.3 MB (with policy optimization enabled in both cases). With the > > unconfined module disabled, the size is reduced from 3.3 MB to 2.4 MB. > > > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > > Acked-by: James Carter <jwcart2@gmail.com> > Sorry, it has been a while. I was going to merge these today, but upon further testing, I noticed a couple of problems. First, creating a policy.conf from a binary like this: ~/local/usr/bin/checkpolicy -F -M -b -o policy.31.conf policy.31 give the following error: libsepol.avtab_read_item: invalid item count Second, creating a binary from a policy.conf and then trying to create a policy.conf from that binary: ~/local/usr/bin/checkpolicy -o policy.bin policy.conf ~/local/usr/bin/checkpolicy -F -b -o policy.bin.conf policy.bin gives the following error: security: ebitmap: high bit (32) is not a multiple of the map size (64) I don't know if this is related to the problems you had on the kernel side or not. I'll keep looking into this, but, for now, I'll have to remove my Acked-by. Jim > > --- > > libsepol/include/sepol/policydb/policydb.h | 3 +- > > libsepol/src/policydb.c | 218 +++++++++++++++++---- > > libsepol/src/write.c | 72 ++++++- > > 3 files changed, 242 insertions(+), 51 deletions(-) > > > > diff --git a/libsepol/include/sepol/policydb/policydb.h b/libsepol/include/sepol/policydb/policydb.h > > index c3180c61..9ef43abc 100644 > > --- a/libsepol/include/sepol/policydb/policydb.h > > +++ b/libsepol/include/sepol/policydb/policydb.h > > @@ -755,10 +755,11 @@ extern int policydb_set_target_platform(policydb_t *p, int platform); > > #define POLICYDB_VERSION_XPERMS_IOCTL 30 /* Linux-specific */ > > #define POLICYDB_VERSION_INFINIBAND 31 /* Linux-specific */ > > #define POLICYDB_VERSION_GLBLUB 32 > > +#define POLICYDB_VERSION_COMP_FTRANS 33 /* compressed filename transitions */ > > > > /* Range of policy versions we understand*/ > > #define POLICYDB_VERSION_MIN POLICYDB_VERSION_BASE > > -#define POLICYDB_VERSION_MAX POLICYDB_VERSION_GLBLUB > > +#define POLICYDB_VERSION_MAX POLICYDB_VERSION_COMP_FTRANS > > > > /* Module versions and specific changes*/ > > #define MOD_POLICYDB_VERSION_BASE 4 > > diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c > > index 6b121d66..18ce1912 100644 > > --- a/libsepol/src/policydb.c > > +++ b/libsepol/src/policydb.c > > @@ -200,6 +200,13 @@ static struct policydb_compat_info policydb_compat[] = { > > .ocon_num = OCON_IBENDPORT + 1, > > .target_platform = SEPOL_TARGET_SELINUX, > > }, > > + { > > + .type = POLICY_KERN, > > + .version = POLICYDB_VERSION_COMP_FTRANS, > > + .sym_num = SYM_NUM, > > + .ocon_num = OCON_IBENDPORT + 1, > > + .target_platform = SEPOL_TARGET_SELINUX, > > + }, > > { > > .type = POLICY_BASE, > > .version = MOD_POLICYDB_VERSION_BASE, > > @@ -2661,73 +2668,202 @@ int policydb_filetrans_insert(policydb_t *p, uint32_t stype, uint32_t ttype, > > return ebitmap_set_bit(&datum->stypes, stype - 1, 1); > > } > > > > -int filename_trans_read(policydb_t *p, struct policy_file *fp) > > +static int filename_trans_read_one_old(policydb_t *p, struct policy_file *fp) > > { > > - unsigned int i; > > - uint32_t buf[4], nel, len, stype, ttype, tclass, otype; > > + uint32_t buf[4], len, stype, ttype, tclass, otype; > > + char *name = NULL; > > int rc; > > - char *name; > > > > rc = next_entry(buf, fp, sizeof(uint32_t)); > > if (rc < 0) > > return -1; > > - nel = le32_to_cpu(buf[0]); > > + len = le32_to_cpu(buf[0]); > > + if (zero_or_saturated(len)) > > + return -1; > > > > - for (i = 0; i < nel; i++) { > > - name = NULL; > > + name = calloc(len + 1, sizeof(*name)); > > + if (!name) > > + return -1; > > > > - rc = next_entry(buf, fp, sizeof(uint32_t)); > > - if (rc < 0) > > - goto err; > > - len = le32_to_cpu(buf[0]); > > - if (zero_or_saturated(len)) > > + rc = next_entry(name, fp, len); > > + if (rc < 0) > > + goto err; > > + > > + rc = next_entry(buf, fp, sizeof(uint32_t) * 4); > > + if (rc < 0) > > + goto err; > > + > > + stype = le32_to_cpu(buf[0]); > > + ttype = le32_to_cpu(buf[1]); > > + tclass = le32_to_cpu(buf[2]); > > + otype = le32_to_cpu(buf[3]); > > + > > + rc = policydb_filetrans_insert(p, stype, ttype, tclass, name, &name, > > + otype, NULL); > > + if (rc) { > > + if (rc != SEPOL_EEXIST) > > goto err; > > + /* > > + * Some old policies were wrongly generated with > > + * duplicate filename transition rules. For backward > > + * compatibility, do not reject such policies, just > > + * issue a warning and ignore the duplicate. > > + */ > > + WARN(fp->handle, > > + "Duplicate name-based type_transition %s %s:%s \"%s\": %s, ignoring", > > + p->p_type_val_to_name[stype - 1], > > + p->p_type_val_to_name[ttype - 1], > > + p->p_class_val_to_name[tclass - 1], > > + name, > > + p->p_type_val_to_name[otype - 1]); > > + /* continue, ignoring this one */ > > + } > > + free(name); > > + return 0; > > +err: > > + free(name); > > + return -1; > > +} > > + > > +static int filename_trans_check_datum(filename_trans_datum_t *datum) > > +{ > > + ebitmap_t stypes, otypes; > > + > > + ebitmap_init(&stypes); > > + ebitmap_init(&otypes); > > + > > + while (datum) { > > + if (ebitmap_get_bit(&otypes, datum->otype)) > > + return -1; > > + > > + if (ebitmap_set_bit(&otypes, datum->otype, 1)) > > + return -1; > > > > - name = calloc(len + 1, sizeof(*name)); > > - if (!name) > > + if (ebitmap_match_any(&stypes, &datum->stypes)) > > + return -1; > > + > > + if (ebitmap_union(&stypes, &datum->stypes)) > > + return -1; > > + } > > + return 0; > > +} > > + > > +static int filename_trans_read_one_new(policydb_t *p, struct policy_file *fp) > > +{ > > + filename_trans_key_t *ft = NULL; > > + filename_trans_datum_t **dst, *datum, *first = NULL; > > + unsigned int i; > > + uint32_t buf[3], len, ttype, tclass, ndatum; > > + char *name = NULL; > > + int rc; > > + > > + rc = next_entry(buf, fp, sizeof(uint32_t)); > > + if (rc < 0) > > + return -1; > > + len = le32_to_cpu(buf[0]); > > + if (zero_or_saturated(len)) > > + return -1; > > + > > + name = calloc(len + 1, sizeof(*name)); > > + if (!name) > > + return -1; > > + > > + rc = next_entry(name, fp, len); > > + if (rc < 0) > > + goto err; > > + > > + rc = next_entry(buf, fp, sizeof(uint32_t) * 3); > > + if (rc < 0) > > + goto err; > > + > > + ttype = le32_to_cpu(buf[0]); > > + tclass = le32_to_cpu(buf[1]); > > + ndatum = le32_to_cpu(buf[2]); > > + if (ndatum == 0) > > + goto err; > > + > > + dst = &first; > > + for (i = 0; i < ndatum; i++) { > > + datum = malloc(sizeof(*datum)); > > + if (!datum) > > goto err; > > > > - rc = next_entry(name, fp, len); > > + *dst = datum; > > + > > + /* ebitmap_read() will at least init the bitmap */ > > + rc = ebitmap_read(&datum->stypes, fp); > > if (rc < 0) > > goto err; > > > > - rc = next_entry(buf, fp, sizeof(uint32_t) * 4); > > + rc = next_entry(buf, fp, sizeof(uint32_t)); > > if (rc < 0) > > goto err; > > > > - stype = le32_to_cpu(buf[0]); > > - ttype = le32_to_cpu(buf[1]); > > - tclass = le32_to_cpu(buf[2]); > > - otype = le32_to_cpu(buf[3]); > > + datum->otype = le32_to_cpu(buf[0]); > > > > - rc = policydb_filetrans_insert(p, stype, ttype, tclass, name, > > - &name, otype, NULL); > > - if (rc) { > > - if (rc != SEPOL_EEXIST) > > - goto err; > > - /* > > - * Some old policies were wrongly generated with > > - * duplicate filename transition rules. For backward > > - * compatibility, do not reject such policies, just > > - * issue a warning and ignore the duplicate. > > - */ > > - WARN(fp->handle, > > - "Duplicate name-based type_transition %s %s:%s \"%s\": %s, ignoring", > > - p->p_type_val_to_name[stype - 1], > > - p->p_type_val_to_name[ttype - 1], > > - p->p_class_val_to_name[tclass - 1], > > - name, > > - p->p_type_val_to_name[otype - 1]); > > - /* continue, ignoring this one */ > > - } > > - free(name); > > + p->filename_trans_count += ebitmap_cardinality(&datum->stypes); > > + > > + dst = &datum->next; > > } > > + *dst = NULL; > > + > > + if (ndatum > 1 && filename_trans_check_datum(first)) > > + goto err; > > + > > + ft = malloc(sizeof(*ft)); > > + if (!ft) > > + goto err; > > + > > + ft->ttype = ttype; > > + ft->tclass = tclass; > > + ft->name = name; > > + > > + rc = hashtab_insert(p->filename_trans, (hashtab_key_t)ft, > > + (hashtab_datum_t)first); > > + if (rc) > > + goto err; > > + > > return 0; > > err: > > + free(ft); > > free(name); > > + while (first) { > > + datum = first; > > + first = first->next; > > + > > + ebitmap_destroy(&datum->stypes); > > + free(datum); > > + } > > return -1; > > } > > > > +int filename_trans_read(policydb_t *p, struct policy_file *fp) > > +{ > > + unsigned int i; > > + uint32_t buf[1], nel; > > + int rc; > > + > > + rc = next_entry(buf, fp, sizeof(uint32_t)); > > + if (rc < 0) > > + return -1; > > + nel = le32_to_cpu(buf[0]); > > + > > + if (p->policyvers < POLICYDB_VERSION_COMP_FTRANS) { > > + for (i = 0; i < nel; i++) { > > + rc = filename_trans_read_one_old(p, fp); > > + if (rc < 0) > > + return -1; > > + } > > + } else { > > + for (i = 0; i < nel; i++) { > > + rc = filename_trans_read_one_new(p, fp); > > + if (rc < 0) > > + return -1; > > + } > > + } > > + return 0; > > +} > > + > > static int ocontext_read_xen(struct policydb_compat_info *info, > > policydb_t *p, struct policy_file *fp) > > { > > diff --git a/libsepol/src/write.c b/libsepol/src/write.c > > index d3aee8d5..9c74d9f5 100644 > > --- a/libsepol/src/write.c > > +++ b/libsepol/src/write.c > > @@ -569,7 +569,7 @@ static int role_allow_write(role_allow_t * r, struct policy_file *fp) > > return POLICYDB_SUCCESS; > > } > > > > -static int filename_write_helper(hashtab_key_t key, void *data, void *ptr) > > +static int filename_write_one_old(hashtab_key_t key, void *data, void *ptr) > > { > > uint32_t bit, buf[4]; > > size_t items, len; > > @@ -605,6 +605,54 @@ static int filename_write_helper(hashtab_key_t key, void *data, void *ptr) > > return 0; > > } > > > > +static int filename_write_one_new(hashtab_key_t key, void *data, void *ptr) > > +{ > > + uint32_t buf[3]; > > + size_t items, len, ndatum; > > + filename_trans_key_t *ft = (filename_trans_key_t *)key; > > + filename_trans_datum_t *datum; > > + void *fp = ptr; > > + > > + len = strlen(ft->name); > > + buf[0] = cpu_to_le32(len); > > + items = put_entry(buf, sizeof(uint32_t), 1, fp); > > + if (items != 1) > > + return POLICYDB_ERROR; > > + > > + items = put_entry(ft->name, sizeof(char), len, fp); > > + if (items != len) > > + return POLICYDB_ERROR; > > + > > + ndatum = 0; > > + datum = data; > > + do { > > + ndatum++; > > + datum = datum->next; > > + } while (datum); > > + > > + buf[0] = cpu_to_le32(ft->ttype); > > + buf[1] = cpu_to_le32(ft->tclass); > > + buf[2] = cpu_to_le32(ndatum); > > + items = put_entry(buf, sizeof(uint32_t), 3, fp); > > + if (items != 3) > > + return POLICYDB_ERROR; > > + > > + datum = data; > > + do { > > + if (ebitmap_write(&datum->stypes, fp)) > > + return POLICYDB_ERROR; > > + > > + buf[0] = cpu_to_le32(datum->otype); > > + items = put_entry(buf, sizeof(uint32_t), 1, fp); > > + if (items != 1) > > + return POLICYDB_ERROR; > > + > > + datum = datum->next; > > + } while (datum); > > + > > + return 0; > > +} > > + > > static int filename_trans_write(struct policydb *p, void *fp) > > { > > size_t items; > > @@ -614,16 +662,22 @@ static int filename_trans_write(struct policydb *p, void *fp) > > if (p->policyvers < POLICYDB_VERSION_FILENAME_TRANS) > > return 0; > > > > - buf[0] = cpu_to_le32(p->filename_trans_count); > > - items = put_entry(buf, sizeof(uint32_t), 1, fp); > > - if (items != 1) > > - return POLICYDB_ERROR; > > + if (p->policyvers < POLICYDB_VERSION_COMP_FTRANS) { > > + buf[0] = cpu_to_le32(p->filename_trans_count); > > + items = put_entry(buf, sizeof(uint32_t), 1, fp); > > + if (items != 1) > > + return POLICYDB_ERROR; > > > > - rc = hashtab_map(p->filename_trans, filename_write_helper, fp); > > - if (rc) > > - return rc; > > + rc = hashtab_map(p->filename_trans, filename_write_one_old, fp); > > + } else { > > + buf[0] = cpu_to_le32(p->filename_trans->nel); > > + items = put_entry(buf, sizeof(uint32_t), 1, fp); > > + if (items != 1) > > + return POLICYDB_ERROR; > > > > - return 0; > > + rc = hashtab_map(p->filename_trans, filename_write_one_new, fp); > > + } > > + return rc; > > } > > > > static int role_set_write(role_set_t * x, struct policy_file *fp) > > -- > > 2.25.1 > >
On Mon, Apr 27, 2020 at 9:39 PM James Carter <jwcart2@gmail.com> wrote: > On Tue, Mar 31, 2020 at 4:30 PM James Carter <jwcart2@gmail.com> wrote: > > > > On Sat, Mar 28, 2020 at 8:46 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > > > > Implement a new, more space-efficient form of storing filename > > > transitions in the binary policy. The internal structures have already > > > been converted to this new representation; this patch just implements > > > reading/writing an equivalent representation from/to the binary policy. > > > > > > This new format reduces the size of Fedora policy from 7.6 MB to only > > > 3.3 MB (with policy optimization enabled in both cases). With the > > > unconfined module disabled, the size is reduced from 3.3 MB to 2.4 MB. > > > > > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > > > > Acked-by: James Carter <jwcart2@gmail.com> > > > > Sorry, it has been a while. No problem. I would've merged it myself, I just wasn't sure if we shouldn't try to do something about the setools issue somehow... But I really don't feel like touching that code, so if the consensus is that this is worth the breakage then I'm fine with it :) > > I was going to merge these today, but upon further testing, I noticed > a couple of problems. > > First, creating a policy.conf from a binary like this: > ~/local/usr/bin/checkpolicy -F -M -b -o policy.31.conf policy.31 > give the following error: > libsepol.avtab_read_item: invalid item count Hm... I can't reproduce that on my side. Are you sure you're not using a broken policy generated by the buggy v1 of this series? > > Second, creating a binary from a policy.conf and then trying to create > a policy.conf from that binary: > ~/local/usr/bin/checkpolicy -o policy.bin policy.conf > ~/local/usr/bin/checkpolicy -F -b -o policy.bin.conf policy.bin > gives the following error: > security: ebitmap: high bit (32) is not a multiple of the map size (64) Likewise, couldn't reproduce it with the conf file I tried... Can you share the policy.conf that triggers this? > > I don't know if this is related to the problems you had on the kernel > side or not. I'll keep looking into this, but, for now, I'll have to > remove my Acked-by. > > Jim > > > > > --- > > > libsepol/include/sepol/policydb/policydb.h | 3 +- > > > libsepol/src/policydb.c | 218 +++++++++++++++++---- > > > libsepol/src/write.c | 72 ++++++- > > > 3 files changed, 242 insertions(+), 51 deletions(-) > > > > > > diff --git a/libsepol/include/sepol/policydb/policydb.h b/libsepol/include/sepol/policydb/policydb.h > > > index c3180c61..9ef43abc 100644 > > > --- a/libsepol/include/sepol/policydb/policydb.h > > > +++ b/libsepol/include/sepol/policydb/policydb.h > > > @@ -755,10 +755,11 @@ extern int policydb_set_target_platform(policydb_t *p, int platform); > > > #define POLICYDB_VERSION_XPERMS_IOCTL 30 /* Linux-specific */ > > > #define POLICYDB_VERSION_INFINIBAND 31 /* Linux-specific */ > > > #define POLICYDB_VERSION_GLBLUB 32 > > > +#define POLICYDB_VERSION_COMP_FTRANS 33 /* compressed filename transitions */ > > > > > > /* Range of policy versions we understand*/ > > > #define POLICYDB_VERSION_MIN POLICYDB_VERSION_BASE > > > -#define POLICYDB_VERSION_MAX POLICYDB_VERSION_GLBLUB > > > +#define POLICYDB_VERSION_MAX POLICYDB_VERSION_COMP_FTRANS > > > > > > /* Module versions and specific changes*/ > > > #define MOD_POLICYDB_VERSION_BASE 4 > > > diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c > > > index 6b121d66..18ce1912 100644 > > > --- a/libsepol/src/policydb.c > > > +++ b/libsepol/src/policydb.c > > > @@ -200,6 +200,13 @@ static struct policydb_compat_info policydb_compat[] = { > > > .ocon_num = OCON_IBENDPORT + 1, > > > .target_platform = SEPOL_TARGET_SELINUX, > > > }, > > > + { > > > + .type = POLICY_KERN, > > > + .version = POLICYDB_VERSION_COMP_FTRANS, > > > + .sym_num = SYM_NUM, > > > + .ocon_num = OCON_IBENDPORT + 1, > > > + .target_platform = SEPOL_TARGET_SELINUX, > > > + }, > > > { > > > .type = POLICY_BASE, > > > .version = MOD_POLICYDB_VERSION_BASE, > > > @@ -2661,73 +2668,202 @@ int policydb_filetrans_insert(policydb_t *p, uint32_t stype, uint32_t ttype, > > > return ebitmap_set_bit(&datum->stypes, stype - 1, 1); > > > } > > > > > > -int filename_trans_read(policydb_t *p, struct policy_file *fp) > > > +static int filename_trans_read_one_old(policydb_t *p, struct policy_file *fp) > > > { > > > - unsigned int i; > > > - uint32_t buf[4], nel, len, stype, ttype, tclass, otype; > > > + uint32_t buf[4], len, stype, ttype, tclass, otype; > > > + char *name = NULL; > > > int rc; > > > - char *name; > > > > > > rc = next_entry(buf, fp, sizeof(uint32_t)); > > > if (rc < 0) > > > return -1; > > > - nel = le32_to_cpu(buf[0]); > > > + len = le32_to_cpu(buf[0]); > > > + if (zero_or_saturated(len)) > > > + return -1; > > > > > > - for (i = 0; i < nel; i++) { > > > - name = NULL; > > > + name = calloc(len + 1, sizeof(*name)); > > > + if (!name) > > > + return -1; > > > > > > - rc = next_entry(buf, fp, sizeof(uint32_t)); > > > - if (rc < 0) > > > - goto err; > > > - len = le32_to_cpu(buf[0]); > > > - if (zero_or_saturated(len)) > > > + rc = next_entry(name, fp, len); > > > + if (rc < 0) > > > + goto err; > > > + > > > + rc = next_entry(buf, fp, sizeof(uint32_t) * 4); > > > + if (rc < 0) > > > + goto err; > > > + > > > + stype = le32_to_cpu(buf[0]); > > > + ttype = le32_to_cpu(buf[1]); > > > + tclass = le32_to_cpu(buf[2]); > > > + otype = le32_to_cpu(buf[3]); > > > + > > > + rc = policydb_filetrans_insert(p, stype, ttype, tclass, name, &name, > > > + otype, NULL); > > > + if (rc) { > > > + if (rc != SEPOL_EEXIST) > > > goto err; > > > + /* > > > + * Some old policies were wrongly generated with > > > + * duplicate filename transition rules. For backward > > > + * compatibility, do not reject such policies, just > > > + * issue a warning and ignore the duplicate. > > > + */ > > > + WARN(fp->handle, > > > + "Duplicate name-based type_transition %s %s:%s \"%s\": %s, ignoring", > > > + p->p_type_val_to_name[stype - 1], > > > + p->p_type_val_to_name[ttype - 1], > > > + p->p_class_val_to_name[tclass - 1], > > > + name, > > > + p->p_type_val_to_name[otype - 1]); > > > + /* continue, ignoring this one */ > > > + } > > > + free(name); > > > + return 0; > > > +err: > > > + free(name); > > > + return -1; > > > +} > > > + > > > +static int filename_trans_check_datum(filename_trans_datum_t *datum) > > > +{ > > > + ebitmap_t stypes, otypes; > > > + > > > + ebitmap_init(&stypes); > > > + ebitmap_init(&otypes); > > > + > > > + while (datum) { > > > + if (ebitmap_get_bit(&otypes, datum->otype)) > > > + return -1; > > > + > > > + if (ebitmap_set_bit(&otypes, datum->otype, 1)) > > > + return -1; > > > > > > - name = calloc(len + 1, sizeof(*name)); > > > - if (!name) > > > + if (ebitmap_match_any(&stypes, &datum->stypes)) > > > + return -1; > > > + > > > + if (ebitmap_union(&stypes, &datum->stypes)) > > > + return -1; > > > + } > > > + return 0; > > > +} > > > + > > > +static int filename_trans_read_one_new(policydb_t *p, struct policy_file *fp) > > > +{ > > > + filename_trans_key_t *ft = NULL; > > > + filename_trans_datum_t **dst, *datum, *first = NULL; > > > + unsigned int i; > > > + uint32_t buf[3], len, ttype, tclass, ndatum; > > > + char *name = NULL; > > > + int rc; > > > + > > > + rc = next_entry(buf, fp, sizeof(uint32_t)); > > > + if (rc < 0) > > > + return -1; > > > + len = le32_to_cpu(buf[0]); > > > + if (zero_or_saturated(len)) > > > + return -1; > > > + > > > + name = calloc(len + 1, sizeof(*name)); > > > + if (!name) > > > + return -1; > > > + > > > + rc = next_entry(name, fp, len); > > > + if (rc < 0) > > > + goto err; > > > + > > > + rc = next_entry(buf, fp, sizeof(uint32_t) * 3); > > > + if (rc < 0) > > > + goto err; > > > + > > > + ttype = le32_to_cpu(buf[0]); > > > + tclass = le32_to_cpu(buf[1]); > > > + ndatum = le32_to_cpu(buf[2]); > > > + if (ndatum == 0) > > > + goto err; > > > + > > > + dst = &first; > > > + for (i = 0; i < ndatum; i++) { > > > + datum = malloc(sizeof(*datum)); > > > + if (!datum) > > > goto err; > > > > > > - rc = next_entry(name, fp, len); > > > + *dst = datum; > > > + > > > + /* ebitmap_read() will at least init the bitmap */ > > > + rc = ebitmap_read(&datum->stypes, fp); > > > if (rc < 0) > > > goto err; > > > > > > - rc = next_entry(buf, fp, sizeof(uint32_t) * 4); > > > + rc = next_entry(buf, fp, sizeof(uint32_t)); > > > if (rc < 0) > > > goto err; > > > > > > - stype = le32_to_cpu(buf[0]); > > > - ttype = le32_to_cpu(buf[1]); > > > - tclass = le32_to_cpu(buf[2]); > > > - otype = le32_to_cpu(buf[3]); > > > + datum->otype = le32_to_cpu(buf[0]); > > > > > > - rc = policydb_filetrans_insert(p, stype, ttype, tclass, name, > > > - &name, otype, NULL); > > > - if (rc) { > > > - if (rc != SEPOL_EEXIST) > > > - goto err; > > > - /* > > > - * Some old policies were wrongly generated with > > > - * duplicate filename transition rules. For backward > > > - * compatibility, do not reject such policies, just > > > - * issue a warning and ignore the duplicate. > > > - */ > > > - WARN(fp->handle, > > > - "Duplicate name-based type_transition %s %s:%s \"%s\": %s, ignoring", > > > - p->p_type_val_to_name[stype - 1], > > > - p->p_type_val_to_name[ttype - 1], > > > - p->p_class_val_to_name[tclass - 1], > > > - name, > > > - p->p_type_val_to_name[otype - 1]); > > > - /* continue, ignoring this one */ > > > - } > > > - free(name); > > > + p->filename_trans_count += ebitmap_cardinality(&datum->stypes); > > > + > > > + dst = &datum->next; > > > } > > > + *dst = NULL; > > > + > > > + if (ndatum > 1 && filename_trans_check_datum(first)) > > > + goto err; > > > + > > > + ft = malloc(sizeof(*ft)); > > > + if (!ft) > > > + goto err; > > > + > > > + ft->ttype = ttype; > > > + ft->tclass = tclass; > > > + ft->name = name; > > > + > > > + rc = hashtab_insert(p->filename_trans, (hashtab_key_t)ft, > > > + (hashtab_datum_t)first); > > > + if (rc) > > > + goto err; > > > + > > > return 0; > > > err: > > > + free(ft); > > > free(name); > > > + while (first) { > > > + datum = first; > > > + first = first->next; > > > + > > > + ebitmap_destroy(&datum->stypes); > > > + free(datum); > > > + } > > > return -1; > > > } > > > > > > +int filename_trans_read(policydb_t *p, struct policy_file *fp) > > > +{ > > > + unsigned int i; > > > + uint32_t buf[1], nel; > > > + int rc; > > > + > > > + rc = next_entry(buf, fp, sizeof(uint32_t)); > > > + if (rc < 0) > > > + return -1; > > > + nel = le32_to_cpu(buf[0]); > > > + > > > + if (p->policyvers < POLICYDB_VERSION_COMP_FTRANS) { > > > + for (i = 0; i < nel; i++) { > > > + rc = filename_trans_read_one_old(p, fp); > > > + if (rc < 0) > > > + return -1; > > > + } > > > + } else { > > > + for (i = 0; i < nel; i++) { > > > + rc = filename_trans_read_one_new(p, fp); > > > + if (rc < 0) > > > + return -1; > > > + } > > > + } > > > + return 0; > > > +} > > > + > > > static int ocontext_read_xen(struct policydb_compat_info *info, > > > policydb_t *p, struct policy_file *fp) > > > { > > > diff --git a/libsepol/src/write.c b/libsepol/src/write.c > > > index d3aee8d5..9c74d9f5 100644 > > > --- a/libsepol/src/write.c > > > +++ b/libsepol/src/write.c > > > @@ -569,7 +569,7 @@ static int role_allow_write(role_allow_t * r, struct policy_file *fp) > > > return POLICYDB_SUCCESS; > > > } > > > > > > -static int filename_write_helper(hashtab_key_t key, void *data, void *ptr) > > > +static int filename_write_one_old(hashtab_key_t key, void *data, void *ptr) > > > { > > > uint32_t bit, buf[4]; > > > size_t items, len; > > > @@ -605,6 +605,54 @@ static int filename_write_helper(hashtab_key_t key, void *data, void *ptr) > > > return 0; > > > } > > > > > > +static int filename_write_one_new(hashtab_key_t key, void *data, void *ptr) > > > +{ > > > + uint32_t buf[3]; > > > + size_t items, len, ndatum; > > > + filename_trans_key_t *ft = (filename_trans_key_t *)key; > > > + filename_trans_datum_t *datum; > > > + void *fp = ptr; > > > + > > > + len = strlen(ft->name); > > > + buf[0] = cpu_to_le32(len); > > > + items = put_entry(buf, sizeof(uint32_t), 1, fp); > > > + if (items != 1) > > > + return POLICYDB_ERROR; > > > + > > > + items = put_entry(ft->name, sizeof(char), len, fp); > > > + if (items != len) > > > + return POLICYDB_ERROR; > > > + > > > + ndatum = 0; > > > + datum = data; > > > + do { > > > + ndatum++; > > > + datum = datum->next; > > > + } while (datum); > > > + > > > + buf[0] = cpu_to_le32(ft->ttype); > > > + buf[1] = cpu_to_le32(ft->tclass); > > > + buf[2] = cpu_to_le32(ndatum); > > > + items = put_entry(buf, sizeof(uint32_t), 3, fp); > > > + if (items != 3) > > > + return POLICYDB_ERROR; > > > + > > > + datum = data; > > > + do { > > > + if (ebitmap_write(&datum->stypes, fp)) > > > + return POLICYDB_ERROR; > > > + > > > + buf[0] = cpu_to_le32(datum->otype); > > > + items = put_entry(buf, sizeof(uint32_t), 1, fp); > > > + if (items != 1) > > > + return POLICYDB_ERROR; > > > + > > > + datum = datum->next; > > > + } while (datum); > > > + > > > + return 0; > > > +} > > > + > > > static int filename_trans_write(struct policydb *p, void *fp) > > > { > > > size_t items; > > > @@ -614,16 +662,22 @@ static int filename_trans_write(struct policydb *p, void *fp) > > > if (p->policyvers < POLICYDB_VERSION_FILENAME_TRANS) > > > return 0; > > > > > > - buf[0] = cpu_to_le32(p->filename_trans_count); > > > - items = put_entry(buf, sizeof(uint32_t), 1, fp); > > > - if (items != 1) > > > - return POLICYDB_ERROR; > > > + if (p->policyvers < POLICYDB_VERSION_COMP_FTRANS) { > > > + buf[0] = cpu_to_le32(p->filename_trans_count); > > > + items = put_entry(buf, sizeof(uint32_t), 1, fp); > > > + if (items != 1) > > > + return POLICYDB_ERROR; > > > > > > - rc = hashtab_map(p->filename_trans, filename_write_helper, fp); > > > - if (rc) > > > - return rc; > > > + rc = hashtab_map(p->filename_trans, filename_write_one_old, fp); > > > + } else { > > > + buf[0] = cpu_to_le32(p->filename_trans->nel); > > > + items = put_entry(buf, sizeof(uint32_t), 1, fp); > > > + if (items != 1) > > > + return POLICYDB_ERROR; > > > > > > - return 0; > > > + rc = hashtab_map(p->filename_trans, filename_write_one_new, fp); > > > + } > > > + return rc; > > > } > > > > > > static int role_set_write(role_set_t * x, struct policy_file *fp) > > > -- > > > 2.25.1 > > > > -- Ondrej Mosnacek <omosnace at redhat dot com> Software Engineer, Security Technologies Red Hat, Inc.
On Tue, Apr 28, 2020 at 4:49 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > On Mon, Apr 27, 2020 at 9:39 PM James Carter <jwcart2@gmail.com> wrote: > > On Tue, Mar 31, 2020 at 4:30 PM James Carter <jwcart2@gmail.com> wrote: > > > > > > On Sat, Mar 28, 2020 at 8:46 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > > > > > > Implement a new, more space-efficient form of storing filename > > > > transitions in the binary policy. The internal structures have already > > > > been converted to this new representation; this patch just implements > > > > reading/writing an equivalent representation from/to the binary policy. > > > > > > > > This new format reduces the size of Fedora policy from 7.6 MB to only > > > > 3.3 MB (with policy optimization enabled in both cases). With the > > > > unconfined module disabled, the size is reduced from 3.3 MB to 2.4 MB. > > > > > > > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > > > > > > Acked-by: James Carter <jwcart2@gmail.com> > > > > > > > Sorry, it has been a while. > > No problem. I would've merged it myself, I just wasn't sure if we > shouldn't try to do something about the setools issue somehow... But I > really don't feel like touching that code, so if the consensus is that > this is worth the breakage then I'm fine with it :) > I thought the consensus was to apply this now, but it would be nice to not break setools, so I am going to hold off merging for day or two and take a look at how much work it would be to export what setools needs. > > > > I was going to merge these today, but upon further testing, I noticed > > a couple of problems. > > > > First, creating a policy.conf from a binary like this: > > ~/local/usr/bin/checkpolicy -F -M -b -o policy.31.conf policy.31 > > give the following error: > > libsepol.avtab_read_item: invalid item count > > Hm... I can't reproduce that on my side. Are you sure you're not using > a broken policy generated by the buggy v1 of this series? > No, it wasn't that. It turns out that I didn't do "make clean" between building master and building the branch with this patch series. I don't see any problems now. Jim > > > > Second, creating a binary from a policy.conf and then trying to create > > a policy.conf from that binary: > > ~/local/usr/bin/checkpolicy -o policy.bin policy.conf > > ~/local/usr/bin/checkpolicy -F -b -o policy.bin.conf policy.bin > > gives the following error: > > security: ebitmap: high bit (32) is not a multiple of the map size (64) > > Likewise, couldn't reproduce it with the conf file I tried... Can you > share the policy.conf that triggers this? > > > > > I don't know if this is related to the problems you had on the kernel > > side or not. I'll keep looking into this, but, for now, I'll have to > > remove my Acked-by. > > > > Jim > > > > > > > > --- > > > > libsepol/include/sepol/policydb/policydb.h | 3 +- > > > > libsepol/src/policydb.c | 218 +++++++++++++++++---- > > > > libsepol/src/write.c | 72 ++++++- > > > > 3 files changed, 242 insertions(+), 51 deletions(-) > > > > > > > > diff --git a/libsepol/include/sepol/policydb/policydb.h b/libsepol/include/sepol/policydb/policydb.h > > > > index c3180c61..9ef43abc 100644 > > > > --- a/libsepol/include/sepol/policydb/policydb.h > > > > +++ b/libsepol/include/sepol/policydb/policydb.h > > > > @@ -755,10 +755,11 @@ extern int policydb_set_target_platform(policydb_t *p, int platform); > > > > #define POLICYDB_VERSION_XPERMS_IOCTL 30 /* Linux-specific */ > > > > #define POLICYDB_VERSION_INFINIBAND 31 /* Linux-specific */ > > > > #define POLICYDB_VERSION_GLBLUB 32 > > > > +#define POLICYDB_VERSION_COMP_FTRANS 33 /* compressed filename transitions */ > > > > > > > > /* Range of policy versions we understand*/ > > > > #define POLICYDB_VERSION_MIN POLICYDB_VERSION_BASE > > > > -#define POLICYDB_VERSION_MAX POLICYDB_VERSION_GLBLUB > > > > +#define POLICYDB_VERSION_MAX POLICYDB_VERSION_COMP_FTRANS > > > > > > > > /* Module versions and specific changes*/ > > > > #define MOD_POLICYDB_VERSION_BASE 4 > > > > diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c > > > > index 6b121d66..18ce1912 100644 > > > > --- a/libsepol/src/policydb.c > > > > +++ b/libsepol/src/policydb.c > > > > @@ -200,6 +200,13 @@ static struct policydb_compat_info policydb_compat[] = { > > > > .ocon_num = OCON_IBENDPORT + 1, > > > > .target_platform = SEPOL_TARGET_SELINUX, > > > > }, > > > > + { > > > > + .type = POLICY_KERN, > > > > + .version = POLICYDB_VERSION_COMP_FTRANS, > > > > + .sym_num = SYM_NUM, > > > > + .ocon_num = OCON_IBENDPORT + 1, > > > > + .target_platform = SEPOL_TARGET_SELINUX, > > > > + }, > > > > { > > > > .type = POLICY_BASE, > > > > .version = MOD_POLICYDB_VERSION_BASE, > > > > @@ -2661,73 +2668,202 @@ int policydb_filetrans_insert(policydb_t *p, uint32_t stype, uint32_t ttype, > > > > return ebitmap_set_bit(&datum->stypes, stype - 1, 1); > > > > } > > > > > > > > -int filename_trans_read(policydb_t *p, struct policy_file *fp) > > > > +static int filename_trans_read_one_old(policydb_t *p, struct policy_file *fp) > > > > { > > > > - unsigned int i; > > > > - uint32_t buf[4], nel, len, stype, ttype, tclass, otype; > > > > + uint32_t buf[4], len, stype, ttype, tclass, otype; > > > > + char *name = NULL; > > > > int rc; > > > > - char *name; > > > > > > > > rc = next_entry(buf, fp, sizeof(uint32_t)); > > > > if (rc < 0) > > > > return -1; > > > > - nel = le32_to_cpu(buf[0]); > > > > + len = le32_to_cpu(buf[0]); > > > > + if (zero_or_saturated(len)) > > > > + return -1; > > > > > > > > - for (i = 0; i < nel; i++) { > > > > - name = NULL; > > > > + name = calloc(len + 1, sizeof(*name)); > > > > + if (!name) > > > > + return -1; > > > > > > > > - rc = next_entry(buf, fp, sizeof(uint32_t)); > > > > - if (rc < 0) > > > > - goto err; > > > > - len = le32_to_cpu(buf[0]); > > > > - if (zero_or_saturated(len)) > > > > + rc = next_entry(name, fp, len); > > > > + if (rc < 0) > > > > + goto err; > > > > + > > > > + rc = next_entry(buf, fp, sizeof(uint32_t) * 4); > > > > + if (rc < 0) > > > > + goto err; > > > > + > > > > + stype = le32_to_cpu(buf[0]); > > > > + ttype = le32_to_cpu(buf[1]); > > > > + tclass = le32_to_cpu(buf[2]); > > > > + otype = le32_to_cpu(buf[3]); > > > > + > > > > + rc = policydb_filetrans_insert(p, stype, ttype, tclass, name, &name, > > > > + otype, NULL); > > > > + if (rc) { > > > > + if (rc != SEPOL_EEXIST) > > > > goto err; > > > > + /* > > > > + * Some old policies were wrongly generated with > > > > + * duplicate filename transition rules. For backward > > > > + * compatibility, do not reject such policies, just > > > > + * issue a warning and ignore the duplicate. > > > > + */ > > > > + WARN(fp->handle, > > > > + "Duplicate name-based type_transition %s %s:%s \"%s\": %s, ignoring", > > > > + p->p_type_val_to_name[stype - 1], > > > > + p->p_type_val_to_name[ttype - 1], > > > > + p->p_class_val_to_name[tclass - 1], > > > > + name, > > > > + p->p_type_val_to_name[otype - 1]); > > > > + /* continue, ignoring this one */ > > > > + } > > > > + free(name); > > > > + return 0; > > > > +err: > > > > + free(name); > > > > + return -1; > > > > +} > > > > + > > > > +static int filename_trans_check_datum(filename_trans_datum_t *datum) > > > > +{ > > > > + ebitmap_t stypes, otypes; > > > > + > > > > + ebitmap_init(&stypes); > > > > + ebitmap_init(&otypes); > > > > + > > > > + while (datum) { > > > > + if (ebitmap_get_bit(&otypes, datum->otype)) > > > > + return -1; > > > > + > > > > + if (ebitmap_set_bit(&otypes, datum->otype, 1)) > > > > + return -1; > > > > > > > > - name = calloc(len + 1, sizeof(*name)); > > > > - if (!name) > > > > + if (ebitmap_match_any(&stypes, &datum->stypes)) > > > > + return -1; > > > > + > > > > + if (ebitmap_union(&stypes, &datum->stypes)) > > > > + return -1; > > > > + } > > > > + return 0; > > > > +} > > > > + > > > > +static int filename_trans_read_one_new(policydb_t *p, struct policy_file *fp) > > > > +{ > > > > + filename_trans_key_t *ft = NULL; > > > > + filename_trans_datum_t **dst, *datum, *first = NULL; > > > > + unsigned int i; > > > > + uint32_t buf[3], len, ttype, tclass, ndatum; > > > > + char *name = NULL; > > > > + int rc; > > > > + > > > > + rc = next_entry(buf, fp, sizeof(uint32_t)); > > > > + if (rc < 0) > > > > + return -1; > > > > + len = le32_to_cpu(buf[0]); > > > > + if (zero_or_saturated(len)) > > > > + return -1; > > > > + > > > > + name = calloc(len + 1, sizeof(*name)); > > > > + if (!name) > > > > + return -1; > > > > + > > > > + rc = next_entry(name, fp, len); > > > > + if (rc < 0) > > > > + goto err; > > > > + > > > > + rc = next_entry(buf, fp, sizeof(uint32_t) * 3); > > > > + if (rc < 0) > > > > + goto err; > > > > + > > > > + ttype = le32_to_cpu(buf[0]); > > > > + tclass = le32_to_cpu(buf[1]); > > > > + ndatum = le32_to_cpu(buf[2]); > > > > + if (ndatum == 0) > > > > + goto err; > > > > + > > > > + dst = &first; > > > > + for (i = 0; i < ndatum; i++) { > > > > + datum = malloc(sizeof(*datum)); > > > > + if (!datum) > > > > goto err; > > > > > > > > - rc = next_entry(name, fp, len); > > > > + *dst = datum; > > > > + > > > > + /* ebitmap_read() will at least init the bitmap */ > > > > + rc = ebitmap_read(&datum->stypes, fp); > > > > if (rc < 0) > > > > goto err; > > > > > > > > - rc = next_entry(buf, fp, sizeof(uint32_t) * 4); > > > > + rc = next_entry(buf, fp, sizeof(uint32_t)); > > > > if (rc < 0) > > > > goto err; > > > > > > > > - stype = le32_to_cpu(buf[0]); > > > > - ttype = le32_to_cpu(buf[1]); > > > > - tclass = le32_to_cpu(buf[2]); > > > > - otype = le32_to_cpu(buf[3]); > > > > + datum->otype = le32_to_cpu(buf[0]); > > > > > > > > - rc = policydb_filetrans_insert(p, stype, ttype, tclass, name, > > > > - &name, otype, NULL); > > > > - if (rc) { > > > > - if (rc != SEPOL_EEXIST) > > > > - goto err; > > > > - /* > > > > - * Some old policies were wrongly generated with > > > > - * duplicate filename transition rules. For backward > > > > - * compatibility, do not reject such policies, just > > > > - * issue a warning and ignore the duplicate. > > > > - */ > > > > - WARN(fp->handle, > > > > - "Duplicate name-based type_transition %s %s:%s \"%s\": %s, ignoring", > > > > - p->p_type_val_to_name[stype - 1], > > > > - p->p_type_val_to_name[ttype - 1], > > > > - p->p_class_val_to_name[tclass - 1], > > > > - name, > > > > - p->p_type_val_to_name[otype - 1]); > > > > - /* continue, ignoring this one */ > > > > - } > > > > - free(name); > > > > + p->filename_trans_count += ebitmap_cardinality(&datum->stypes); > > > > + > > > > + dst = &datum->next; > > > > } > > > > + *dst = NULL; > > > > + > > > > + if (ndatum > 1 && filename_trans_check_datum(first)) > > > > + goto err; > > > > + > > > > + ft = malloc(sizeof(*ft)); > > > > + if (!ft) > > > > + goto err; > > > > + > > > > + ft->ttype = ttype; > > > > + ft->tclass = tclass; > > > > + ft->name = name; > > > > + > > > > + rc = hashtab_insert(p->filename_trans, (hashtab_key_t)ft, > > > > + (hashtab_datum_t)first); > > > > + if (rc) > > > > + goto err; > > > > + > > > > return 0; > > > > err: > > > > + free(ft); > > > > free(name); > > > > + while (first) { > > > > + datum = first; > > > > + first = first->next; > > > > + > > > > + ebitmap_destroy(&datum->stypes); > > > > + free(datum); > > > > + } > > > > return -1; > > > > } > > > > > > > > +int filename_trans_read(policydb_t *p, struct policy_file *fp) > > > > +{ > > > > + unsigned int i; > > > > + uint32_t buf[1], nel; > > > > + int rc; > > > > + > > > > + rc = next_entry(buf, fp, sizeof(uint32_t)); > > > > + if (rc < 0) > > > > + return -1; > > > > + nel = le32_to_cpu(buf[0]); > > > > + > > > > + if (p->policyvers < POLICYDB_VERSION_COMP_FTRANS) { > > > > + for (i = 0; i < nel; i++) { > > > > + rc = filename_trans_read_one_old(p, fp); > > > > + if (rc < 0) > > > > + return -1; > > > > + } > > > > + } else { > > > > + for (i = 0; i < nel; i++) { > > > > + rc = filename_trans_read_one_new(p, fp); > > > > + if (rc < 0) > > > > + return -1; > > > > + } > > > > + } > > > > + return 0; > > > > +} > > > > + > > > > static int ocontext_read_xen(struct policydb_compat_info *info, > > > > policydb_t *p, struct policy_file *fp) > > > > { > > > > diff --git a/libsepol/src/write.c b/libsepol/src/write.c > > > > index d3aee8d5..9c74d9f5 100644 > > > > --- a/libsepol/src/write.c > > > > +++ b/libsepol/src/write.c > > > > @@ -569,7 +569,7 @@ static int role_allow_write(role_allow_t * r, struct policy_file *fp) > > > > return POLICYDB_SUCCESS; > > > > } > > > > > > > > -static int filename_write_helper(hashtab_key_t key, void *data, void *ptr) > > > > +static int filename_write_one_old(hashtab_key_t key, void *data, void *ptr) > > > > { > > > > uint32_t bit, buf[4]; > > > > size_t items, len; > > > > @@ -605,6 +605,54 @@ static int filename_write_helper(hashtab_key_t key, void *data, void *ptr) > > > > return 0; > > > > } > > > > > > > > +static int filename_write_one_new(hashtab_key_t key, void *data, void *ptr) > > > > +{ > > > > + uint32_t buf[3]; > > > > + size_t items, len, ndatum; > > > > + filename_trans_key_t *ft = (filename_trans_key_t *)key; > > > > + filename_trans_datum_t *datum; > > > > + void *fp = ptr; > > > > + > > > > + len = strlen(ft->name); > > > > + buf[0] = cpu_to_le32(len); > > > > + items = put_entry(buf, sizeof(uint32_t), 1, fp); > > > > + if (items != 1) > > > > + return POLICYDB_ERROR; > > > > + > > > > + items = put_entry(ft->name, sizeof(char), len, fp); > > > > + if (items != len) > > > > + return POLICYDB_ERROR; > > > > + > > > > + ndatum = 0; > > > > + datum = data; > > > > + do { > > > > + ndatum++; > > > > + datum = datum->next; > > > > + } while (datum); > > > > + > > > > + buf[0] = cpu_to_le32(ft->ttype); > > > > + buf[1] = cpu_to_le32(ft->tclass); > > > > + buf[2] = cpu_to_le32(ndatum); > > > > + items = put_entry(buf, sizeof(uint32_t), 3, fp); > > > > + if (items != 3) > > > > + return POLICYDB_ERROR; > > > > + > > > > + datum = data; > > > > + do { > > > > + if (ebitmap_write(&datum->stypes, fp)) > > > > + return POLICYDB_ERROR; > > > > + > > > > + buf[0] = cpu_to_le32(datum->otype); > > > > + items = put_entry(buf, sizeof(uint32_t), 1, fp); > > > > + if (items != 1) > > > > + return POLICYDB_ERROR; > > > > + > > > > + datum = datum->next; > > > > + } while (datum); > > > > + > > > > + return 0; > > > > +} > > > > + > > > > static int filename_trans_write(struct policydb *p, void *fp) > > > > { > > > > size_t items; > > > > @@ -614,16 +662,22 @@ static int filename_trans_write(struct policydb *p, void *fp) > > > > if (p->policyvers < POLICYDB_VERSION_FILENAME_TRANS) > > > > return 0; > > > > > > > > - buf[0] = cpu_to_le32(p->filename_trans_count); > > > > - items = put_entry(buf, sizeof(uint32_t), 1, fp); > > > > - if (items != 1) > > > > - return POLICYDB_ERROR; > > > > + if (p->policyvers < POLICYDB_VERSION_COMP_FTRANS) { > > > > + buf[0] = cpu_to_le32(p->filename_trans_count); > > > > + items = put_entry(buf, sizeof(uint32_t), 1, fp); > > > > + if (items != 1) > > > > + return POLICYDB_ERROR; > > > > > > > > - rc = hashtab_map(p->filename_trans, filename_write_helper, fp); > > > > - if (rc) > > > > - return rc; > > > > + rc = hashtab_map(p->filename_trans, filename_write_one_old, fp); > > > > + } else { > > > > + buf[0] = cpu_to_le32(p->filename_trans->nel); > > > > + items = put_entry(buf, sizeof(uint32_t), 1, fp); > > > > + if (items != 1) > > > > + return POLICYDB_ERROR; > > > > > > > > - return 0; > > > > + rc = hashtab_map(p->filename_trans, filename_write_one_new, fp); > > > > + } > > > > + return rc; > > > > } > > > > > > > > static int role_set_write(role_set_t * x, struct policy_file *fp) > > > > -- > > > > 2.25.1 > > > > > > > > > -- > Ondrej Mosnacek <omosnace at redhat dot com> > Software Engineer, Security Technologies > Red Hat, Inc. >
On Tue, Apr 28, 2020 at 11:29 AM James Carter <jwcart2@gmail.com> wrote: > > On Tue, Apr 28, 2020 at 4:49 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > > On Mon, Apr 27, 2020 at 9:39 PM James Carter <jwcart2@gmail.com> wrote: > > > On Tue, Mar 31, 2020 at 4:30 PM James Carter <jwcart2@gmail.com> wrote: > > > > > > > > On Sat, Mar 28, 2020 at 8:46 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > > > > > > > > Implement a new, more space-efficient form of storing filename > > > > > transitions in the binary policy. The internal structures have already > > > > > been converted to this new representation; this patch just implements > > > > > reading/writing an equivalent representation from/to the binary policy. > > > > > > > > > > This new format reduces the size of Fedora policy from 7.6 MB to only > > > > > 3.3 MB (with policy optimization enabled in both cases). With the > > > > > unconfined module disabled, the size is reduced from 3.3 MB to 2.4 MB. > > > > > > > > > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > > > > > > > > Acked-by: James Carter <jwcart2@gmail.com> > > > > > > > > > > Sorry, it has been a while. > > > > No problem. I would've merged it myself, I just wasn't sure if we > > shouldn't try to do something about the setools issue somehow... But I > > really don't feel like touching that code, so if the consensus is that > > this is worth the breakage then I'm fine with it :) > > > > I thought the consensus was to apply this now, but it would be nice to > not break setools, so I am going to hold off merging for day or two > and take a look at how much work it would be to export what setools > needs. There was an earlier discussion about whether we ought to make another selinux userspace release soon. If so, we should defer applying this change until after that release to avoid breaking setools with no fix in sight for it.
On Thu, Apr 30, 2020 at 10:25 AM Stephen Smalley <stephen.smalley.work@gmail.com> wrote: > > On Tue, Apr 28, 2020 at 11:29 AM James Carter <jwcart2@gmail.com> wrote: > > > > On Tue, Apr 28, 2020 at 4:49 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > > > > On Mon, Apr 27, 2020 at 9:39 PM James Carter <jwcart2@gmail.com> wrote: > > > > On Tue, Mar 31, 2020 at 4:30 PM James Carter <jwcart2@gmail.com> wrote: > > > > > > > > > > On Sat, Mar 28, 2020 at 8:46 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > > > > > > > > > > > Implement a new, more space-efficient form of storing filename > > > > > > transitions in the binary policy. The internal structures have already > > > > > > been converted to this new representation; this patch just implements > > > > > > reading/writing an equivalent representation from/to the binary policy. > > > > > > > > > > > > This new format reduces the size of Fedora policy from 7.6 MB to only > > > > > > 3.3 MB (with policy optimization enabled in both cases). With the > > > > > > unconfined module disabled, the size is reduced from 3.3 MB to 2.4 MB. > > > > > > > > > > > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > > > > > > > > > > Acked-by: James Carter <jwcart2@gmail.com> > > > > > > > > > > > > > Sorry, it has been a while. > > > > > > No problem. I would've merged it myself, I just wasn't sure if we > > > shouldn't try to do something about the setools issue somehow... But I > > > really don't feel like touching that code, so if the consensus is that > > > this is worth the breakage then I'm fine with it :) > > > > > > > I thought the consensus was to apply this now, but it would be nice to > > not break setools, so I am going to hold off merging for day or two > > and take a look at how much work it would be to export what setools > > needs. > > There was an earlier discussion about whether we ought to make another > selinux userspace release soon. > If so, we should defer applying this change until after that release > to avoid breaking setools with no fix > in sight for it. I agree. That discussion was what got me really thinking about what to do about setools. Jim
diff --git a/libsepol/include/sepol/policydb/policydb.h b/libsepol/include/sepol/policydb/policydb.h index c3180c61..9ef43abc 100644 --- a/libsepol/include/sepol/policydb/policydb.h +++ b/libsepol/include/sepol/policydb/policydb.h @@ -755,10 +755,11 @@ extern int policydb_set_target_platform(policydb_t *p, int platform); #define POLICYDB_VERSION_XPERMS_IOCTL 30 /* Linux-specific */ #define POLICYDB_VERSION_INFINIBAND 31 /* Linux-specific */ #define POLICYDB_VERSION_GLBLUB 32 +#define POLICYDB_VERSION_COMP_FTRANS 33 /* compressed filename transitions */ /* Range of policy versions we understand*/ #define POLICYDB_VERSION_MIN POLICYDB_VERSION_BASE -#define POLICYDB_VERSION_MAX POLICYDB_VERSION_GLBLUB +#define POLICYDB_VERSION_MAX POLICYDB_VERSION_COMP_FTRANS /* Module versions and specific changes*/ #define MOD_POLICYDB_VERSION_BASE 4 diff --git a/libsepol/src/policydb.c b/libsepol/src/policydb.c index 6b121d66..18ce1912 100644 --- a/libsepol/src/policydb.c +++ b/libsepol/src/policydb.c @@ -200,6 +200,13 @@ static struct policydb_compat_info policydb_compat[] = { .ocon_num = OCON_IBENDPORT + 1, .target_platform = SEPOL_TARGET_SELINUX, }, + { + .type = POLICY_KERN, + .version = POLICYDB_VERSION_COMP_FTRANS, + .sym_num = SYM_NUM, + .ocon_num = OCON_IBENDPORT + 1, + .target_platform = SEPOL_TARGET_SELINUX, + }, { .type = POLICY_BASE, .version = MOD_POLICYDB_VERSION_BASE, @@ -2661,73 +2668,202 @@ int policydb_filetrans_insert(policydb_t *p, uint32_t stype, uint32_t ttype, return ebitmap_set_bit(&datum->stypes, stype - 1, 1); } -int filename_trans_read(policydb_t *p, struct policy_file *fp) +static int filename_trans_read_one_old(policydb_t *p, struct policy_file *fp) { - unsigned int i; - uint32_t buf[4], nel, len, stype, ttype, tclass, otype; + uint32_t buf[4], len, stype, ttype, tclass, otype; + char *name = NULL; int rc; - char *name; rc = next_entry(buf, fp, sizeof(uint32_t)); if (rc < 0) return -1; - nel = le32_to_cpu(buf[0]); + len = le32_to_cpu(buf[0]); + if (zero_or_saturated(len)) + return -1; - for (i = 0; i < nel; i++) { - name = NULL; + name = calloc(len + 1, sizeof(*name)); + if (!name) + return -1; - rc = next_entry(buf, fp, sizeof(uint32_t)); - if (rc < 0) - goto err; - len = le32_to_cpu(buf[0]); - if (zero_or_saturated(len)) + rc = next_entry(name, fp, len); + if (rc < 0) + goto err; + + rc = next_entry(buf, fp, sizeof(uint32_t) * 4); + if (rc < 0) + goto err; + + stype = le32_to_cpu(buf[0]); + ttype = le32_to_cpu(buf[1]); + tclass = le32_to_cpu(buf[2]); + otype = le32_to_cpu(buf[3]); + + rc = policydb_filetrans_insert(p, stype, ttype, tclass, name, &name, + otype, NULL); + if (rc) { + if (rc != SEPOL_EEXIST) goto err; + /* + * Some old policies were wrongly generated with + * duplicate filename transition rules. For backward + * compatibility, do not reject such policies, just + * issue a warning and ignore the duplicate. + */ + WARN(fp->handle, + "Duplicate name-based type_transition %s %s:%s \"%s\": %s, ignoring", + p->p_type_val_to_name[stype - 1], + p->p_type_val_to_name[ttype - 1], + p->p_class_val_to_name[tclass - 1], + name, + p->p_type_val_to_name[otype - 1]); + /* continue, ignoring this one */ + } + free(name); + return 0; +err: + free(name); + return -1; +} + +static int filename_trans_check_datum(filename_trans_datum_t *datum) +{ + ebitmap_t stypes, otypes; + + ebitmap_init(&stypes); + ebitmap_init(&otypes); + + while (datum) { + if (ebitmap_get_bit(&otypes, datum->otype)) + return -1; + + if (ebitmap_set_bit(&otypes, datum->otype, 1)) + return -1; - name = calloc(len + 1, sizeof(*name)); - if (!name) + if (ebitmap_match_any(&stypes, &datum->stypes)) + return -1; + + if (ebitmap_union(&stypes, &datum->stypes)) + return -1; + } + return 0; +} + +static int filename_trans_read_one_new(policydb_t *p, struct policy_file *fp) +{ + filename_trans_key_t *ft = NULL; + filename_trans_datum_t **dst, *datum, *first = NULL; + unsigned int i; + uint32_t buf[3], len, ttype, tclass, ndatum; + char *name = NULL; + int rc; + + rc = next_entry(buf, fp, sizeof(uint32_t)); + if (rc < 0) + return -1; + len = le32_to_cpu(buf[0]); + if (zero_or_saturated(len)) + return -1; + + name = calloc(len + 1, sizeof(*name)); + if (!name) + return -1; + + rc = next_entry(name, fp, len); + if (rc < 0) + goto err; + + rc = next_entry(buf, fp, sizeof(uint32_t) * 3); + if (rc < 0) + goto err; + + ttype = le32_to_cpu(buf[0]); + tclass = le32_to_cpu(buf[1]); + ndatum = le32_to_cpu(buf[2]); + if (ndatum == 0) + goto err; + + dst = &first; + for (i = 0; i < ndatum; i++) { + datum = malloc(sizeof(*datum)); + if (!datum) goto err; - rc = next_entry(name, fp, len); + *dst = datum; + + /* ebitmap_read() will at least init the bitmap */ + rc = ebitmap_read(&datum->stypes, fp); if (rc < 0) goto err; - rc = next_entry(buf, fp, sizeof(uint32_t) * 4); + rc = next_entry(buf, fp, sizeof(uint32_t)); if (rc < 0) goto err; - stype = le32_to_cpu(buf[0]); - ttype = le32_to_cpu(buf[1]); - tclass = le32_to_cpu(buf[2]); - otype = le32_to_cpu(buf[3]); + datum->otype = le32_to_cpu(buf[0]); - rc = policydb_filetrans_insert(p, stype, ttype, tclass, name, - &name, otype, NULL); - if (rc) { - if (rc != SEPOL_EEXIST) - goto err; - /* - * Some old policies were wrongly generated with - * duplicate filename transition rules. For backward - * compatibility, do not reject such policies, just - * issue a warning and ignore the duplicate. - */ - WARN(fp->handle, - "Duplicate name-based type_transition %s %s:%s \"%s\": %s, ignoring", - p->p_type_val_to_name[stype - 1], - p->p_type_val_to_name[ttype - 1], - p->p_class_val_to_name[tclass - 1], - name, - p->p_type_val_to_name[otype - 1]); - /* continue, ignoring this one */ - } - free(name); + p->filename_trans_count += ebitmap_cardinality(&datum->stypes); + + dst = &datum->next; } + *dst = NULL; + + if (ndatum > 1 && filename_trans_check_datum(first)) + goto err; + + ft = malloc(sizeof(*ft)); + if (!ft) + goto err; + + ft->ttype = ttype; + ft->tclass = tclass; + ft->name = name; + + rc = hashtab_insert(p->filename_trans, (hashtab_key_t)ft, + (hashtab_datum_t)first); + if (rc) + goto err; + return 0; err: + free(ft); free(name); + while (first) { + datum = first; + first = first->next; + + ebitmap_destroy(&datum->stypes); + free(datum); + } return -1; } +int filename_trans_read(policydb_t *p, struct policy_file *fp) +{ + unsigned int i; + uint32_t buf[1], nel; + int rc; + + rc = next_entry(buf, fp, sizeof(uint32_t)); + if (rc < 0) + return -1; + nel = le32_to_cpu(buf[0]); + + if (p->policyvers < POLICYDB_VERSION_COMP_FTRANS) { + for (i = 0; i < nel; i++) { + rc = filename_trans_read_one_old(p, fp); + if (rc < 0) + return -1; + } + } else { + for (i = 0; i < nel; i++) { + rc = filename_trans_read_one_new(p, fp); + if (rc < 0) + return -1; + } + } + return 0; +} + static int ocontext_read_xen(struct policydb_compat_info *info, policydb_t *p, struct policy_file *fp) { diff --git a/libsepol/src/write.c b/libsepol/src/write.c index d3aee8d5..9c74d9f5 100644 --- a/libsepol/src/write.c +++ b/libsepol/src/write.c @@ -569,7 +569,7 @@ static int role_allow_write(role_allow_t * r, struct policy_file *fp) return POLICYDB_SUCCESS; } -static int filename_write_helper(hashtab_key_t key, void *data, void *ptr) +static int filename_write_one_old(hashtab_key_t key, void *data, void *ptr) { uint32_t bit, buf[4]; size_t items, len; @@ -605,6 +605,54 @@ static int filename_write_helper(hashtab_key_t key, void *data, void *ptr) return 0; } +static int filename_write_one_new(hashtab_key_t key, void *data, void *ptr) +{ + uint32_t buf[3]; + size_t items, len, ndatum; + filename_trans_key_t *ft = (filename_trans_key_t *)key; + filename_trans_datum_t *datum; + void *fp = ptr; + + len = strlen(ft->name); + buf[0] = cpu_to_le32(len); + items = put_entry(buf, sizeof(uint32_t), 1, fp); + if (items != 1) + return POLICYDB_ERROR; + + items = put_entry(ft->name, sizeof(char), len, fp); + if (items != len) + return POLICYDB_ERROR; + + ndatum = 0; + datum = data; + do { + ndatum++; + datum = datum->next; + } while (datum); + + buf[0] = cpu_to_le32(ft->ttype); + buf[1] = cpu_to_le32(ft->tclass); + buf[2] = cpu_to_le32(ndatum); + items = put_entry(buf, sizeof(uint32_t), 3, fp); + if (items != 3) + return POLICYDB_ERROR; + + datum = data; + do { + if (ebitmap_write(&datum->stypes, fp)) + return POLICYDB_ERROR; + + buf[0] = cpu_to_le32(datum->otype); + items = put_entry(buf, sizeof(uint32_t), 1, fp); + if (items != 1) + return POLICYDB_ERROR; + + datum = datum->next; + } while (datum); + + return 0; +} + static int filename_trans_write(struct policydb *p, void *fp) { size_t items; @@ -614,16 +662,22 @@ static int filename_trans_write(struct policydb *p, void *fp) if (p->policyvers < POLICYDB_VERSION_FILENAME_TRANS) return 0; - buf[0] = cpu_to_le32(p->filename_trans_count); - items = put_entry(buf, sizeof(uint32_t), 1, fp); - if (items != 1) - return POLICYDB_ERROR; + if (p->policyvers < POLICYDB_VERSION_COMP_FTRANS) { + buf[0] = cpu_to_le32(p->filename_trans_count); + items = put_entry(buf, sizeof(uint32_t), 1, fp); + if (items != 1) + return POLICYDB_ERROR; - rc = hashtab_map(p->filename_trans, filename_write_helper, fp); - if (rc) - return rc; + rc = hashtab_map(p->filename_trans, filename_write_one_old, fp); + } else { + buf[0] = cpu_to_le32(p->filename_trans->nel); + items = put_entry(buf, sizeof(uint32_t), 1, fp); + if (items != 1) + return POLICYDB_ERROR; - return 0; + rc = hashtab_map(p->filename_trans, filename_write_one_new, fp); + } + return rc; } static int role_set_write(role_set_t * x, struct policy_file *fp)
Implement a new, more space-efficient form of storing filename transitions in the binary policy. The internal structures have already been converted to this new representation; this patch just implements reading/writing an equivalent representation from/to the binary policy. This new format reduces the size of Fedora policy from 7.6 MB to only 3.3 MB (with policy optimization enabled in both cases). With the unconfined module disabled, the size is reduced from 3.3 MB to 2.4 MB. Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> --- libsepol/include/sepol/policydb/policydb.h | 3 +- libsepol/src/policydb.c | 218 +++++++++++++++++---- libsepol/src/write.c | 72 ++++++- 3 files changed, 242 insertions(+), 51 deletions(-)