| Message ID | 20200212112255.105678-2-omosnace@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Optimize storage of filename transitions | expand |
On 2/12/20 6:22 AM, Ondrej Mosnacek wrote: > It simplifies cleanup in the error path. This will be extra useful in > later patch. > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> Acked-by: Stephen Smalley <sds@tycho.nsa.gov> > --- > security/selinux/ss/policydb.c | 122 +++++++++++++++++---------------- > 1 file changed, 63 insertions(+), 59 deletions(-) > > diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c > index 2aa7f2e1a8e7..981797bfc547 100644 > --- a/security/selinux/ss/policydb.c > +++ b/security/selinux/ss/policydb.c > @@ -1880,88 +1880,92 @@ out: > return rc; > } > > -static int filename_trans_read(struct policydb *p, void *fp) > +static int filename_trans_read_one(struct policydb *p, void *fp) > { > struct filename_trans *ft; > - struct filename_trans_datum *otype; > - char *name; > - u32 nel, len; > + struct filename_trans_datum *otype = NULL; > + char *name = NULL; > + u32 len; > __le32 buf[4]; > - int rc, i; > + int rc; > > - if (p->policyvers < POLICYDB_VERSION_FILENAME_TRANS) > - return 0; > + ft = kzalloc(sizeof(*ft), GFP_KERNEL); > + if (!ft) > + return -ENOMEM; > + > + rc = -ENOMEM; > + otype = kmalloc(sizeof(*otype), GFP_KERNEL); > + if (!otype) > + goto out; > > + /* length of the path component string */ > rc = next_entry(buf, fp, sizeof(u32)); > if (rc) > - return rc; > - nel = le32_to_cpu(buf[0]); > - > - for (i = 0; i < nel; i++) { > - otype = NULL; > - name = NULL; > - > - rc = -ENOMEM; > - ft = kzalloc(sizeof(*ft), GFP_KERNEL); > - if (!ft) > - goto out; > - > - rc = -ENOMEM; > - otype = kmalloc(sizeof(*otype), GFP_KERNEL); > - if (!otype) > - goto out; > - > - /* length of the path component string */ > - rc = next_entry(buf, fp, sizeof(u32)); > - if (rc) > - goto out; > - len = le32_to_cpu(buf[0]); > + goto out; > + len = le32_to_cpu(buf[0]); > > - /* path component string */ > - rc = str_read(&name, GFP_KERNEL, fp, len); > - if (rc) > - goto out; > + /* path component string */ > + rc = str_read(&name, GFP_KERNEL, fp, len); > + if (rc) > + goto out; > > - ft->name = name; > + ft->name = name; > > - rc = next_entry(buf, fp, sizeof(u32) * 4); > - if (rc) > - goto out; > + rc = next_entry(buf, fp, sizeof(u32) * 4); > + if (rc) > + goto out; > > - ft->stype = le32_to_cpu(buf[0]); > - ft->ttype = le32_to_cpu(buf[1]); > - ft->tclass = le32_to_cpu(buf[2]); > + ft->stype = le32_to_cpu(buf[0]); > + ft->ttype = le32_to_cpu(buf[1]); > + ft->tclass = le32_to_cpu(buf[2]); > > - otype->otype = le32_to_cpu(buf[3]); > + otype->otype = le32_to_cpu(buf[3]); > > - rc = ebitmap_set_bit(&p->filename_trans_ttypes, ft->ttype, 1); > - if (rc) > - goto out; > + rc = ebitmap_set_bit(&p->filename_trans_ttypes, ft->ttype, 1); > + if (rc) > + goto out; > > - rc = hashtab_insert(p->filename_trans, ft, otype); > - if (rc) { > - /* > - * Do not return -EEXIST to the caller, or the system > - * will not boot. > - */ > - if (rc != -EEXIST) > - goto out; > - /* But free memory to avoid memory leak. */ > - kfree(ft); > - kfree(name); > - kfree(otype); > - } > + rc = hashtab_insert(p->filename_trans, ft, otype); > + if (rc) { > + /* > + * Do not return -EEXIST to the caller, or the system > + * will not boot. > + */ > + if (rc == -EEXIST) > + rc = 0; > + goto out; > } > - hash_eval(p->filename_trans, "filenametr"); > return 0; > out: > kfree(ft); > kfree(name); > kfree(otype); > - > return rc; > } > > +static int filename_trans_read(struct policydb *p, void *fp) > +{ > + u32 nel; > + __le32 buf[1]; > + int rc, i; > + > + if (p->policyvers < POLICYDB_VERSION_FILENAME_TRANS) > + return 0; > + > + rc = next_entry(buf, fp, sizeof(u32)); > + if (rc) > + return rc; > + nel = le32_to_cpu(buf[0]); > + > + for (i = 0; i < nel; i++) { > + rc = filename_trans_read_one(p, fp); > + if (rc) > + return rc; > + } > + hash_eval(p->filename_trans, "filenametr"); > + return 0; > +} > + > static int genfs_read(struct policydb *p, void *fp) > { > int i, j, rc; >
On Wed, Feb 12, 2020 at 6:23 AM Ondrej Mosnacek <omosnace@redhat.com> wrote: > > It simplifies cleanup in the error path. This will be extra useful in > later patch. > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> > --- > security/selinux/ss/policydb.c | 122 +++++++++++++++++---------------- > 1 file changed, 63 insertions(+), 59 deletions(-) Merged into selinux/next.
diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c index 2aa7f2e1a8e7..981797bfc547 100644 --- a/security/selinux/ss/policydb.c +++ b/security/selinux/ss/policydb.c @@ -1880,88 +1880,92 @@ out: return rc; } -static int filename_trans_read(struct policydb *p, void *fp) +static int filename_trans_read_one(struct policydb *p, void *fp) { struct filename_trans *ft; - struct filename_trans_datum *otype; - char *name; - u32 nel, len; + struct filename_trans_datum *otype = NULL; + char *name = NULL; + u32 len; __le32 buf[4]; - int rc, i; + int rc; - if (p->policyvers < POLICYDB_VERSION_FILENAME_TRANS) - return 0; + ft = kzalloc(sizeof(*ft), GFP_KERNEL); + if (!ft) + return -ENOMEM; + + rc = -ENOMEM; + otype = kmalloc(sizeof(*otype), GFP_KERNEL); + if (!otype) + goto out; + /* length of the path component string */ rc = next_entry(buf, fp, sizeof(u32)); if (rc) - return rc; - nel = le32_to_cpu(buf[0]); - - for (i = 0; i < nel; i++) { - otype = NULL; - name = NULL; - - rc = -ENOMEM; - ft = kzalloc(sizeof(*ft), GFP_KERNEL); - if (!ft) - goto out; - - rc = -ENOMEM; - otype = kmalloc(sizeof(*otype), GFP_KERNEL); - if (!otype) - goto out; - - /* length of the path component string */ - rc = next_entry(buf, fp, sizeof(u32)); - if (rc) - goto out; - len = le32_to_cpu(buf[0]); + goto out; + len = le32_to_cpu(buf[0]); - /* path component string */ - rc = str_read(&name, GFP_KERNEL, fp, len); - if (rc) - goto out; + /* path component string */ + rc = str_read(&name, GFP_KERNEL, fp, len); + if (rc) + goto out; - ft->name = name; + ft->name = name; - rc = next_entry(buf, fp, sizeof(u32) * 4); - if (rc) - goto out; + rc = next_entry(buf, fp, sizeof(u32) * 4); + if (rc) + goto out; - ft->stype = le32_to_cpu(buf[0]); - ft->ttype = le32_to_cpu(buf[1]); - ft->tclass = le32_to_cpu(buf[2]); + ft->stype = le32_to_cpu(buf[0]); + ft->ttype = le32_to_cpu(buf[1]); + ft->tclass = le32_to_cpu(buf[2]); - otype->otype = le32_to_cpu(buf[3]); + otype->otype = le32_to_cpu(buf[3]); - rc = ebitmap_set_bit(&p->filename_trans_ttypes, ft->ttype, 1); - if (rc) - goto out; + rc = ebitmap_set_bit(&p->filename_trans_ttypes, ft->ttype, 1); + if (rc) + goto out; - rc = hashtab_insert(p->filename_trans, ft, otype); - if (rc) { - /* - * Do not return -EEXIST to the caller, or the system - * will not boot. - */ - if (rc != -EEXIST) - goto out; - /* But free memory to avoid memory leak. */ - kfree(ft); - kfree(name); - kfree(otype); - } + rc = hashtab_insert(p->filename_trans, ft, otype); + if (rc) { + /* + * Do not return -EEXIST to the caller, or the system + * will not boot. + */ + if (rc == -EEXIST) + rc = 0; + goto out; } - hash_eval(p->filename_trans, "filenametr"); return 0; out: kfree(ft); kfree(name); kfree(otype); - return rc; } +static int filename_trans_read(struct policydb *p, void *fp) +{ + u32 nel; + __le32 buf[1]; + int rc, i; + + if (p->policyvers < POLICYDB_VERSION_FILENAME_TRANS) + return 0; + + rc = next_entry(buf, fp, sizeof(u32)); + if (rc) + return rc; + nel = le32_to_cpu(buf[0]); + + for (i = 0; i < nel; i++) { + rc = filename_trans_read_one(p, fp); + if (rc) + return rc; + } + hash_eval(p->filename_trans, "filenametr"); + return 0; +} + static int genfs_read(struct policydb *p, void *fp) { int i, j, rc;
It simplifies cleanup in the error path. This will be extra useful in later patch. Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com> --- security/selinux/ss/policydb.c | 122 +++++++++++++++++---------------- 1 file changed, 63 insertions(+), 59 deletions(-)