selinux: ensure the context is NUL terminated in security_context_to_sid_core()

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
NUL 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 NUL 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 <syzkaller@googlegroups.com>
Signed-off-by: Paul Moore <paul@paul-moore.com>
Reviewed-By: William Roberts <william.c.roberts@intel.com>

diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 33cfe5d3d6cb8cbda6927310aa10954f5d01f3a1..d05496deb2295a4035e4fab767fdf2315fc6ae67 100644 (file)
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -1413,27 +1413,25 @@ 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 NUL terminator */
+       scontext2 = kmemdup_nul(scontext, scontext_len, gfp_flags);
+       if (!scontext2)
+               return -ENOMEM;
+
        if (!ss_initialized) {
                int i;
 
                for (i = 1; i < SECINITSID_NUM; i++) {
-                       if (!strcmp(initial_sid_to_string[i], scontext)) {
+                       if (!strcmp(initial_sid_to_string[i], scontext2)) {
                                *sid = i;
-                               return 0;
+                               goto out;
                        }
                }
                *sid = SECINITSID_KERNEL;
-               return 0;
+               goto out;
        }
        *sid = SECSID_NULL;
 
-       /* Copy the string so that we can modify the copy as we parse it. */
-       scontext2 = kmalloc(scontext_len + 1, gfp_flags);
-       if (!scontext2)
-               return -ENOMEM;
-       memcpy(scontext2, scontext, scontext_len);
-       scontext2[scontext_len] = 0;
-
        if (force) {
                /* Save another copy for storing in uninterpreted form */
                rc = -ENOMEM;