[PATCH 2/5] selinux: bounds-check filename transition source types at load

[PATCH 2/5] selinux: bounds-check filename transition source types at load

[Christian Göttsche]

From: Christian Göttsche <cgzones@googlemail.com>

filename_trans_read_helper() validates the target type, target class and
each datum's otype, but the per-datum stypes ebitmap is read without any
bound on its set bits.
Reject a stypes bitmap whose highest set bit is not a defined type.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 security/selinux/ss/policydb.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index 3451a0295cc2..1a60d0f4be55 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -2367,6 +2367,10 @@ static int filename_trans_read_helper(struct policydb *p, struct policy_file *fp
 		if (rc)
 			goto out;
 
+		rc = -EINVAL;
+		if (ebitmap_get_highest_set_bit(&datum->stypes) >= p->p_types.nprim)
+			goto out;
+
 		rc = next_entry(buf, fp, sizeof(u32));
 		if (rc)
 			goto out;
-- 
2.55.0

[PATCH 3/5] selinux: bounds-check role and user membership bitmaps at load

[Christian Göttsche]

From: Christian Göttsche <cgzones@googlemail.com>

role->dominates, role->types and user->roles are read as ebitmaps but
their set bits are never checked against the role and type symbol tables.
An out-of-range bit is used to index those tables later; in particular
role_bounds_sanity_check() passes such a bit straight to sym_name(),
reading p->sym_val_to_name[] out of bounds on a crafted bounded role.

Validate the three bitmaps in policydb_index() once every symbol table is
populated (roles are read before types, so this cannot be done in
role_read()).

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 security/selinux/ss/policydb.c | 42 ++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index 1a60d0f4be55..3bef28434180 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -707,6 +707,40 @@ static int sens_cat_index_check(void *key, void *datum, void *datap)
 	return 0;
 }
 
+static int role_index_check(void *key, void *datum, void *datap)
+{
+	const struct policydb *p = datap;
+	const struct role_datum *role = datum;
+
+	if (ebitmap_get_highest_set_bit(&role->dominates) >= p->p_roles.nprim) {
+		pr_err("SELinux: role %s has an out-of-range dominance bit\n",
+		       (const char *)key);
+		return -EINVAL;
+	}
+
+	if (ebitmap_get_highest_set_bit(&role->types) >= p->p_types.nprim) {
+		pr_err("SELinux: role %s has an out-of-range type bit\n",
+		       (const char *)key);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int user_index_check(void *key, void *datum, void *datap)
+{
+	const struct policydb *p = datap;
+	const struct user_datum *usr = datum;
+
+	if (ebitmap_get_highest_set_bit(&usr->roles) >= p->p_roles.nprim) {
+		pr_err("SELinux: user %s authorizes an out-of-range role\n",
+		       (const char *)key);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 /* clang-format off */
 static int (*const index_f[SYM_NUM])(void *key, void *datum, void *datap) = {
 	common_index,
@@ -836,6 +870,14 @@ static int policydb_index(struct policydb *p)
 			goto out;
 	}
 
+	rc = hashtab_map(&p->p_roles.table, role_index_check, p);
+	if (rc)
+		goto out;
+
+	rc = hashtab_map(&p->p_users.table, user_index_check, p);
+	if (rc)
+		goto out;
+
 	rc = 0;
 out:
 	return rc;
-- 
2.55.0

Re: [PATCH 3/5] selinux: bounds-check role and user membership bitmaps at load

[sashiko-bot]

> From: Christian Göttsche <cgzones@googlemail.com>
> 
> role->dominates, role->types and user->roles are read as ebitmaps but
> their set bits are never checked against the role and type symbol tables.
> An out-of-range bit is used to index those tables later; in particular
> role_bounds_sanity_check() passes such a bit straight to sym_name(),
> reading p->sym_val_to_name[] out of bounds on a crafted bounded role.
> 
> Validate the three bitmaps in policydb_index() once every symbol table is
> populated (roles are read before types, so this cannot be done in
> role_read()).
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903113538.72942-1-cgoettsche@seltendoof.de?part=3

Re: [PATCH 3/5] selinux: bounds-check role and user membership bitmaps at load

[Stephen Smalley]

On Thu, Sep 3, 2026 at 7:36 AM Christian Göttsche
<cgoettsche@seltendoof.de> wrote:
>
> From: Christian Göttsche <cgzones@googlemail.com>
>
> role->dominates, role->types and user->roles are read as ebitmaps but
> their set bits are never checked against the role and type symbol tables.
> An out-of-range bit is used to index those tables later; in particular
> role_bounds_sanity_check() passes such a bit straight to sym_name(),
> reading p->sym_val_to_name[] out of bounds on a crafted bounded role.
>
> Validate the three bitmaps in policydb_index() once every symbol table is
> populated (roles are read before types, so this cannot be done in
> role_read()).
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

The check is against nprim rather than defined so an unclaimed value
under nprim can still hit sym_name() == NULL and pr_err() prints
"(null)". Not blocking.

Reviewed-by: Stephen Smalley <stephen.smalley.work@gmail.com>

[PATCH 4/5] selinux: validate user MLS range and default level at load

[Christian Göttsche]

From: Christian Göttsche <cgzones@googlemail.com>

user_read() reads each user's MLS range and default level but, unlike
range transitions and security contexts, never validates them.
Validate usr->range and usr->dfltlevel in user_index_check() when MLS is
enabled, once the level and category symbol tables are populated.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 security/selinux/ss/policydb.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index 3bef28434180..efa8df26791d 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -738,6 +738,20 @@ static int user_index_check(void *key, void *datum, void *datap)
 		return -EINVAL;
 	}
 
+	if (p->mls_enabled) {
+		if (!mls_range_isvalid(p, &usr->range)) {
+			pr_err("SELinux: user %s has an invalid MLS range\n",
+			       (const char *)key);
+			return -EINVAL;
+		}
+
+		if (!mls_level_isvalid(p, &usr->dfltlevel)) {
+			pr_err("SELinux: user %s has an invalid MLS default level\n",
+			       (const char *)key);
+			return -EINVAL;
+		}
+	}
+
 	return 0;
 }
 
-- 
2.55.0

Re: [PATCH 4/5] selinux: validate user MLS range and default level at load

[sashiko-bot]

> From: Christian Göttsche <cgzones@googlemail.com>
> 
> user_read() reads each user's MLS range and default level but, unlike
> range transitions and security contexts, never validates them.
> Validate usr->range and usr->dfltlevel in user_index_check() when MLS is
> enabled, once the level and category symbol tables are populated.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903113538.72942-1-cgoettsche@seltendoof.de?part=4

Re: [PATCH 4/5] selinux: validate user MLS range and default level at load

[Stephen Smalley]

On Thu, Sep 3, 2026 at 7:36 AM Christian Göttsche
<cgoettsche@seltendoof.de> wrote:
>
> From: Christian Göttsche <cgzones@googlemail.com>
>
> user_read() reads each user's MLS range and default level but, unlike
> range transitions and security contexts, never validates them.
> Validate usr->range and usr->dfltlevel in user_index_check() when MLS is
> enabled, once the level and category symbol tables are populated.
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

Reviewed-by: Stephen Smalley <stephen.smalley.work@gmail.com>

[PATCH 5/5] selinux: validate permissive and neveraudit map types at load

[Christian Göttsche]

From: Christian Göttsche <cgzones@googlemail.com>

The permissive and neveraudit maps are read as ebitmaps and indexed by
type value, but their set bits are never checked. A bit that is not a
defined, non-attribute type is silently accepted. Validate every set bit
of both maps in policydb_index() with policydb_simpletype_isvalid().

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 security/selinux/ss/policydb.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index efa8df26791d..c901a0148034 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -892,6 +892,28 @@ static int policydb_index(struct policydb *p)
 	if (rc)
 		goto out;
 
+	{
+		struct ebitmap_node *node;
+		u32 bit;
+
+		ebitmap_for_each_positive_bit(&p->permissive_map, node, bit) {
+			if (!policydb_simpletype_isvalid(p, bit)) {
+				pr_err("SELinux: permissive map refers to invalid type %u\n",
+				       bit);
+				rc = -EINVAL;
+				goto out;
+			}
+		}
+		ebitmap_for_each_positive_bit(&p->neveraudit_map, node, bit) {
+			if (!policydb_simpletype_isvalid(p, bit)) {
+				pr_err("SELinux: neveraudit map refers to invalid type %u\n",
+				       bit);
+				rc = -EINVAL;
+				goto out;
+			}
+		}
+	}
+
 	rc = 0;
 out:
 	return rc;
-- 
2.55.0

Re: [PATCH 5/5] selinux: validate permissive and neveraudit map types at load

[sashiko-bot]

> From: Christian Göttsche <cgzones@googlemail.com>
> 
> The permissive and neveraudit maps are read as ebitmaps and indexed by
> type value, but their set bits are never checked. A bit that is not a
> defined, non-attribute type is silently accepted. Validate every set bit
> of both maps in policydb_index() with policydb_simpletype_isvalid().
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903113538.72942-1-cgoettsche@seltendoof.de?part=5

Re: [PATCH 5/5] selinux: validate permissive and neveraudit map types at load

[Stephen Smalley]

On Thu, Sep 3, 2026 at 7:36 AM Christian Göttsche
<cgoettsche@seltendoof.de> wrote:
>
> From: Christian Göttsche <cgzones@googlemail.com>
>
> The permissive and neveraudit maps are read as ebitmaps and indexed by
> type value, but their set bits are never checked. A bit that is not a
> defined, non-attribute type is silently accepted. Validate every set bit
> of both maps in policydb_index() with policydb_simpletype_isvalid().
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  security/selinux/ss/policydb.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index efa8df26791d..c901a0148034 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -892,6 +892,28 @@ static int policydb_index(struct policydb *p)
>         if (rc)
>                 goto out;
>
> +       {
> +               struct ebitmap_node *node;
> +               u32 bit;

I am not a fan of embedded { } blocks inside a function but defer to Paul.
Otherwise,
Reviewed-by: Stephen Smalley <stephen.smalley.work@gmail.com>

[PATCH 1/5] selinux: reject duplicate symbol values during policy load

[Christian Göttsche]

From: Christian Göttsche <cgzones@googlemail.com>

The symbol index functions assign unconditionally, so two symbols that
share a value silently overwrite each other and leave another value with
no entry.
Reject a value whose slot is already populated, for commons, classes,
roles, types, users, sensitivities and categories;
booleans already have their own "declared but not defined" check.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 security/selinux/ss/policydb.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index 4342258cb148..3451a0295cc2 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -560,6 +560,9 @@ static int common_index(void *key, void *datum, void *datap)
 	if (!comdatum->value || comdatum->value > p->p_commons.nprim)
 		return -EINVAL;
 
+	if (p->sym_val_to_name[SYM_COMMONS][comdatum->value - 1])
+		return -EINVAL;
+
 	p->sym_val_to_name[SYM_COMMONS][comdatum->value - 1] = key;
 
 	return 0;
@@ -575,6 +578,9 @@ static int class_index(void *key, void *datum, void *datap)
 	if (!cladatum->value || cladatum->value > p->p_classes.nprim)
 		return -EINVAL;
 
+	if (p->class_val_to_struct[cladatum->value - 1])
+		return -EINVAL;
+
 	p->sym_val_to_name[SYM_CLASSES][cladatum->value - 1] = key;
 	p->class_val_to_struct[cladatum->value - 1] = cladatum;
 	return 0;
@@ -591,6 +597,9 @@ static int role_index(void *key, void *datum, void *datap)
 	    role->bounds > p->p_roles.nprim)
 		return -EINVAL;
 
+	if (p->role_val_to_struct[role->value - 1])
+		return -EINVAL;
+
 	p->sym_val_to_name[SYM_ROLES][role->value - 1] = key;
 	p->role_val_to_struct[role->value - 1] = role;
 	return 0;
@@ -613,6 +622,8 @@ static int type_index(void *key, void *datum, void *datap)
 	}
 
 	if (typdatum->primary) {
+		if (p->type_val_to_struct[typdatum->value - 1])
+			return -EINVAL;
 		p->sym_val_to_name[SYM_TYPES][typdatum->value - 1] = key;
 		p->type_val_to_struct[typdatum->value - 1] = typdatum;
 	}
@@ -631,6 +642,9 @@ static int user_index(void *key, void *datum, void *datap)
 	    usrdatum->bounds > p->p_users.nprim)
 		return -EINVAL;
 
+	if (p->user_val_to_struct[usrdatum->value - 1])
+		return -EINVAL;
+
 	p->sym_val_to_name[SYM_USERS][usrdatum->value - 1] = key;
 	p->user_val_to_struct[usrdatum->value - 1] = usrdatum;
 	return 0;
@@ -647,8 +661,11 @@ static int sens_index(void *key, void *datum, void *datap)
 	if (!levdatum->level.sens || levdatum->level.sens > p->p_levels.nprim)
 		return -EINVAL;
 
-	if (!levdatum->isalias)
+	if (!levdatum->isalias) {
+		if (p->sym_val_to_name[SYM_LEVELS][levdatum->level.sens - 1])
+			return -EINVAL;
 		p->sym_val_to_name[SYM_LEVELS][levdatum->level.sens - 1] = key;
+	}
 
 	return 0;
 }
@@ -664,8 +681,11 @@ static int cat_index(void *key, void *datum, void *datap)
 	if (!catdatum->value || catdatum->value > p->p_cats.nprim)
 		return -EINVAL;
 
-	if (!catdatum->isalias)
+	if (!catdatum->isalias) {
+		if (p->sym_val_to_name[SYM_CATS][catdatum->value - 1])
+			return -EINVAL;
 		p->sym_val_to_name[SYM_CATS][catdatum->value - 1] = key;
+	}
 
 	return 0;
 }
-- 
2.55.0

Re: [PATCH 1/5] selinux: reject duplicate symbol values during policy load

[sashiko-bot]

> From: Christian Göttsche <cgzones@googlemail.com>
> 
> The symbol index functions assign unconditionally, so two symbols that
> share a value silently overwrite each other and leave another value with
> no entry.
> Reject a value whose slot is already populated, for commons, classes,
> roles, types, users, sensitivities and categories;
> booleans already have their own "declared but not defined" check.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903113538.72942-1-cgoettsche@seltendoof.de?part=1

Re: [PATCH 1/5] selinux: reject duplicate symbol values during policy load

[Stephen Smalley]

On Thu, Sep 3, 2026 at 7:36 AM Christian Göttsche
<cgoettsche@seltendoof.de> wrote:
>
> From: Christian Göttsche <cgzones@googlemail.com>
>
> The symbol index functions assign unconditionally, so two symbols that
> share a value silently overwrite each other and leave another value with
> no entry.
> Reject a value whose slot is already populated, for commons, classes,
> roles, types, users, sensitivities and categories;
> booleans already have their own "declared but not defined" check.
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

Reviewed-by: Stephen Smalley <stephen.smalley.work@gmail.com>

Re: [PATCH 2/5] selinux: bounds-check filename transition source types at load

[sashiko-bot]

> From: Christian Göttsche <cgzones@googlemail.com>
> 
> filename_trans_read_helper() validates the target type, target class and
> each datum's otype, but the per-datum stypes ebitmap is read without any
> bound on its set bits.
> Reject a stypes bitmap whose highest set bit is not a defined type.
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903113538.72942-1-cgoettsche@seltendoof.de?part=2

Re: [PATCH 2/5] selinux: bounds-check filename transition source types at load

[Stephen Smalley]

On Thu, Sep 3, 2026 at 7:36 AM Christian Göttsche
<cgoettsche@seltendoof.de> wrote:
>
> From: Christian Göttsche <cgzones@googlemail.com>
>
> filename_trans_read_helper() validates the target type, target class and
> each datum's otype, but the per-datum stypes ebitmap is read without any
> bound on its set bits.
> Reject a stypes bitmap whose highest set bit is not a defined type.
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

Technically this will reject a policy with an empty stypes bitmap and
no types (p->p_types.nprim == 0) but that's fine since no real policy
should do that.

Reviewed-by: Stephen Smalley <stephen.smalley.work@gmail.com>

Re: [PATCH 2/5] selinux: bounds-check filename transition source types at load

[Christian Göttsche]

On Thu, 3 Sept 2026 at 15:47, Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
>
> On Thu, Sep 3, 2026 at 7:36 AM Christian Göttsche
> <cgoettsche@seltendoof.de> wrote:
> >
> > From: Christian Göttsche <cgzones@googlemail.com>
> >
> > filename_trans_read_helper() validates the target type, target class and
> > each datum's otype, but the per-datum stypes ebitmap is read without any
> > bound on its set bits.
> > Reject a stypes bitmap whose highest set bit is not a defined type.
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
>
> Technically this will reject a policy with an empty stypes bitmap and
> no types (p->p_types.nprim == 0) but that's fine since no real policy
> should do that.

Since a valid policy needs at least one entry in the global avtab,
isn't at least one type already required?

> Reviewed-by: Stephen Smalley <stephen.smalley.work@gmail.com>

Re: [PATCH 2/5] selinux: bounds-check filename transition source types at load

[Stephen Smalley]

On Thu, Sep 3, 2026 at 11:18 AM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> On Thu, 3 Sept 2026 at 15:47, Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> >
> > On Thu, Sep 3, 2026 at 7:36 AM Christian Göttsche
> > <cgoettsche@seltendoof.de> wrote:
> > >
> > > From: Christian Göttsche <cgzones@googlemail.com>
> > >
> > > filename_trans_read_helper() validates the target type, target class and
> > > each datum's otype, but the per-datum stypes ebitmap is read without any
> > > bound on its set bits.
> > > Reject a stypes bitmap whose highest set bit is not a defined type.
> > >
> > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> >
> > Technically this will reject a policy with an empty stypes bitmap and
> > no types (p->p_types.nprim == 0) but that's fine since no real policy
> > should do that.
>
> Since a valid policy needs at least one entry in the global avtab,
> isn't at least one type already required?

Yes, that seems perfectly reasonable to me.

>
> > Reviewed-by: Stephen Smalley <stephen.smalley.work@gmail.com>