On Aug 29, 2016 16:56, "Paul Moore" <p...@paul-moore.com> wrote: > > On Tue, Aug 23, 2016 at 4:49 PM, <william.c.robe...@intel.com> wrote: > > From: William Roberts <william.c.robe...@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.robe...@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 > seli...@tycho.nsa.gov > To unsubscribe, send email to selinux-le...@tycho.nsa.gov. > To get help, send an email containing "help" to selinux-requ...@tycho.nsa.gov.
_______________________________________________ Seandroid-list mailing list Seandroid-list@tycho.nsa.gov To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov. To get help, send an email containing "help" to seandroid-list-requ...@tycho.nsa.gov.
