| Message ID | 20220125135022.25513-1-cgzones@googlemail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | libsepol: invert only valid range of role bitmap | expand |
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 >
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 > >
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;
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(-)