| Message ID | 20240131130840.48155-6-cgzones@googlemail.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Petr Lautrbach |
| Headers | show |
| Series | libselinux: rework selabel_file(5) database | expand |
On Wed, Jan 31, 2024 at 8:18 AM Christian Göttsche <cgzones@googlemail.com> wrote: > > Add sidtab_context_lookup() to just lookup a context, not inserting > non-existent ones. > > Tweak sidtab_destroy() to accept a zero'ed struct sidtab. > > Remove redundant lookup in sidtab_context_to_sid() after insertion by > returning the newly created node directly from sidtab_insert(). > > Drop declaration of only internal used sidtab_insert(). > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > --- > v2: > add patch > --- > libselinux/src/avc_sidtab.c | 55 +++++++++++++++++++++---------------- > libselinux/src/avc_sidtab.h | 2 +- > 2 files changed, 32 insertions(+), 25 deletions(-) > > diff --git a/libselinux/src/avc_sidtab.c b/libselinux/src/avc_sidtab.c > index 9475dcb0..3d347cea 100644 > --- a/libselinux/src/avc_sidtab.c > +++ b/libselinux/src/avc_sidtab.c > @@ -44,28 +44,23 @@ int sidtab_init(struct sidtab *s) > return rc; > } > > -int sidtab_insert(struct sidtab *s, const char * ctx) > +static struct sidtab_node * > +sidtab_insert(struct sidtab *s, const char * ctx) > { > unsigned hvalue; > - int rc = 0; > struct sidtab_node *newnode; > char * newctx; > > - if (s->nel >= UINT_MAX - 1) { > - rc = -1; > - goto out; > - } > + if (s->nel >= UINT_MAX - 1) > + return NULL; > > newnode = (struct sidtab_node *)avc_malloc(sizeof(*newnode)); > - if (!newnode) { > - rc = -1; > - goto out; > - } > + if (!newnode) > + return NULL; > newctx = strdup(ctx); > if (!newctx) { > - rc = -1; > avc_free(newnode); > - goto out; > + return NULL; > } > > hvalue = sidtab_hash(newctx); > @@ -73,8 +68,25 @@ int sidtab_insert(struct sidtab *s, const char * ctx) > newnode->sid_s.ctx = newctx; > newnode->sid_s.id = ++s->nel; > s->htable[hvalue] = newnode; > - out: > - return rc; > + return newnode; > +} > + > +const struct security_id * > +sidtab_context_lookup(const struct sidtab *s, const char *ctx) > +{ > + unsigned hvalue; > + const struct sidtab_node *cur; > + > + hvalue = sidtab_hash(ctx); > + > + cur = s->htable[hvalue]; > + while (cur != NULL && strcmp(cur->sid_s.ctx, ctx)) > + cur = cur->next; > + > + if (cur == NULL) > + return NULL; > + > + return &cur->sid_s; > } > > int > @@ -82,27 +94,23 @@ sidtab_context_to_sid(struct sidtab *s, > const char * ctx, security_id_t * sid) > { > unsigned hvalue; > - int rc = 0; > struct sidtab_node *cur; > > *sid = NULL; > hvalue = sidtab_hash(ctx); > > - loop: > cur = s->htable[hvalue]; > while (cur != NULL && strcmp(cur->sid_s.ctx, ctx)) > cur = cur->next; > > if (cur == NULL) { /* need to make a new entry */ > - rc = sidtab_insert(s, ctx); > - if (rc) > - goto out; > - goto loop; /* find the newly inserted node */ > + cur = sidtab_insert(s, ctx); > + if (cur == NULL) > + return -1; > } > > *sid = &cur->sid_s; > - out: > - return rc; > + return 0; > } > This duplicates the sidtab_context_lookup() code above, so why not just use that. If that returns NULL, then insert the context. Thanks, Jim > void sidtab_sid_stats(const struct sidtab *s, char *buf, size_t buflen) > @@ -138,7 +146,7 @@ void sidtab_destroy(struct sidtab *s) > int i; > struct sidtab_node *cur, *temp; > > - if (!s) > + if (!s || !s->htable) > return; > > for (i = 0; i < SIDTAB_SIZE; i++) { > @@ -149,7 +157,6 @@ void sidtab_destroy(struct sidtab *s) > freecon(temp->sid_s.ctx); > avc_free(temp); > } > - s->htable[i] = NULL; > } > avc_free(s->htable); > s->htable = NULL; > diff --git a/libselinux/src/avc_sidtab.h b/libselinux/src/avc_sidtab.h > index e823e3f3..f62fd353 100644 > --- a/libselinux/src/avc_sidtab.h > +++ b/libselinux/src/avc_sidtab.h > @@ -24,8 +24,8 @@ struct sidtab { > }; > > int sidtab_init(struct sidtab *s) ; > -int sidtab_insert(struct sidtab *s, const char * ctx) ; > > +const struct security_id * sidtab_context_lookup(const struct sidtab *s, const char *ctx); > int sidtab_context_to_sid(struct sidtab *s, > const char * ctx, security_id_t * sid) ; > > -- > 2.43.0 > >
On Thu, 7 Mar 2024 at 21:53, James Carter <jwcart2@gmail.com> wrote: > > On Wed, Jan 31, 2024 at 8:18 AM Christian Göttsche > <cgzones@googlemail.com> wrote: > > > > Add sidtab_context_lookup() to just lookup a context, not inserting > > non-existent ones. > > > > Tweak sidtab_destroy() to accept a zero'ed struct sidtab. > > > > Remove redundant lookup in sidtab_context_to_sid() after insertion by > > returning the newly created node directly from sidtab_insert(). > > > > Drop declaration of only internal used sidtab_insert(). > > > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com> > > --- > > v2: > > add patch > > --- > > libselinux/src/avc_sidtab.c | 55 +++++++++++++++++++++---------------- > > libselinux/src/avc_sidtab.h | 2 +- > > 2 files changed, 32 insertions(+), 25 deletions(-) > > > > diff --git a/libselinux/src/avc_sidtab.c b/libselinux/src/avc_sidtab.c > > index 9475dcb0..3d347cea 100644 > > --- a/libselinux/src/avc_sidtab.c > > +++ b/libselinux/src/avc_sidtab.c > > @@ -44,28 +44,23 @@ int sidtab_init(struct sidtab *s) > > return rc; > > } > > > > -int sidtab_insert(struct sidtab *s, const char * ctx) > > +static struct sidtab_node * > > +sidtab_insert(struct sidtab *s, const char * ctx) > > { > > unsigned hvalue; > > - int rc = 0; > > struct sidtab_node *newnode; > > char * newctx; > > > > - if (s->nel >= UINT_MAX - 1) { > > - rc = -1; > > - goto out; > > - } > > + if (s->nel >= UINT_MAX - 1) > > + return NULL; > > > > newnode = (struct sidtab_node *)avc_malloc(sizeof(*newnode)); > > - if (!newnode) { > > - rc = -1; > > - goto out; > > - } > > + if (!newnode) > > + return NULL; > > newctx = strdup(ctx); > > if (!newctx) { > > - rc = -1; > > avc_free(newnode); > > - goto out; > > + return NULL; > > } > > > > hvalue = sidtab_hash(newctx); > > @@ -73,8 +68,25 @@ int sidtab_insert(struct sidtab *s, const char * ctx) > > newnode->sid_s.ctx = newctx; > > newnode->sid_s.id = ++s->nel; > > s->htable[hvalue] = newnode; > > - out: > > - return rc; > > + return newnode; > > +} > > + > > +const struct security_id * > > +sidtab_context_lookup(const struct sidtab *s, const char *ctx) > > +{ > > + unsigned hvalue; > > + const struct sidtab_node *cur; > > + > > + hvalue = sidtab_hash(ctx); > > + > > + cur = s->htable[hvalue]; > > + while (cur != NULL && strcmp(cur->sid_s.ctx, ctx)) > > + cur = cur->next; > > + > > + if (cur == NULL) > > + return NULL; > > + > > + return &cur->sid_s; > > } > > > > int > > @@ -82,27 +94,23 @@ sidtab_context_to_sid(struct sidtab *s, > > const char * ctx, security_id_t * sid) > > { > > unsigned hvalue; > > - int rc = 0; > > struct sidtab_node *cur; > > > > *sid = NULL; > > hvalue = sidtab_hash(ctx); > > > > - loop: > > cur = s->htable[hvalue]; > > while (cur != NULL && strcmp(cur->sid_s.ctx, ctx)) > > cur = cur->next; > > > > if (cur == NULL) { /* need to make a new entry */ > > - rc = sidtab_insert(s, ctx); > > - if (rc) > > - goto out; > > - goto loop; /* find the newly inserted node */ > > + cur = sidtab_insert(s, ctx); > > + if (cur == NULL) > > + return -1; > > } > > > > *sid = &cur->sid_s; > > - out: > > - return rc; > > + return 0; > > } > > > > This duplicates the sidtab_context_lookup() code above, so why not > just use that. If that returns NULL, then insert the context. > > Thanks, > Jim True; applied in wip-v3: https://github.com/SELinuxProject/selinux/pull/406/commits/445cc5fc903fb6da7cefa059e29e6a7ed91302e9 > > void sidtab_sid_stats(const struct sidtab *s, char *buf, size_t buflen) > > @@ -138,7 +146,7 @@ void sidtab_destroy(struct sidtab *s) > > int i; > > struct sidtab_node *cur, *temp; > > > > - if (!s) > > + if (!s || !s->htable) > > return; > > > > for (i = 0; i < SIDTAB_SIZE; i++) { > > @@ -149,7 +157,6 @@ void sidtab_destroy(struct sidtab *s) > > freecon(temp->sid_s.ctx); > > avc_free(temp); > > } > > - s->htable[i] = NULL; > > } > > avc_free(s->htable); > > s->htable = NULL; > > diff --git a/libselinux/src/avc_sidtab.h b/libselinux/src/avc_sidtab.h > > index e823e3f3..f62fd353 100644 > > --- a/libselinux/src/avc_sidtab.h > > +++ b/libselinux/src/avc_sidtab.h > > @@ -24,8 +24,8 @@ struct sidtab { > > }; > > > > int sidtab_init(struct sidtab *s) ; > > -int sidtab_insert(struct sidtab *s, const char * ctx) ; > > > > +const struct security_id * sidtab_context_lookup(const struct sidtab *s, const char *ctx); > > int sidtab_context_to_sid(struct sidtab *s, > > const char * ctx, security_id_t * sid) ; > > > > -- > > 2.43.0 > > > >
diff --git a/libselinux/src/avc_sidtab.c b/libselinux/src/avc_sidtab.c index 9475dcb0..3d347cea 100644 --- a/libselinux/src/avc_sidtab.c +++ b/libselinux/src/avc_sidtab.c @@ -44,28 +44,23 @@ int sidtab_init(struct sidtab *s) return rc; } -int sidtab_insert(struct sidtab *s, const char * ctx) +static struct sidtab_node * +sidtab_insert(struct sidtab *s, const char * ctx) { unsigned hvalue; - int rc = 0; struct sidtab_node *newnode; char * newctx; - if (s->nel >= UINT_MAX - 1) { - rc = -1; - goto out; - } + if (s->nel >= UINT_MAX - 1) + return NULL; newnode = (struct sidtab_node *)avc_malloc(sizeof(*newnode)); - if (!newnode) { - rc = -1; - goto out; - } + if (!newnode) + return NULL; newctx = strdup(ctx); if (!newctx) { - rc = -1; avc_free(newnode); - goto out; + return NULL; } hvalue = sidtab_hash(newctx); @@ -73,8 +68,25 @@ int sidtab_insert(struct sidtab *s, const char * ctx) newnode->sid_s.ctx = newctx; newnode->sid_s.id = ++s->nel; s->htable[hvalue] = newnode; - out: - return rc; + return newnode; +} + +const struct security_id * +sidtab_context_lookup(const struct sidtab *s, const char *ctx) +{ + unsigned hvalue; + const struct sidtab_node *cur; + + hvalue = sidtab_hash(ctx); + + cur = s->htable[hvalue]; + while (cur != NULL && strcmp(cur->sid_s.ctx, ctx)) + cur = cur->next; + + if (cur == NULL) + return NULL; + + return &cur->sid_s; } int @@ -82,27 +94,23 @@ sidtab_context_to_sid(struct sidtab *s, const char * ctx, security_id_t * sid) { unsigned hvalue; - int rc = 0; struct sidtab_node *cur; *sid = NULL; hvalue = sidtab_hash(ctx); - loop: cur = s->htable[hvalue]; while (cur != NULL && strcmp(cur->sid_s.ctx, ctx)) cur = cur->next; if (cur == NULL) { /* need to make a new entry */ - rc = sidtab_insert(s, ctx); - if (rc) - goto out; - goto loop; /* find the newly inserted node */ + cur = sidtab_insert(s, ctx); + if (cur == NULL) + return -1; } *sid = &cur->sid_s; - out: - return rc; + return 0; } void sidtab_sid_stats(const struct sidtab *s, char *buf, size_t buflen) @@ -138,7 +146,7 @@ void sidtab_destroy(struct sidtab *s) int i; struct sidtab_node *cur, *temp; - if (!s) + if (!s || !s->htable) return; for (i = 0; i < SIDTAB_SIZE; i++) { @@ -149,7 +157,6 @@ void sidtab_destroy(struct sidtab *s) freecon(temp->sid_s.ctx); avc_free(temp); } - s->htable[i] = NULL; } avc_free(s->htable); s->htable = NULL; diff --git a/libselinux/src/avc_sidtab.h b/libselinux/src/avc_sidtab.h index e823e3f3..f62fd353 100644 --- a/libselinux/src/avc_sidtab.h +++ b/libselinux/src/avc_sidtab.h @@ -24,8 +24,8 @@ struct sidtab { }; int sidtab_init(struct sidtab *s) ; -int sidtab_insert(struct sidtab *s, const char * ctx) ; +const struct security_id * sidtab_context_lookup(const struct sidtab *s, const char *ctx); int sidtab_context_to_sid(struct sidtab *s, const char * ctx, security_id_t * sid) ;
Add sidtab_context_lookup() to just lookup a context, not inserting non-existent ones. Tweak sidtab_destroy() to accept a zero'ed struct sidtab. Remove redundant lookup in sidtab_context_to_sid() after insertion by returning the newly created node directly from sidtab_insert(). Drop declaration of only internal used sidtab_insert(). Signed-off-by: Christian Göttsche <cgzones@googlemail.com> --- v2: add patch --- libselinux/src/avc_sidtab.c | 55 +++++++++++++++++++++---------------- libselinux/src/avc_sidtab.h | 2 +- 2 files changed, 32 insertions(+), 25 deletions(-)