[PATCH] libsepol: invert only valid range of role bitmap

[PATCH] libsepol: invert only valid range of role bitmap

[Christian Göttsche]

The length of an ebitmap is the current highest allocated (not set) bit
and always a multiple of MAPTYPE (= 64). The role ebitmap should only
have valid role bits set, even after inverting. The length might be
smaller than the maximum number of defined roles leading to non defined
role bits set afterwards.
Only invert up to the number of roles defined instead the full ebitmap
length, similar to type_set_expand().

This also avoids timeouts on an invalid huge highbit set, since the
ebitmap has not been validated yet, on which inverting will take
excessive amount of memory and time, found by oss-fuzz (#43709).

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
This patch supersedes "libsepol: reject invalid roles before inverting"
https://patchwork.kernel.org/project/selinux/patch/20220117150200.24953-1-cgzones@googlemail.com/
---
 libsepol/src/expand.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
index 898e6b87..df8683ef 100644
--- a/libsepol/src/expand.c
+++ b/libsepol/src/expand.c
@@ -2481,7 +2481,7 @@ int role_set_expand(role_set_t * x, ebitmap_t * r, policydb_t * out, policydb_t
 
 	/* if role is to be complimented, invert the entire bitmap here */
 	if (x->flags & ROLE_COMP) {
-		for (i = 0; i < ebitmap_length(r); i++) {
+		for (i = 0; i < p->p_roles.nprim; i++) {
 			if (ebitmap_get_bit(r, i)) {
 				if (ebitmap_set_bit(r, i, 0))
 					return -1;
-- 
2.34.1

Re: [PATCH] libsepol: invert only valid range of role bitmap

[James Carter]

On Tue, Jan 25, 2022 at 2:46 PM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> The length of an ebitmap is the current highest allocated (not set) bit
> and always a multiple of MAPTYPE (= 64). The role ebitmap should only
> have valid role bits set, even after inverting. The length might be
> smaller than the maximum number of defined roles leading to non defined
> role bits set afterwards.
> Only invert up to the number of roles defined instead the full ebitmap
> length, similar to type_set_expand().
>
> This also avoids timeouts on an invalid huge highbit set, since the
> ebitmap has not been validated yet, on which inverting will take
> excessive amount of memory and time, found by oss-fuzz (#43709).
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

Acked-by: James Carter <jwcart2@gmail.com>

> ---
> This patch supersedes "libsepol: reject invalid roles before inverting"
> https://patchwork.kernel.org/project/selinux/patch/20220117150200.24953-1-cgzones@googlemail.com/
> ---
>  libsepol/src/expand.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
> index 898e6b87..df8683ef 100644
> --- a/libsepol/src/expand.c
> +++ b/libsepol/src/expand.c
> @@ -2481,7 +2481,7 @@ int role_set_expand(role_set_t * x, ebitmap_t * r, policydb_t * out, policydb_t
>
>         /* if role is to be complimented, invert the entire bitmap here */
>         if (x->flags & ROLE_COMP) {
> -               for (i = 0; i < ebitmap_length(r); i++) {
> +               for (i = 0; i < p->p_roles.nprim; i++) {
>                         if (ebitmap_get_bit(r, i)) {
>                                 if (ebitmap_set_bit(r, i, 0))
>                                         return -1;
> --
> 2.34.1
>

Re: [PATCH] libsepol: invert only valid range of role bitmap

[James Carter]

On Tue, Jan 25, 2022 at 4:17 PM James Carter <jwcart2@gmail.com> wrote:
>
> On Tue, Jan 25, 2022 at 2:46 PM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> >
> > The length of an ebitmap is the current highest allocated (not set) bit
> > and always a multiple of MAPTYPE (= 64). The role ebitmap should only
> > have valid role bits set, even after inverting. The length might be
> > smaller than the maximum number of defined roles leading to non defined
> > role bits set afterwards.
> > Only invert up to the number of roles defined instead the full ebitmap
> > length, similar to type_set_expand().
> >
> > This also avoids timeouts on an invalid huge highbit set, since the
> > ebitmap has not been validated yet, on which inverting will take
> > excessive amount of memory and time, found by oss-fuzz (#43709).
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
>
> Acked-by: James Carter <jwcart2@gmail.com>
>

Merged.
Thanks,
Jim

> > ---
> > This patch supersedes "libsepol: reject invalid roles before inverting"
> > https://patchwork.kernel.org/project/selinux/patch/20220117150200.24953-1-cgzones@googlemail.com/
> > ---
> >  libsepol/src/expand.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libsepol/src/expand.c b/libsepol/src/expand.c
> > index 898e6b87..df8683ef 100644
> > --- a/libsepol/src/expand.c
> > +++ b/libsepol/src/expand.c
> > @@ -2481,7 +2481,7 @@ int role_set_expand(role_set_t * x, ebitmap_t * r, policydb_t * out, policydb_t
> >
> >         /* if role is to be complimented, invert the entire bitmap here */
> >         if (x->flags & ROLE_COMP) {
> > -               for (i = 0; i < ebitmap_length(r); i++) {
> > +               for (i = 0; i < p->p_roles.nprim; i++) {
> >                         if (ebitmap_get_bit(r, i)) {
> >                                 if (ebitmap_set_bit(r, i, 0))
> >                                         return -1;
> > --
> > 2.34.1
> >