On Fri, 2017-12-01 at 10:34 -0500, Paul Moore wrote: > On Thu, Nov 30, 2017 at 6:44 PM, William Roberts > <bill.c.robe...@gmail.com> wrote: > > On Thu, Nov 30, 2017 at 8:52 AM, Paul Moore <pmo...@redhat.com> > > wrote: > > > From: Paul Moore <p...@paul-moore.com> > > > > > > The syzbot/syzkaller automated tests found a problem in > > > security_context_to_sid_core() during early boot (before we load > > > the > > > SELinux policy) where we could potentially feed context strings > > > without > > > NULL terminators into the strcmp() function. > > > > > > We already guard against this during normal operation (after the > > > SELinux > > > policy has been loaded) by making a copy of the context strings > > > and > > > explicitly adding a NULL terminator to the end. The patch > > > extends this > > > protection to the early boot case (no loaded policy) by moving > > > the context > > > copy earlier in security_context_to_sid_core(). > > > > > > Reported-by: syzbot <syzkal...@googlegroups.com> > > > Signed-off-by: Paul Moore <p...@paul-moore.com> > > > --- > > > security/selinux/ss/services.c | 20 ++++++++++---------- > > > 1 file changed, 10 insertions(+), 10 deletions(-) > > > > > > diff --git a/security/selinux/ss/services.c > > > b/security/selinux/ss/services.c > > > index 33cfe5d3d6cb..6ec4cdc8af8f 100644 > > > --- a/security/selinux/ss/services.c > > > +++ b/security/selinux/ss/services.c > > > @@ -1413,27 +1413,27 @@ static int > > > security_context_to_sid_core(const char *scontext, u32 > > > scontext_len, > > > if (!scontext_len) > > > return -EINVAL; > > > > > > + /* Copy the string to allow changes and ensure a NULL > > > terminator */ > > > + scontext2 = kmalloc(scontext_len + 1, gfp_flags); > > > + if (!scontext2) > > > + return -ENOMEM; > > > + memcpy(scontext2, scontext, scontext_len); > > > + scontext2[scontext_len] = 0; > > > > Call me crazy, but can't we use kmemdup_nul() here? > > Crazy good idea ;) > > I didn't realize that function existed, I'll respin. Thanks.
Also note that it should be NUL not NULL in the patch subject and description. '\0' vs (void*)0 > > > /** > > * kmemdup_nul - Create a NUL-terminated string from unterminated > > data > > * @s: The data to stringify > > * @len: The size of the data > > * @gfp: the GFP mask used in the kmalloc() call when allocating > > memory > > */ > > char *kmemdup_nul(const char *s, size_t len, gfp_t gfp) > >
