Re: [PATCH 1/2 v2] libsepol: Validate more access vector permissions

On Thu, Jul 23, 2026 at 3:18 PM James Carter <[email protected]> wrote:
>
> The commit 8c64e5bb6fe7 ("libsepol: validate access vector
> permissions") added a check that at least one permission of an
> access vector is valid. Add a macro to simplify the check and
> add a check for av rules in conditional blocks.
>
> Signed-off-by: James Carter <[email protected]>

Already acked the previous one, but:
Acked-by: Stephen Smalley <[email protected]>

> ---
> v2: No changes
>
>  libsepol/src/policydb_validate.c | 36 +++++++++++++++++++++-----------
>  1 file changed, 24 insertions(+), 12 deletions(-)
>
> diff --git a/libsepol/src/policydb_validate.c b/libsepol/src/policydb_validate.c
> index cde0b8d6..166e20a5 100644
> --- a/libsepol/src/policydb_validate.c
> +++ b/libsepol/src/policydb_validate.c
> @@ -11,9 +11,15 @@
>
>  #define bool_xor(a, b) (!(a) != !(b))
>  #define bool_xnor(a, b) (!bool_xor(a, b))
> +
> +/*
> + * Check that at least one permission bit is valid.
> + * Older compilers might set invalid bits for the wildcard permission.
> + */
>  #define PERMISSION_MASK(nprim)                          \
>         ((nprim) == PERM_SYMTAB_SIZE ? (~UINT32_C(0)) : \
>                                        ((UINT32_C(1) << (nprim)) - 1))
> +#define NO_VALID_PERMS(av, nprim) (!((av) & PERMISSION_MASK(nprim)))
>
>  typedef struct validate {
>         uint32_t nprim;
> @@ -265,10 +271,8 @@ static int validate_constraint_nodes(sepol_handle_t *handle, uint32_t nperms,
>         for (; cons; cons = cons->next) {
>                 if (is_validatetrans && cons->permissions != 0)
>                         goto bad;
> -               if (!is_validatetrans && cons->permissions == 0)
> -                       goto bad;
> -               if (!is_validatetrans && nperms != PERM_SYMTAB_SIZE &&
> -                   cons->permissions >= (UINT32_C(1) << nperms))
> +               if (!is_validatetrans &&
> +                   NO_VALID_PERMS(cons->permissions, nperms))
>                         goto bad;
>
>                 if (!cons->expr)
> @@ -1091,11 +1095,7 @@ static int validate_access_vector(sepol_handle_t *handle, const policydb_t *p,
>  {
>         const class_datum_t *cladatum = p->class_val_to_struct[tclass - 1];
>
> -       /*
> -        * Check that at least one permission bit is valid.
> -        * Older compilers might set invalid bits for the wildcard permission.
> -        */
> -       if (!(av & PERMISSION_MASK(cladatum->permissions.nprim)))
> +       if (NO_VALID_PERMS(av, cladatum->permissions.nprim))
>                 goto bad;
>
>         return 0;
> @@ -1214,12 +1214,24 @@ static int validate_avrules(sepol_handle_t *handle, const avrule_t *avrule,
>
>                 for (classperm = avrule->perms; classperm;
>                      classperm = classperm->next) {
> +                       class_datum_t *cladatum;
>                         if (validate_value(classperm->tclass,
>                                            &flavors[SYM_CLASSES]))
>                                 goto bad;
> -                       if ((avrule->specified & AVRULE_TYPE) &&
> -                           validate_simpletype(classperm->data, p, flavors))
> -                               goto bad;
> +                       cladatum =
> +                               p->class_val_to_struct[classperm->tclass - 1];
> +                       if (avrule->specified & AVRULE_AV) {
> +                               if (NO_VALID_PERMS(
> +                                           classperm->data,
> +                                           cladatum->permissions.nprim)) {
> +                                       goto bad;
> +                               }
> +                       } else if (avrule->specified & AVRULE_TYPE) {
> +                               if (validate_simpletype(classperm->data, p,
> +                                                       flavors)) {
> +                                       goto bad;
> +                               }
> +                       }
>                 }
>
>                 if (avrule->specified & AVRULE_XPERMS) {
> --
> 2.55.0
>