On Tue, Jun 11, 2024 at 2:27 PM James Carter <jwcart2@xxxxxxxxx> wrote:
>
> On Sat, Jun 8, 2024 at 1:22 PM Christian Göttsche
> <cgoettsche@xxxxxxxxxxxxx> wrote:
> >
> > From: Christian Göttsche <cgzones@xxxxxxxxxxxxxx>
> >
> > Check the class is defined once, and not for every permission via
> > is_perm_enabled(). Also pass the class datum to avoid an unnecessary
> > name lookup.
> >
> > Signed-off-by: Christian Göttsche <cgzones@xxxxxxxxxxxxxx>
I will fix the comment when I merge the patch.
Acked-by: James Carter <jwcart2@xxxxxxxxx>
> > ---
> > .../include/sepol/policydb/avrule_block.h | 4 +--
> > libsepol/src/avrule_block.c | 27 +++++++------------
> > libsepol/src/link.c | 6 ++++-
> > 3 files changed, 17 insertions(+), 20 deletions(-)
> >
> > diff --git a/libsepol/include/sepol/policydb/avrule_block.h b/libsepol/include/sepol/policydb/avrule_block.h
> > index 27047d43..18a1dc78 100644
> > --- a/libsepol/include/sepol/policydb/avrule_block.h
> > +++ b/libsepol/include/sepol/policydb/avrule_block.h
> > @@ -35,8 +35,8 @@ extern avrule_decl_t *get_avrule_decl(policydb_t * p, uint32_t decl_id);
> > extern cond_list_t *get_decl_cond_list(policydb_t * p,
> > avrule_decl_t * decl,
> > cond_list_t * cond);
> > -extern int is_id_enabled(char *id, policydb_t * p, int symbol_table);
> > -extern int is_perm_enabled(char *class_id, char *perm_id, policydb_t * p);
> > +extern int is_id_enabled(const char *id, const policydb_t * p, int symbol_table);
> > +extern int is_perm_existent(const class_datum_t *cladatum, const char *perm_id);
> >
> > #ifdef __cplusplus
> > }
> > diff --git a/libsepol/src/avrule_block.c b/libsepol/src/avrule_block.c
> > index dcfce8b8..547021e9 100644
> > --- a/libsepol/src/avrule_block.c
> > +++ b/libsepol/src/avrule_block.c
> > @@ -152,11 +152,11 @@ cond_list_t *get_decl_cond_list(policydb_t * p, avrule_decl_t * decl,
> > * marked as SCOPE_DECL, and any of its declaring block has been enabled,
> > * then return 1. Otherwise return 0. Can only be called after the
> > * decl_val_to_struct index has been created */
> > -int is_id_enabled(char *id, policydb_t * p, int symbol_table)
> > +int is_id_enabled(const char *id, const policydb_t * p, int symbol_table)
> > {
> > - scope_datum_t *scope =
> > + const scope_datum_t *scope =
> > (scope_datum_t *) hashtab_search(p->scope[symbol_table].table, id);
> > - avrule_decl_t *decl;
> > + const avrule_decl_t *decl;
> > uint32_t len;
> >
> > if (scope == NULL) {
> > @@ -189,21 +189,14 @@ int is_id_enabled(char *id, policydb_t * p, int symbol_table)
> > return 0;
> > }
> >
> > -/* Check if a particular permission is present within the given class,
> > - * and that the class is enabled. Returns 1 if both conditions are
> > - * true, 0 if neither could be found or if the class id disabled. */
> > -int is_perm_enabled(char *class_id, char *perm_id, policydb_t * p)
> > +/* Check if a particular permission is present within the given class.
> > + * Whether the class is enabled is NOT checked.
> > + * Returns 1 if both conditions are true,
> > + * 0 if neither could be found or if the class id disabled. */
>
> It returns 0 only if the permission is not found.
>
> Everything else looks good to me.
> Thanks,
> Jim
>
>
> > +int is_perm_existent(const class_datum_t *cladatum, const char *perm_id)
> > {
> > - class_datum_t *cladatum;
> > - perm_datum_t *perm;
> > - if (!is_id_enabled(class_id, p, SYM_CLASSES)) {
> > - return 0;
> > - }
> > - cladatum =
> > - (class_datum_t *) hashtab_search(p->p_classes.table, class_id);
> > - if (cladatum == NULL) {
> > - return 0;
> > - }
> > + const perm_datum_t *perm;
> > +
> > perm = hashtab_search(cladatum->permissions.table, perm_id);
> > if (perm == NULL && cladatum->comdatum != 0) {
> > /* permission was not in this class. before giving
> > diff --git a/libsepol/src/link.c b/libsepol/src/link.c
> > index a6f2a251..9281a986 100644
> > --- a/libsepol/src/link.c
> > +++ b/libsepol/src/link.c
> > @@ -1968,6 +1968,10 @@ static int is_decl_requires_met(link_state_t * state,
> > id = pol->p_class_val_to_name[i];
> >
> >
> > + if (!is_id_enabled(id, state->base, SYM_CLASSES)) {
> > + return 0;
> > + }
> > +
> > scope = hashtab_search(state->base->p_classes_scope.table, id);
> > if (scope == NULL) {
> > ERR(state->handle,
> > @@ -1994,7 +1998,7 @@ static int is_decl_requires_met(link_state_t * state,
> > perm_id = fparg.key;
> >
> > assert(perm_id != NULL);
> > - if (!is_perm_enabled(id, perm_id, state->base)) {
> > + if (!is_perm_existent(cladatum, perm_id)) {
> > if (req != NULL) {
> > req->symbol_type = SYM_CLASSES;
> > req->symbol_value = i + 1;
> > --
> > 2.45.1
> >
> >
