| Message ID | 20200420132731.1085015-1-omosnace@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | selinux: don't produce incorrect filename_trans_count | expand |
On Mon, Apr 20, 2020 at 9:27 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > I thought I fixed the counting in filename_trans_read_helper() to count > the compat rule count correctly in the final version, but it's still > wrong. To really count the same thing as in the compat path, we'd need > to add up the cardinalities of stype bitmaps of all datums. > > Since the kernel currently doesn't implement an ebitmap_cardinality() > function (and computing the proper count would just waste CPU cycles > anyway), just document that we use the field only in case of the old > format and stop updating it in filename_trans_read_helper(). > > Fixes: 430059024389 ("selinux: implement new format of filename transitions") > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > --- > security/selinux/ss/policydb.c | 11 +++-------- > security/selinux/ss/policydb.h | 3 ++- > 2 files changed, 5 insertions(+), 9 deletions(-) Seems reasonable. Merged into selinux/next.
diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index ef8d5b46b05b..1c0041576643 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -2016,12 +2016,7 @@ static int filename_trans_read_helper(struct policydb *p, void *fp) if (rc) goto out; - rc = ebitmap_set_bit(&p->filename_trans_ttypes, ttype, 1); - if (rc) - return rc; - - p->filename_trans_count += ndatum; - return 0; + return ebitmap_set_bit(&p->filename_trans_ttypes, ttype, 1); out: kfree(ft); @@ -2051,7 +2046,7 @@ static int filename_trans_read(struct policydb *p, void *fp) nel = le32_to_cpu(buf[0]); if (p->policyvers < POLICYDB_VERSION_COMP_FTRANS) { - p->filename_trans_count = nel; + p->compat_filename_trans_count = nel; p->filename_trans = hashtab_create(filenametr_hash, filenametr_cmp, (1 << 11)); if (!p->filename_trans) @@ -3568,7 +3563,7 @@ static int filename_trans_write(struct policydb *p, void *fp) return 0; if (p->policyvers < POLICYDB_VERSION_COMP_FTRANS) { - buf[0] = cpu_to_le32(p->filename_trans_count); + buf[0] = cpu_to_le32(p->compat_filename_trans_count); rc = put_entry(buf, sizeof(u32), 1, fp); if (rc) return rc; diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h index d3adb522d3f3..35dc6aa7904d 100644 --- a/security/selinux/ss/policydb.h +++ b/security/selinux/ss/policydb.h @@ -270,7 +270,8 @@ struct policydb { struct ebitmap filename_trans_ttypes; /* actual set of filename_trans rules */ struct hashtab *filename_trans; - u32 filename_trans_count; + /* only used if policyvers < POLICYDB_VERSION_COMP_FTRANS */ + u32 compat_filename_trans_count; /* bools indexed by (value - 1) */ struct cond_bool_datum **bool_val_to_struct;
I thought I fixed the counting in filename_trans_read_helper() to count the compat rule count correctly in the final version, but it's still wrong. To really count the same thing as in the compat path, we'd need to add up the cardinalities of stype bitmaps of all datums. Since the kernel currently doesn't implement an ebitmap_cardinality() function (and computing the proper count would just waste CPU cycles anyway), just document that we use the field only in case of the old format and stop updating it in filename_trans_read_helper(). Fixes: 430059024389 ("selinux: implement new format of filename transitions") Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> --- security/selinux/ss/policydb.c | 11 +++-------- security/selinux/ss/policydb.h | 3 ++- 2 files changed, 5 insertions(+), 9 deletions(-)