On Thu, 7 Mar 2024 at 21:53, James Carter <jwcart2@xxxxxxxxx> wrote:
>
> On Wed, Jan 31, 2024 at 8:18 AM Christian Göttsche
> <cgzones@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
> > ---
> > 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
> >
> >
