[PATCH 1/3] selinux: detect invalid ebitmap

[PATCH 1/3] selinux: detect invalid ebitmap

[william.c.roberts]

From: William Roberts <william.c.roberts@intel.com>

When count is 0 and the highbit is not zero, the ebitmap is not
valid and the internal node is not allocated. This causes issues
when routines, like mls_context_isvalid() attempt to use the
ebitmap_for_each_bit() and ebitmap_node_get_bit() as they assume
a highbit > 0 will have a node allocated.
---
 security/selinux/ss/ebitmap.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/security/selinux/ss/ebitmap.c b/security/selinux/ss/ebitmap.c
index 894b6cd..7d10e5d 100644
--- a/security/selinux/ss/ebitmap.c
+++ b/security/selinux/ss/ebitmap.c
@@ -374,6 +374,9 @@ int ebitmap_read(struct ebitmap *e, void *fp)
 		goto ok;
 	}
 
+	if (e->highbit && !count)
+		goto bad;
+
 	for (i = 0; i < count; i++) {
 		rc = next_entry(&startbit, fp, sizeof(u32));
 		if (rc < 0) {
-- 
1.9.1

[PATCH 2/3] selinux: initialize structures

[william.c.roberts]

From: William Roberts <william.c.roberts@intel.com>

libsepol pointed out an issue where its possible to have
an unitialized jmp and invalid dereference, fix this.
While we're here, zero allocate all the *_val_to_struct
structures.

Signed-off-by: William Roberts <william.c.roberts@intel.com>
---
 security/selinux/ss/policydb.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index 992a315..4b24385 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -541,21 +541,21 @@ static int policydb_index(struct policydb *p)
 
 	rc = -ENOMEM;
 	p->class_val_to_struct =
-		kmalloc(p->p_classes.nprim * sizeof(*(p->class_val_to_struct)),
+		kzalloc(p->p_classes.nprim * sizeof(*(p->class_val_to_struct)),
 			GFP_KERNEL);
 	if (!p->class_val_to_struct)
 		goto out;
 
 	rc = -ENOMEM;
 	p->role_val_to_struct =
-		kmalloc(p->p_roles.nprim * sizeof(*(p->role_val_to_struct)),
+		kzalloc(p->p_roles.nprim * sizeof(*(p->role_val_to_struct)),
 			GFP_KERNEL);
 	if (!p->role_val_to_struct)
 		goto out;
 
 	rc = -ENOMEM;
 	p->user_val_to_struct =
-		kmalloc(p->p_users.nprim * sizeof(*(p->user_val_to_struct)),
+		kzalloc(p->p_users.nprim * sizeof(*(p->user_val_to_struct)),
 			GFP_KERNEL);
 	if (!p->user_val_to_struct)
 		goto out;
@@ -964,7 +964,7 @@ int policydb_context_isvalid(struct policydb *p, struct context *c)
 		 * Role must be authorized for the type.
 		 */
 		role = p->role_val_to_struct[c->role - 1];
-		if (!ebitmap_get_bit(&role->types, c->type - 1))
+		if (!role || !ebitmap_get_bit(&role->types, c->type - 1))
 			/* role may not be associated with type */
 			return 0;
 
-- 
1.9.1

[PATCH 3/3] selinux: fix overflow and 0 length allocations

[william.c.roberts]

From: William Roberts <william.c.roberts@intel.com>

Throughout the SE Linux LSM, values taken from sepolicy are
used in places where length == 0 or length == <saturated>
matter, find and fix these.

Signed-off-by: William Roberts <william.c.roberts@intel.com>
---
 security/selinux/ss/conditional.c | 3 +++
 security/selinux/ss/policydb.c    | 4 ++++
 security/selinux/ss/private.h     | 7 +++++++
 3 files changed, 14 insertions(+)
 create mode 100644 security/selinux/ss/private.h

diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c
index 456e1a9..ecc0fb6 100644
--- a/security/selinux/ss/conditional.c
+++ b/security/selinux/ss/conditional.c
@@ -16,6 +16,7 @@
 #include "security.h"
 #include "conditional.h"
 #include "services.h"
+#include "private.h"
 
 /*
  * cond_evaluate_expr evaluates a conditional expr
@@ -242,6 +243,8 @@ int cond_read_bool(struct policydb *p, struct hashtab *h, void *fp)
 		goto err;
 
 	len = le32_to_cpu(buf[2]);
+	if (zero_or_saturated(len))
+		goto err;
 
 	rc = -ENOMEM;
 	key = kmalloc(len + 1, GFP_KERNEL);
diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
index 4b24385..0e881f3 100644
--- a/security/selinux/ss/policydb.c
+++ b/security/selinux/ss/policydb.c
@@ -38,6 +38,7 @@
 #include "conditional.h"
 #include "mls.h"
 #include "services.h"
+#include "private.h"
 
 #define _DEBUG_HASHES
 
@@ -1094,6 +1095,9 @@ static int str_read(char **strp, gfp_t flags, void *fp, u32 len)
 	int rc;
 	char *str;
 
+	if (zero_or_saturated(len))
+		return -EINVAL;
+
 	str = kmalloc(len + 1, flags);
 	if (!str)
 		return -ENOMEM;
diff --git a/security/selinux/ss/private.h b/security/selinux/ss/private.h
new file mode 100644
index 0000000..0e81a78
--- /dev/null
+++ b/security/selinux/ss/private.h
@@ -0,0 +1,7 @@
+#ifndef PRIVATE_H_
+#define PRIVATE_H_
+
+#define is_saturated(x) (x == (typeof(x))-1)
+#define zero_or_saturated(x) ((x == 0) || is_saturated(x))
+
+#endif
-- 
1.9.1

Re: [PATCH 3/3] selinux: fix overflow and 0 length allocations

[Paul Moore]

On Tue, Aug 23, 2016 at 4:49 PM,  <william.c.roberts@intel.com> wrote:
> From: William Roberts <william.c.roberts@intel.com>
>
> Throughout the SE Linux LSM, values taken from sepolicy are

I'll take a closer look at this patchset after LinuxCon/LSS, but
thanks for doing this ... however, one little bikeshed thing that
drives me crazy is the use of "SE Linux" instead of "SELinux".  Don't
change anything you've submitted, but in the future please use SELinux
;)

> used in places where length == 0 or length == <saturated>
> matter, find and fix these.
>
> Signed-off-by: William Roberts <william.c.roberts@intel.com>
> ---
>  security/selinux/ss/conditional.c | 3 +++
>  security/selinux/ss/policydb.c    | 4 ++++
>  security/selinux/ss/private.h     | 7 +++++++
>  3 files changed, 14 insertions(+)
>  create mode 100644 security/selinux/ss/private.h
>
> diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c
> index 456e1a9..ecc0fb6 100644
> --- a/security/selinux/ss/conditional.c
> +++ b/security/selinux/ss/conditional.c
> @@ -16,6 +16,7 @@
>  #include "security.h"
>  #include "conditional.h"
>  #include "services.h"
> +#include "private.h"
>
>  /*
>   * cond_evaluate_expr evaluates a conditional expr
> @@ -242,6 +243,8 @@ int cond_read_bool(struct policydb *p, struct hashtab *h, void *fp)
>                 goto err;
>
>         len = le32_to_cpu(buf[2]);
> +       if (zero_or_saturated(len))
> +               goto err;
>
>         rc = -ENOMEM;
>         key = kmalloc(len + 1, GFP_KERNEL);
> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index 4b24385..0e881f3 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -38,6 +38,7 @@
>  #include "conditional.h"
>  #include "mls.h"
>  #include "services.h"
> +#include "private.h"
>
>  #define _DEBUG_HASHES
>
> @@ -1094,6 +1095,9 @@ static int str_read(char **strp, gfp_t flags, void *fp, u32 len)
>         int rc;
>         char *str;
>
> +       if (zero_or_saturated(len))
> +               return -EINVAL;
> +
>         str = kmalloc(len + 1, flags);
>         if (!str)
>                 return -ENOMEM;
> diff --git a/security/selinux/ss/private.h b/security/selinux/ss/private.h
> new file mode 100644
> index 0000000..0e81a78
> --- /dev/null
> +++ b/security/selinux/ss/private.h
> @@ -0,0 +1,7 @@
> +#ifndef PRIVATE_H_
> +#define PRIVATE_H_
> +
> +#define is_saturated(x) (x == (typeof(x))-1)
> +#define zero_or_saturated(x) ((x == 0) || is_saturated(x))
> +
> +#endif
> --
> 1.9.1
>
> _______________________________________________
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.



-- 
paul moore
www.paul-moore.com

Re: [PATCH 1/3] selinux: detect invalid ebitmap

[Paul Moore]

On Tue, Aug 23, 2016 at 4:49 PM,  <william.c.roberts@intel.com> wrote:
> From: William Roberts <william.c.roberts@intel.com>
>
> When count is 0 and the highbit is not zero, the ebitmap is not
> valid and the internal node is not allocated. This causes issues
> when routines, like mls_context_isvalid() attempt to use the
> ebitmap_for_each_bit() and ebitmap_node_get_bit() as they assume
> a highbit > 0 will have a node allocated.
> ---
>  security/selinux/ss/ebitmap.c | 3 +++
>  1 file changed, 3 insertions(+)

Hi William,

This patch looks good to me, but do I have your permission to add your sign-off?

> diff --git a/security/selinux/ss/ebitmap.c b/security/selinux/ss/ebitmap.c
> index 894b6cd..7d10e5d 100644
> --- a/security/selinux/ss/ebitmap.c
> +++ b/security/selinux/ss/ebitmap.c
> @@ -374,6 +374,9 @@ int ebitmap_read(struct ebitmap *e, void *fp)
>                 goto ok;
>         }
>
> +       if (e->highbit && !count)
> +               goto bad;
> +
>         for (i = 0; i < count; i++) {
>                 rc = next_entry(&startbit, fp, sizeof(u32));
>                 if (rc < 0) {
> --
> 1.9.1
>
> _______________________________________________
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.



-- 
paul moore
www.paul-moore.com

RE: [PATCH 1/3] selinux: detect invalid ebitmap

[Roberts, William C]

> -----Original Message-----
> From: Paul Moore [mailto:paul@paul-moore.com]
> Sent: Monday, August 29, 2016 4:21 PM
> To: Roberts, William C <william.c.roberts@intel.com>
> Cc: selinux@tycho.nsa.gov; seandroid-list@tycho.nsa.gov; Stephen Smalley
> <sds@tycho.nsa.gov>
> Subject: Re: [PATCH 1/3] selinux: detect invalid ebitmap
> 
> On Tue, Aug 23, 2016 at 4:49 PM,  <william.c.roberts@intel.com> wrote:
> > From: William Roberts <william.c.roberts@intel.com>
> >
> > When count is 0 and the highbit is not zero, the ebitmap is not valid
> > and the internal node is not allocated. This causes issues when
> > routines, like mls_context_isvalid() attempt to use the
> > ebitmap_for_each_bit() and ebitmap_node_get_bit() as they assume a
> > highbit > 0 will have a node allocated.
> > ---
> >  security/selinux/ss/ebitmap.c | 3 +++
> >  1 file changed, 3 insertions(+)
> 
> Hi William,
> 
> This patch looks good to me, but do I have your permission to add your sign-off?

Yes, I guess I missed it. Just so it's easy for you to copy paste:
Signed-off-by: William Roberts <william.c.roberts@intel.com>


> 
> > diff --git a/security/selinux/ss/ebitmap.c
> > b/security/selinux/ss/ebitmap.c index 894b6cd..7d10e5d 100644
> > --- a/security/selinux/ss/ebitmap.c
> > +++ b/security/selinux/ss/ebitmap.c
> > @@ -374,6 +374,9 @@ int ebitmap_read(struct ebitmap *e, void *fp)
> >                 goto ok;
> >         }
> >
> > +       if (e->highbit && !count)
> > +               goto bad;
> > +
> >         for (i = 0; i < count; i++) {
> >                 rc = next_entry(&startbit, fp, sizeof(u32));
> >                 if (rc < 0) {
> > --
> > 1.9.1
> >
> > _______________________________________________
> > Selinux mailing list
> > Selinux@tycho.nsa.gov
> > To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> > To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
> 
> 
> 
> --
> paul moore
> www.paul-moore.com

Re: [PATCH 2/3] selinux: initialize structures

[Paul Moore]

On Tue, Aug 23, 2016 at 4:49 PM,  <william.c.roberts@intel.com> wrote:
> From: William Roberts <william.c.roberts@intel.com>
>
> libsepol pointed out an issue where its possible to have
> an unitialized jmp and invalid dereference, fix this.
> While we're here, zero allocate all the *_val_to_struct
> structures.
>
> Signed-off-by: William Roberts <william.c.roberts@intel.com>
> ---
>  security/selinux/ss/policydb.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Merged, thanks.

> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index 992a315..4b24385 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -541,21 +541,21 @@ static int policydb_index(struct policydb *p)
>
>         rc = -ENOMEM;
>         p->class_val_to_struct =
> -               kmalloc(p->p_classes.nprim * sizeof(*(p->class_val_to_struct)),
> +               kzalloc(p->p_classes.nprim * sizeof(*(p->class_val_to_struct)),
>                         GFP_KERNEL);
>         if (!p->class_val_to_struct)
>                 goto out;
>
>         rc = -ENOMEM;
>         p->role_val_to_struct =
> -               kmalloc(p->p_roles.nprim * sizeof(*(p->role_val_to_struct)),
> +               kzalloc(p->p_roles.nprim * sizeof(*(p->role_val_to_struct)),
>                         GFP_KERNEL);
>         if (!p->role_val_to_struct)
>                 goto out;
>
>         rc = -ENOMEM;
>         p->user_val_to_struct =
> -               kmalloc(p->p_users.nprim * sizeof(*(p->user_val_to_struct)),
> +               kzalloc(p->p_users.nprim * sizeof(*(p->user_val_to_struct)),
>                         GFP_KERNEL);
>         if (!p->user_val_to_struct)
>                 goto out;
> @@ -964,7 +964,7 @@ int policydb_context_isvalid(struct policydb *p, struct context *c)
>                  * Role must be authorized for the type.
>                  */
>                 role = p->role_val_to_struct[c->role - 1];
> -               if (!ebitmap_get_bit(&role->types, c->type - 1))
> +               if (!role || !ebitmap_get_bit(&role->types, c->type - 1))
>                         /* role may not be associated with type */
>                         return 0;
>
> --
> 1.9.1
>
> _______________________________________________
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.



-- 
paul moore
www.paul-moore.com

Re: [PATCH 1/3] selinux: detect invalid ebitmap

[Paul Moore]

On Mon, Aug 29, 2016 at 7:26 PM, Roberts, William C
<william.c.roberts@intel.com> wrote:
>> -----Original Message-----
>> From: Paul Moore [mailto:paul@paul-moore.com]
>> Sent: Monday, August 29, 2016 4:21 PM
>> To: Roberts, William C <william.c.roberts@intel.com>
>> Cc: selinux@tycho.nsa.gov; seandroid-list@tycho.nsa.gov; Stephen Smalley
>> <sds@tycho.nsa.gov>
>> Subject: Re: [PATCH 1/3] selinux: detect invalid ebitmap
>>
>> On Tue, Aug 23, 2016 at 4:49 PM,  <william.c.roberts@intel.com> wrote:
>> > From: William Roberts <william.c.roberts@intel.com>
>> >
>> > When count is 0 and the highbit is not zero, the ebitmap is not valid
>> > and the internal node is not allocated. This causes issues when
>> > routines, like mls_context_isvalid() attempt to use the
>> > ebitmap_for_each_bit() and ebitmap_node_get_bit() as they assume a
>> > highbit > 0 will have a node allocated.
>> > ---
>> >  security/selinux/ss/ebitmap.c | 3 +++
>> >  1 file changed, 3 insertions(+)
>>
>> Hi William,
>>
>> This patch looks good to me, but do I have your permission to add your sign-off?
>
> Yes, I guess I missed it. Just so it's easy for you to copy paste:
> Signed-off-by: William Roberts <william.c.roberts@intel.com>

Great, thanks!

>> > diff --git a/security/selinux/ss/ebitmap.c
>> > b/security/selinux/ss/ebitmap.c index 894b6cd..7d10e5d 100644
>> > --- a/security/selinux/ss/ebitmap.c
>> > +++ b/security/selinux/ss/ebitmap.c
>> > @@ -374,6 +374,9 @@ int ebitmap_read(struct ebitmap *e, void *fp)
>> >                 goto ok;
>> >         }
>> >
>> > +       if (e->highbit && !count)
>> > +               goto bad;
>> > +
>> >         for (i = 0; i < count; i++) {
>> >                 rc = next_entry(&startbit, fp, sizeof(u32));
>> >                 if (rc < 0) {
>> > --
>> > 1.9.1
>> >
>> > _______________________________________________
>> > Selinux mailing list
>> > Selinux@tycho.nsa.gov
>> > To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
>> > To get help, send an email containing "help" to Selinux-request@tycho.nsa.gov.
>>
>>
>>
>> --
>> paul moore
>> www.paul-moore.com



-- 
paul moore
www.paul-moore.com

Re: [PATCH 3/3] selinux: fix overflow and 0 length allocations

[Paul Moore]

On Tue, Aug 23, 2016 at 4:49 PM,  <william.c.roberts@intel.com> wrote:
> From: William Roberts <william.c.roberts@intel.com>
>
> Throughout the SE Linux LSM, values taken from sepolicy are
> used in places where length == 0 or length == <saturated>
> matter, find and fix these.
>
> Signed-off-by: William Roberts <william.c.roberts@intel.com>
> ---
>  security/selinux/ss/conditional.c | 3 +++
>  security/selinux/ss/policydb.c    | 4 ++++
>  security/selinux/ss/private.h     | 7 +++++++
>  3 files changed, 14 insertions(+)
>  create mode 100644 security/selinux/ss/private.h
>
> diff --git a/security/selinux/ss/conditional.c b/security/selinux/ss/conditional.c
> index 456e1a9..ecc0fb6 100644
> --- a/security/selinux/ss/conditional.c
> +++ b/security/selinux/ss/conditional.c
> @@ -16,6 +16,7 @@
>  #include "security.h"
>  #include "conditional.h"
>  #include "services.h"
> +#include "private.h"
>
>  /*
>   * cond_evaluate_expr evaluates a conditional expr
> @@ -242,6 +243,8 @@ int cond_read_bool(struct policydb *p, struct hashtab *h, void *fp)
>                 goto err;
>
>         len = le32_to_cpu(buf[2]);
> +       if (zero_or_saturated(len))
> +               goto err;
>
>         rc = -ENOMEM;
>         key = kmalloc(len + 1, GFP_KERNEL);
> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index 4b24385..0e881f3 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -38,6 +38,7 @@
>  #include "conditional.h"
>  #include "mls.h"
>  #include "services.h"
> +#include "private.h"
>
>  #define _DEBUG_HASHES
>
> @@ -1094,6 +1095,9 @@ static int str_read(char **strp, gfp_t flags, void *fp, u32 len)
>         int rc;
>         char *str;
>
> +       if (zero_or_saturated(len))
> +               return -EINVAL;
> +
>         str = kmalloc(len + 1, flags);
>         if (!str)
>                 return -ENOMEM;
> diff --git a/security/selinux/ss/private.h b/security/selinux/ss/private.h
> new file mode 100644
> index 0000000..0e81a78
> --- /dev/null
> +++ b/security/selinux/ss/private.h
> @@ -0,0 +1,7 @@
> +#ifndef PRIVATE_H_
> +#define PRIVATE_H_
> +
> +#define is_saturated(x) (x == (typeof(x))-1)
> +#define zero_or_saturated(x) ((x == 0) || is_saturated(x))
> +
> +#endif

While I'm not opposed to the idea of using a macro for this purpose,
e.g. is_saturated() and zero_or_saturated(), I don't see much value in
creating this macro buried in the SELinux directory.  Especially if we
end up creating a new header file just for these macros.  Something as
generic as this should be something we inherit from the generic kernel
code so we can leverage existing conventions.

If you can find a macro like these in the core kernel code, go ahead
and use it, otherwise please respin this with the bound checks open
coded.

Oh, and use "SELinux" ;)

-- 
paul moore
www.paul-moore.com

Re: [PATCH 3/3] selinux: fix overflow and 0 length allocations

[William Roberts]

[-- Attachment #1: Type: text/plain, Size: 3652 bytes --]

On Aug 29, 2016 16:56, "Paul Moore" <paul@paul-moore.com> wrote:
>
> On Tue, Aug 23, 2016 at 4:49 PM,  <william.c.roberts@intel.com> wrote:
> > From: William Roberts <william.c.roberts@intel.com>
> >
> > Throughout the SE Linux LSM, values taken from sepolicy are
> > used in places where length == 0 or length == <saturated>
> > matter, find and fix these.
> >
> > Signed-off-by: William Roberts <william.c.roberts@intel.com>
> > ---
> >  security/selinux/ss/conditional.c | 3 +++
> >  security/selinux/ss/policydb.c    | 4 ++++
> >  security/selinux/ss/private.h     | 7 +++++++
> >  3 files changed, 14 insertions(+)
> >  create mode 100644 security/selinux/ss/private.h
> >
> > diff --git a/security/selinux/ss/conditional.c
b/security/selinux/ss/conditional.c
> > index 456e1a9..ecc0fb6 100644
> > --- a/security/selinux/ss/conditional.c
> > +++ b/security/selinux/ss/conditional.c
> > @@ -16,6 +16,7 @@
> >  #include "security.h"
> >  #include "conditional.h"
> >  #include "services.h"
> > +#include "private.h"
> >
> >  /*
> >   * cond_evaluate_expr evaluates a conditional expr
> > @@ -242,6 +243,8 @@ int cond_read_bool(struct policydb *p, struct
hashtab *h, void *fp)
> >                 goto err;
> >
> >         len = le32_to_cpu(buf[2]);
> > +       if (zero_or_saturated(len))
> > +               goto err;
> >
> >         rc = -ENOMEM;
> >         key = kmalloc(len + 1, GFP_KERNEL);
> > diff --git a/security/selinux/ss/policydb.c
b/security/selinux/ss/policydb.c
> > index 4b24385..0e881f3 100644
> > --- a/security/selinux/ss/policydb.c
> > +++ b/security/selinux/ss/policydb.c
> > @@ -38,6 +38,7 @@
> >  #include "conditional.h"
> >  #include "mls.h"
> >  #include "services.h"
> > +#include "private.h"
> >
> >  #define _DEBUG_HASHES
> >
> > @@ -1094,6 +1095,9 @@ static int str_read(char **strp, gfp_t flags,
void *fp, u32 len)
> >         int rc;
> >         char *str;
> >
> > +       if (zero_or_saturated(len))
> > +               return -EINVAL;
> > +
> >         str = kmalloc(len + 1, flags);
> >         if (!str)
> >                 return -ENOMEM;
> > diff --git a/security/selinux/ss/private.h
b/security/selinux/ss/private.h
> > new file mode 100644
> > index 0000000..0e81a78
> > --- /dev/null
> > +++ b/security/selinux/ss/private.h
> > @@ -0,0 +1,7 @@
> > +#ifndef PRIVATE_H_
> > +#define PRIVATE_H_
> > +
> > +#define is_saturated(x) (x == (typeof(x))-1)
> > +#define zero_or_saturated(x) ((x == 0) || is_saturated(x))
> > +
> > +#endif
>
> While I'm not opposed to the idea of using a macro for this purpose,
> e.g. is_saturated() and zero_or_saturated(), I don't see much value in
> creating this macro buried in the SELinux directory.  Especially if we
> end up creating a new header file just for these macros.  Something as
> generic as this should be something we inherit from the generic kernel
> code so we can leverage existing conventions.

It made more sense when I wanted to keep the change similar to libsepol.
But the str_read() function really reduced the number of checks. I ended up
porting that to libsepol. With that said, the macro serves little purpose
with the exception of readability.

>
> If you can find a macro like these in the core kernel code, go ahead

Ill look, doubt I'll find anything.

> and use it, otherwise please respin this with the bound checks open
> code

Sure

>
> Oh, and use "SELinux" ;)
>
> --
> paul moore
> www.paul-moore.com
> _______________________________________________
> Selinux mailing list
> Selinux@tycho.nsa.gov
> To unsubscribe, send email to Selinux-leave@tycho.nsa.gov.
> To get help, send an email containing "help" to
Selinux-request@tycho.nsa.gov.

[-- Attachment #2: Type: text/html, Size: 5365 bytes --]