lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEjxPJ5y70ncKXw-_noNumz=miCB0U+3c3KnXFwrV=eQ1gwhKQ@mail.gmail.com>
Date: Wed, 14 May 2025 10:15:46 -0400
From: Stephen Smalley <stephen.smalley.work@...il.com>
To: cgzones@...glemail.com
Cc: selinux@...r.kernel.org, Paul Moore <paul@...l-moore.com>, 
	Ondrej Mosnacek <omosnace@...hat.com>, linux-kernel@...r.kernel.org, 
	Thiébaud Weksteen <tweek@...gle.com>, 
	Bram Bonné <brambonne@...gle.com>, 
	Eric Suen <ericsu@...ux.microsoft.com>, Casey Schaufler <casey@...aufler-ca.com>, 
	Canfeng Guo <guocanfeng@...ontech.com>, Takaya Saeki <takayas@...omium.org>
Subject: Re: [PATCH v3 03/14] selinux: more strict policy parsing

On Sun, May 11, 2025 at 1:31 PM Christian Göttsche
<cgoettsche@...tendoof.de> wrote:
>
> From: Christian Göttsche <cgzones@...glemail.com>
>
> Be more strict during parsing of policies and reject invalid values.
>
> Add some error messages in the case of policy parse failures, to
> enhance debugging, either on a malformed policy or a too strict check.
>
> Signed-off-by: Christian Göttsche <cgzones@...glemail.com>

Acked-by: Stephen Smalley <stephen.smalley.work@...il.com>

> ---
> v3:
>   - incorporate the overflow checks on security classes from the
>     previous patch, and permit U16_MAX as class ID
>   - minimize the usage of magic values, by using macros or trivial
>     helper functions
> v2:
>   accept unknown xperm specifiers to support backwards compatibility for
>   future ones, suggested by Thiébaud
> ---
>  security/selinux/include/security.h |   1 +
>  security/selinux/ss/avtab.c         |  35 ++++-
>  security/selinux/ss/avtab.h         |  13 ++
>  security/selinux/ss/conditional.c   |  18 +--
>  security/selinux/ss/constraint.h    |   1 +
>  security/selinux/ss/policydb.c      | 196 +++++++++++++++++++++++-----
>  security/selinux/ss/policydb.h      |  23 +++-
>  security/selinux/ss/services.c      |   6 +-
>  8 files changed, 233 insertions(+), 60 deletions(-)
>
> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> index 278c144c22d6..eca6693227ca 100644
> --- a/security/selinux/include/security.h
> +++ b/security/selinux/include/security.h
> @@ -224,6 +224,7 @@ int security_read_policy(void **data, size_t *len);
>  int security_read_state_kernel(void **data, size_t *len);
>  int security_policycap_supported(unsigned int req_cap);
>
> +/* Maximum supported number of permissions per class */
>  #define SEL_VEC_MAX 32
>  struct av_decision {
>         u32 allowed;
> diff --git a/security/selinux/ss/avtab.c b/security/selinux/ss/avtab.c
> index c2c31521cace..33556922f15e 100644
> --- a/security/selinux/ss/avtab.c
> +++ b/security/selinux/ss/avtab.c
> @@ -349,7 +349,7 @@ int avtab_read_item(struct avtab *a, struct policy_file *fp, struct policydb *po
>         struct avtab_extended_perms xperms;
>         __le32 buf32[ARRAY_SIZE(xperms.perms.p)];
>         int rc;
> -       unsigned int set, vers = pol->policyvers;
> +       unsigned int vers = pol->policyvers;
>
>         memset(&key, 0, sizeof(struct avtab_key));
>         memset(&datum, 0, sizeof(struct avtab_datum));
> @@ -360,9 +360,12 @@ int avtab_read_item(struct avtab *a, struct policy_file *fp, struct policydb *po
>                         pr_err("SELinux: avtab: truncated entry\n");
>                         return rc;
>                 }
> +               /* Read five or more items: source type, target type,
> +                * target class, AV type, and at least one datum.
> +                */
>                 items2 = le32_to_cpu(buf32[0]);
> -               if (items2 > ARRAY_SIZE(buf32)) {
> -                       pr_err("SELinux: avtab: entry overflow\n");
> +               if (items2 < 5 || items2 > ARRAY_SIZE(buf32)) {
> +                       pr_err("SELinux: avtab: invalid item count\n");
>                         return -EINVAL;
>                 }
>                 rc = next_entry(buf32, fp, sizeof(u32) * items2);
> @@ -391,6 +394,13 @@ int avtab_read_item(struct avtab *a, struct policy_file *fp, struct policydb *po
>                         return -EINVAL;
>                 }
>
> +               if (!policydb_type_isvalid(pol, key.source_type) ||
> +                   !policydb_type_isvalid(pol, key.target_type) ||
> +                   !policydb_class_isvalid(pol, key.target_class)) {
> +                       pr_err("SELinux: avtab: invalid type or class\n");
> +                       return -EINVAL;
> +               }
> +
>                 val = le32_to_cpu(buf32[items++]);
>                 enabled = (val & AVTAB_ENABLED_OLD) ? AVTAB_ENABLED : 0;
>
> @@ -409,6 +419,11 @@ int avtab_read_item(struct avtab *a, struct policy_file *fp, struct policydb *po
>
>                 for (i = 0; i < ARRAY_SIZE(spec_order); i++) {
>                         if (val & spec_order[i]) {
> +                               if (items >= items2) {
> +                                       pr_err("SELinux: avtab: entry has too many items (%d/%d)\n",
> +                                              items + 1, items2);
> +                                       return -EINVAL;
> +                               }
>                                 key.specified = spec_order[i] | enabled;
>                                 datum.u.data = le32_to_cpu(buf32[items++]);
>                                 rc = insertf(a, &key, &datum, p);
> @@ -444,9 +459,13 @@ int avtab_read_item(struct avtab *a, struct policy_file *fp, struct policydb *po
>                 return -EINVAL;
>         }
>
> -       set = hweight16(key.specified & (AVTAB_XPERMS | AVTAB_TYPE | AVTAB_AV));
> -       if (!set || set > 1) {
> -               pr_err("SELinux:  avtab:  more than one specifier\n");
> +       if (hweight16(key.specified & ~AVTAB_ENABLED) != 1) {
> +               pr_err("SELinux:  avtab:  not exactly one specifier\n");
> +               return -EINVAL;
> +       }
> +
> +       if (key.specified & ~AVTAB_SPECIFIER_MASK) {
> +               pr_err("SELinux:  avtab:  invalid specifier\n");
>                 return -EINVAL;
>         }
>
> @@ -471,6 +490,10 @@ int avtab_read_item(struct avtab *a, struct policy_file *fp, struct policydb *po
>                         pr_err("SELinux: avtab: truncated entry\n");
>                         return rc;
>                 }
> +               if (!avtab_is_valid_xperm_specified(xperms.specified))
> +                       pr_warn_once_policyload(pol,
> +                                               "SELinux: avtab: unsupported xperm specifier %#x\n",
> +                                               xperms.specified);
>                 rc = next_entry(&xperms.driver, fp, sizeof(u8));
>                 if (rc) {
>                         pr_err("SELinux: avtab: truncated entry\n");
> diff --git a/security/selinux/ss/avtab.h b/security/selinux/ss/avtab.h
> index 850b3453f259..1de4cce288a7 100644
> --- a/security/selinux/ss/avtab.h
> +++ b/security/selinux/ss/avtab.h
> @@ -44,6 +44,7 @@ struct avtab_key {
>          AVTAB_XPERMS_DONTAUDIT)
>  #define AVTAB_ENABLED_OLD 0x80000000 /* reserved for used in cond_avtab */
>  #define AVTAB_ENABLED    0x8000 /* reserved for used in cond_avtab */
> +#define AVTAB_SPECIFIER_MASK (AVTAB_AV | AVTAB_TYPE | AVTAB_XPERMS | AVTAB_ENABLED)
>         u16 specified; /* what field is specified */
>  };
>
> @@ -68,6 +69,18 @@ struct avtab_extended_perms {
>         struct extended_perms_data perms;
>  };
>
> +static inline bool avtab_is_valid_xperm_specified(u8 specified)
> +{
> +       switch (specified) {
> +       case AVTAB_XPERMS_IOCTLFUNCTION:
> +       case AVTAB_XPERMS_IOCTLDRIVER:
> +       case AVTAB_XPERMS_NLMSG:
> +               return true;
> +       default:
> +               return false;
> +       }
> +}
> +
>  struct avtab_datum {
>         union {
>                 u32 data; /* access vector or type value */
> diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c
> index 1bebfcb9c6a1..db30462ed6a3 100644
> --- a/security/selinux/ss/conditional.c
> +++ b/security/selinux/ss/conditional.c
> @@ -199,19 +199,12 @@ int cond_index_bool(void *key, void *datum, void *datap)
>         return 0;
>  }
>
> -static int bool_isvalid(struct cond_bool_datum *b)
> -{
> -       if (!(b->state == 0 || b->state == 1))
> -               return 0;
> -       return 1;
> -}
> -
>  int cond_read_bool(struct policydb *p, struct symtab *s, struct policy_file *fp)
>  {
>         char *key = NULL;
>         struct cond_bool_datum *booldatum;
>         __le32 buf[3];
> -       u32 len;
> +       u32 len, val;
>         int rc;
>
>         booldatum = kzalloc(sizeof(*booldatum), GFP_KERNEL);
> @@ -223,11 +216,12 @@ int cond_read_bool(struct policydb *p, struct symtab *s, struct policy_file *fp)
>                 goto err;
>
>         booldatum->value = le32_to_cpu(buf[0]);
> -       booldatum->state = le32_to_cpu(buf[1]);
> +       val = le32_to_cpu(buf[1]);
>
>         rc = -EINVAL;
> -       if (!bool_isvalid(booldatum))
> +       if (!val_is_boolean(val))
>                 goto err;
> +       booldatum->state = (int)val;
>
>         len = le32_to_cpu(buf[2]);
>
> @@ -241,6 +235,7 @@ int cond_read_bool(struct policydb *p, struct symtab *s, struct policy_file *fp)
>
>         return 0;
>  err:
> +       pr_err("SELinux: conditional: failed to read boolean\n");
>         cond_destroy_bool(key, booldatum, NULL);
>         return rc;
>  }
> @@ -362,7 +357,8 @@ static int expr_node_isvalid(struct policydb *p, struct cond_expr_node *expr)
>                 return 0;
>         }
>
> -       if (expr->boolean > p->p_bools.nprim) {
> +       if (expr->expr_type == COND_BOOL &&
> +           (expr->boolean == 0 || expr->boolean > p->p_bools.nprim)) {
>                 pr_err("SELinux: conditional expressions uses unknown bool.\n");
>                 return 0;
>         }
> diff --git a/security/selinux/ss/constraint.h b/security/selinux/ss/constraint.h
> index 203033cfad67..1d75a8a044df 100644
> --- a/security/selinux/ss/constraint.h
> +++ b/security/selinux/ss/constraint.h
> @@ -50,6 +50,7 @@ struct constraint_expr {
>         u32 op; /* operator */
>
>         struct ebitmap names; /* names */
> +       /* internally unused, only forwarded via policydb_write() */
>         struct type_set *type_names;
>
>         struct constraint_expr *next; /* next expression */
> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index f490556ddb5c..3e4a28a2928b 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -634,13 +634,11 @@ static int sens_index(void *key, void *datum, void *datap)
>         levdatum = datum;
>         p = datap;
>
> -       if (!levdatum->isalias) {
> -               if (!levdatum->level.sens ||
> -                   levdatum->level.sens > p->p_levels.nprim)
> -                       return -EINVAL;
> +       if (!levdatum->level.sens || levdatum->level.sens > p->p_levels.nprim)
> +               return -EINVAL;
>
> +       if (!levdatum->isalias)
>                 p->sym_val_to_name[SYM_LEVELS][levdatum->level.sens - 1] = key;
> -       }
>
>         return 0;
>  }
> @@ -653,12 +651,11 @@ static int cat_index(void *key, void *datum, void *datap)
>         catdatum = datum;
>         p = datap;
>
> -       if (!catdatum->isalias) {
> -               if (!catdatum->value || catdatum->value > p->p_cats.nprim)
> -                       return -EINVAL;
> +       if (!catdatum->value || catdatum->value > p->p_cats.nprim)
> +               return -EINVAL;
>
> +       if (!catdatum->isalias)
>                 p->sym_val_to_name[SYM_CATS][catdatum->value - 1] = key;
> -       }
>
>         return 0;
>  }
> @@ -1136,6 +1133,9 @@ static int perm_read(struct policydb *p, struct symtab *s, struct policy_file *f
>
>         len = le32_to_cpu(buf[0]);
>         perdatum->value = le32_to_cpu(buf[1]);
> +       rc = -EINVAL;
> +       if (perdatum->value < 1 || perdatum->value > SEL_VEC_MAX)
> +               goto bad;
>
>         rc = str_read(&key, GFP_KERNEL, fp, len);
>         if (rc)
> @@ -1170,6 +1170,9 @@ static int common_read(struct policydb *p, struct symtab *s, struct policy_file
>         len = le32_to_cpu(buf[0]);
>         comdatum->value = le32_to_cpu(buf[1]);
>         nel = le32_to_cpu(buf[3]);
> +       rc = -EINVAL;
> +       if (nel > SEL_VEC_MAX)
> +               goto bad;
>
>         rc = symtab_init(&comdatum->permissions, nel);
>         if (rc)
> @@ -1321,7 +1324,7 @@ static int class_read(struct policydb *p, struct symtab *s, struct policy_file *
>         char *key = NULL;
>         struct class_datum *cladatum;
>         __le32 buf[6];
> -       u32 i, len, len2, ncons, nel;
> +       u32 i, len, len2, ncons, nel, val;
>         int rc;
>
>         cladatum = kzalloc(sizeof(*cladatum), GFP_KERNEL);
> @@ -1334,8 +1337,16 @@ static int class_read(struct policydb *p, struct symtab *s, struct policy_file *
>
>         len = le32_to_cpu(buf[0]);
>         len2 = le32_to_cpu(buf[1]);
> -       cladatum->value = le32_to_cpu(buf[2]);
>         nel = le32_to_cpu(buf[4]);
> +       rc = -EINVAL;
> +       if (nel > SEL_VEC_MAX)
> +               goto bad;
> +
> +       val = le32_to_cpu(buf[2]);
> +       rc = -EINVAL;
> +       if (val > U16_MAX)
> +               goto bad;
> +       cladatum->value = val;
>
>         rc = symtab_init(&cladatum->permissions, nel);
>         if (rc)
> @@ -1391,16 +1402,59 @@ static int class_read(struct policydb *p, struct symtab *s, struct policy_file *
>                 if (rc)
>                         goto bad;
>
> -               cladatum->default_user = le32_to_cpu(buf[0]);
> -               cladatum->default_role = le32_to_cpu(buf[1]);
> -               cladatum->default_range = le32_to_cpu(buf[2]);
> +               rc = -EINVAL;
> +               val = le32_to_cpu(buf[0]);
> +               switch (val) {
> +               case 0:
> +               case DEFAULT_SOURCE:
> +               case DEFAULT_TARGET:
> +                       cladatum->default_user = val;
> +                       break;
> +               default:
> +                       goto bad;
> +               }
> +               val = le32_to_cpu(buf[1]);
> +               switch (val) {
> +               case 0:
> +               case DEFAULT_SOURCE:
> +               case DEFAULT_TARGET:
> +                       cladatum->default_role = val;
> +                       break;
> +               default:
> +                       goto bad;
> +               }
> +               val = le32_to_cpu(buf[2]);
> +               switch (val) {
> +               case 0:
> +               case DEFAULT_SOURCE_LOW:
> +               case DEFAULT_SOURCE_HIGH:
> +               case DEFAULT_SOURCE_LOW_HIGH:
> +               case DEFAULT_TARGET_LOW:
> +               case DEFAULT_TARGET_HIGH:
> +               case DEFAULT_TARGET_LOW_HIGH:
> +               case DEFAULT_GLBLUB:
> +                       cladatum->default_range = val;
> +                       break;
> +               default:
> +                       goto bad;
> +               }
>         }
>
>         if (p->policyvers >= POLICYDB_VERSION_DEFAULT_TYPE) {
>                 rc = next_entry(buf, fp, sizeof(u32) * 1);
>                 if (rc)
>                         goto bad;
> -               cladatum->default_type = le32_to_cpu(buf[0]);
> +               rc = -EINVAL;
> +               val = le32_to_cpu(buf[0]);
> +               switch (val) {
> +               case 0:
> +               case DEFAULT_TARGET:
> +               case DEFAULT_SOURCE:
> +                       cladatum->default_type = val;
> +                       break;
> +               default:
> +                       goto bad;
> +               }
>         }
>
>         rc = symtab_insert(s, key, cladatum);
> @@ -1410,6 +1464,8 @@ static int class_read(struct policydb *p, struct symtab *s, struct policy_file *
>         return 0;
>  bad:
>         cls_destroy(key, cladatum, NULL);
> +       if (rc)
> +               pr_err("SELinux:  invalid class\n");
>         return rc;
>  }
>
> @@ -1601,7 +1657,7 @@ static int sens_read(struct policydb *p, struct symtab *s, struct policy_file *f
>         struct level_datum *levdatum;
>         int rc;
>         __le32 buf[2];
> -       u32 len;
> +       u32 len, val;
>
>         levdatum = kzalloc(sizeof(*levdatum), GFP_KERNEL);
>         if (!levdatum)
> @@ -1612,7 +1668,11 @@ static int sens_read(struct policydb *p, struct symtab *s, struct policy_file *f
>                 goto bad;
>
>         len = le32_to_cpu(buf[0]);
> -       levdatum->isalias = le32_to_cpu(buf[1]);
> +       val = le32_to_cpu(buf[1]);
> +       rc = -EINVAL;
> +       if (!val_is_boolean(val))
> +               goto bad;
> +       levdatum->isalias = val;
>
>         rc = str_read(&key, GFP_KERNEL, fp, len);
>         if (rc)
> @@ -1628,6 +1688,8 @@ static int sens_read(struct policydb *p, struct symtab *s, struct policy_file *f
>         return 0;
>  bad:
>         sens_destroy(key, levdatum, NULL);
> +       if (rc)
> +               pr_err("SELinux:  invalid sensitivity\n");
>         return rc;
>  }
>
> @@ -1637,7 +1699,7 @@ static int cat_read(struct policydb *p, struct symtab *s, struct policy_file *fp
>         struct cat_datum *catdatum;
>         int rc;
>         __le32 buf[3];
> -       u32 len;
> +       u32 len, val;
>
>         catdatum = kzalloc(sizeof(*catdatum), GFP_KERNEL);
>         if (!catdatum)
> @@ -1649,7 +1711,11 @@ static int cat_read(struct policydb *p, struct symtab *s, struct policy_file *fp
>
>         len = le32_to_cpu(buf[0]);
>         catdatum->value = le32_to_cpu(buf[1]);
> -       catdatum->isalias = le32_to_cpu(buf[2]);
> +       val = le32_to_cpu(buf[2]);
> +       rc = -EINVAL;
> +       if (!val_is_boolean(val))
> +               goto bad;
> +       catdatum->isalias = val;
>
>         rc = str_read(&key, GFP_KERNEL, fp, len);
>         if (rc)
> @@ -1661,6 +1727,8 @@ static int cat_read(struct policydb *p, struct symtab *s, struct policy_file *fp
>         return 0;
>  bad:
>         cat_destroy(key, catdatum, NULL);
> +       if (rc)
> +               pr_err("SELinux:  invalid category\n");
>         return rc;
>  }
>
> @@ -1842,7 +1910,7 @@ static int range_read(struct policydb *p, struct policy_file *fp)
>         struct mls_range *r = NULL;
>         int rc;
>         __le32 buf[2];
> -       u32 i, nel;
> +       u32 i, nel, val;
>
>         if (p->policyvers < POLICYDB_VERSION_MLS)
>                 return 0;
> @@ -1873,7 +1941,11 @@ static int range_read(struct policydb *p, struct policy_file *fp)
>                         rc = next_entry(buf, fp, sizeof(u32));
>                         if (rc)
>                                 goto out;
> -                       rt->target_class = le32_to_cpu(buf[0]);
> +                       rc = -EINVAL;
> +                       val = le32_to_cpu(buf[0]);
> +                       if (val > U16_MAX)
> +                               goto out;
> +                       rt->target_class = val;
>                 } else
>                         rt->target_class = p->process_class;
>
> @@ -1910,6 +1982,8 @@ static int range_read(struct policydb *p, struct policy_file *fp)
>  out:
>         kfree(rt);
>         kfree(r);
> +       if (rc)
> +               pr_err("SELinux:  invalid range\n");
>         return rc;
>  }
>
> @@ -1918,7 +1992,7 @@ static int filename_trans_read_helper_compat(struct policydb *p, struct policy_f
>         struct filename_trans_key key, *ft = NULL;
>         struct filename_trans_datum *last, *datum = NULL;
>         char *name = NULL;
> -       u32 len, stype, otype;
> +       u32 len, stype, otype, val;
>         __le32 buf[4];
>         int rc;
>
> @@ -1937,9 +2011,17 @@ static int filename_trans_read_helper_compat(struct policydb *p, struct policy_f
>         if (rc)
>                 goto out;
>
> +       rc = -EINVAL;
>         stype = le32_to_cpu(buf[0]);
> +       if (!policydb_type_isvalid(p, stype))
> +               goto out;
>         key.ttype = le32_to_cpu(buf[1]);
> -       key.tclass = le32_to_cpu(buf[2]);
> +       if (!policydb_type_isvalid(p, key.ttype))
> +               goto out;
> +       val = le32_to_cpu(buf[2]);
> +       if (val > U16_MAX || !policydb_class_isvalid(p, val))
> +               goto out;
> +       key.tclass = val;
>         key.name = name;
>
>         otype = le32_to_cpu(buf[3]);
> @@ -1995,6 +2077,9 @@ static int filename_trans_read_helper_compat(struct policydb *p, struct policy_f
>         kfree(ft);
>         kfree(name);
>         kfree(datum);
> +
> +       if (rc)
> +               pr_err("SELinux:  invalid compat filename transition\n");
>         return rc;
>  }
>
> @@ -2003,7 +2088,7 @@ static int filename_trans_read_helper(struct policydb *p, struct policy_file *fp
>         struct filename_trans_key *ft = NULL;
>         struct filename_trans_datum **dst, *datum, *first = NULL;
>         char *name = NULL;
> -       u32 len, ttype, ndatum, i;
> +       u32 len, ttype, ndatum, i, val;
>         u16 tclass;
>         __le32 buf[3];
>         int rc;
> @@ -2023,8 +2108,15 @@ static int filename_trans_read_helper(struct policydb *p, struct policy_file *fp
>         if (rc)
>                 goto out;
>
> +       rc = -EINVAL;
>         ttype = le32_to_cpu(buf[0]);
> -       tclass = le32_to_cpu(buf[1]);
> +       if (!policydb_type_isvalid(p, ttype))
> +               goto out;
> +       val = le32_to_cpu(buf[1]);
> +       rc = -EINVAL;
> +       if (val > U16_MAX || !policydb_class_isvalid(p, val))
> +               goto out;
> +       tclass = val;
>
>         ndatum = le32_to_cpu(buf[2]);
>         if (ndatum == 0) {
> @@ -2054,6 +2146,10 @@ static int filename_trans_read_helper(struct policydb *p, struct policy_file *fp
>
>                 datum->otype = le32_to_cpu(buf[0]);
>
> +               rc = -EINVAL;
> +               if (!policydb_type_isvalid(p, datum->otype))
> +                       goto out;
> +
>                 dst = &datum->next;
>         }
>
> @@ -2085,6 +2181,9 @@ static int filename_trans_read_helper(struct policydb *p, struct policy_file *fp
>                 ebitmap_destroy(&datum->stypes);
>                 kfree(datum);
>         }
> +
> +       if (rc)
> +               pr_err("SELinux:  invalid filename transition\n");
>         return rc;
>  }
>
> @@ -2132,7 +2231,7 @@ static int filename_trans_read(struct policydb *p, struct policy_file *fp)
>  static int genfs_read(struct policydb *p, struct policy_file *fp)
>  {
>         int rc;
> -       u32 i, j, nel, nel2, len, len2;
> +       u32 i, j, nel, nel2, len, len2, val;
>         __le32 buf[1];
>         struct ocontext *l, *c;
>         struct ocontext *newc = NULL;
> @@ -2202,7 +2301,11 @@ static int genfs_read(struct policydb *p, struct policy_file *fp)
>                         if (rc)
>                                 goto out;
>
> -                       newc->v.sclass = le32_to_cpu(buf[0]);
> +                       rc = -EINVAL;
> +                       val = le32_to_cpu(buf[0]);
> +                       if (val > U16_MAX || (val != 0 && !policydb_class_isvalid(p, val)))
> +                               goto out;
> +                       newc->v.sclass = val;
>                         rc = context_read_and_validate(&newc->context[0], p,
>                                                        fp);
>                         if (rc)
> @@ -2239,6 +2342,9 @@ static int genfs_read(struct policydb *p, struct policy_file *fp)
>         }
>         ocontext_destroy(newc, OCON_FSUSE);
>
> +       if (rc)
> +               pr_err("SELinux:  invalid genfs\n");
> +
>         return rc;
>  }
>
> @@ -2247,7 +2353,7 @@ static int ocontext_read(struct policydb *p,
>  {
>         int rc;
>         unsigned int i;
> -       u32 j, nel, len;
> +       u32 j, nel, len, val;
>         __be64 prefixbuf[1];
>         __le32 buf[3];
>         struct ocontext *l, *c;
> @@ -2311,11 +2417,25 @@ static int ocontext_read(struct policydb *p,
>                                 rc = next_entry(buf, fp, sizeof(u32) * 3);
>                                 if (rc)
>                                         goto out;
> -                               c->u.port.protocol = le32_to_cpu(buf[0]);
> -                               c->u.port.low_port = le32_to_cpu(buf[1]);
> -                               c->u.port.high_port = le32_to_cpu(buf[2]);
> -                               rc = context_read_and_validate(&c->context[0],
> -                                                              p, fp);
> +
> +                               rc = -EINVAL;
> +                               val = le32_to_cpu(buf[0]);
> +                               if (val > U8_MAX)
> +                                       goto out;
> +                               c->u.port.protocol = val;
> +                               val = le32_to_cpu(buf[1]);
> +                               if (val > U16_MAX)
> +                                       goto out;
> +                               c->u.port.low_port = val;
> +                               val = le32_to_cpu(buf[2]);
> +                               if (val > U16_MAX)
> +                                       goto out;
> +                               c->u.port.high_port = val;
> +                               if (c->u.port.low_port == 0 ||
> +                                   c->u.port.low_port > c->u.port.high_port)
> +                                       goto out;
> +
> +                               rc = context_read_and_validate(&c->context[0], p, fp);
>                                 if (rc)
>                                         goto out;
>                                 break;
> @@ -2433,6 +2553,8 @@ static int ocontext_read(struct policydb *p,
>         }
>         rc = 0;
>  out:
> +       if (rc)
> +               pr_err("SELinux:  invalid ocon\n");
>         return rc;
>  }
>
> @@ -2447,7 +2569,7 @@ int policydb_read(struct policydb *p, struct policy_file *fp)
>         struct role_trans_datum *rtd = NULL;
>         int rc;
>         __le32 buf[4];
> -       u32 i, j, len, nprim, nel, perm;
> +       u32 i, j, len, nprim, nel, perm, val;
>
>         char *policydb_str;
>         const struct policydb_compat_info *info;
> @@ -2633,7 +2755,11 @@ int policydb_read(struct policydb *p, struct policy_file *fp)
>                         rc = next_entry(buf, fp, sizeof(u32));
>                         if (rc)
>                                 goto bad;
> -                       rtk->tclass = le32_to_cpu(buf[0]);
> +                       rc = -EINVAL;
> +                       val = le32_to_cpu(buf[0]);
> +                       if (val > U16_MAX)
> +                               goto bad;
> +                       rtk->tclass = val;
>                 } else
>                         rtk->tclass = p->process_class;
>
> diff --git a/security/selinux/ss/policydb.h b/security/selinux/ss/policydb.h
> index 0c423ad77fd9..9b3cc393a979 100644
> --- a/security/selinux/ss/policydb.h
> +++ b/security/selinux/ss/policydb.h
> @@ -74,7 +74,7 @@ struct class_datum {
>  /* Role attributes */
>  struct role_datum {
>         u32 value; /* internal role value */
> -       u32 bounds; /* boundary of role */
> +       u32 bounds; /* boundary of role, 0 for none */
>         struct ebitmap dominates; /* set of roles dominated by this role */
>         struct ebitmap types; /* set of authorized types for role */
>  };
> @@ -110,7 +110,8 @@ struct role_allow {
>  /* Type attributes */
>  struct type_datum {
>         u32 value; /* internal type value */
> -       u32 bounds; /* boundary of type */
> +       u32 bounds; /* boundary of type, 0 for none */
> +       /* internally unused, only forwarded via policydb_write() */
>         unsigned char primary; /* primary name? */
>         unsigned char attribute; /* attribute ?*/
>  };
> @@ -118,7 +119,7 @@ struct type_datum {
>  /* User attributes */
>  struct user_datum {
>         u32 value; /* internal user value */
> -       u32 bounds; /* bounds of user */
> +       u32 bounds; /* bounds of user, 0 for none */
>         struct ebitmap roles; /* set of authorized roles for user */
>         struct mls_range range; /* MLS range (min - max) for user */
>         struct mls_level dfltlevel; /* default login MLS level for user */
> @@ -195,7 +196,7 @@ struct ocontext {
>                 } ibendport;
>         } u;
>         union {
> -               u16 sclass; /* security class for genfs */
> +               u16 sclass; /* security class for genfs (can be 0 for wildcard) */
>                 u32 behavior; /* labeling behavior for fs_use */
>         } v;
>         struct context context[2]; /* security context(s) */
> @@ -386,9 +387,23 @@ static inline char *sym_name(struct policydb *p, unsigned int sym_num,
>         return p->sym_val_to_name[sym_num][element_nr];
>  }
>
> +static inline bool val_is_boolean(u32 value)
> +{
> +       return value == 0 || value == 1;
> +}
> +
>  extern int str_read(char **strp, gfp_t flags, struct policy_file *fp, u32 len);
>
>  extern u16 string_to_security_class(struct policydb *p, const char *name);
>  extern u32 string_to_av_perm(struct policydb *p, u16 tclass, const char *name);
>
> +#define pr_warn_once_policyload(policy, fmt, ...)                        \
> +       do {                                                             \
> +               static const void *prev_policy__;                        \
> +               if (prev_policy__ != policy) {                           \
> +                       printk(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__); \
> +                       prev_policy__ = policy;                          \
> +               }                                                        \
> +       } while (0)
> +
>  #endif /* _SS_POLICYDB_H_ */
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index a2dd42e750fe..0f67a030b49b 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -446,8 +446,6 @@ static int dump_masked_av_helper(void *k, void *d, void *args)
>         struct perm_datum *pdatum = d;
>         char **permission_names = args;
>
> -       BUG_ON(pdatum->value < 1 || pdatum->value > 32);
> -
>         permission_names[pdatum->value - 1] = (char *)k;
>
>         return 0;
> @@ -466,7 +464,7 @@ static void security_dump_masked_av(struct policydb *policydb,
>         char *tclass_name;
>         char *scontext_name = NULL;
>         char *tcontext_name = NULL;
> -       char *permission_names[32];
> +       char *permission_names[SEL_VEC_MAX];
>         int index;
>         u32 length;
>         bool need_comma = false;
> @@ -507,7 +505,7 @@ static void security_dump_masked_av(struct policydb *policydb,
>                          "scontext=%s tcontext=%s tclass=%s perms=",
>                          reason, scontext_name, tcontext_name, tclass_name);
>
> -       for (index = 0; index < 32; index++) {
> +       for (index = 0; index < SEL_VEC_MAX; index++) {
>                 u32 mask = (1 << index);
>
>                 if ((mask & permissions) == 0)
> --
> 2.49.0
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ