module: add syscall to load module from fd

As part of the effort to create a stronger boundary between root and
kernel, Chrome OS wants to be able to enforce that kernel modules are
being loaded only from our read-only crypto-hash verified (dm_verity)
root filesystem. Since the init_module syscall hands the kernel a module
as a memory blob, no reasoning about the origin of the blob can be made.

Earlier proposals for appending signatures to kernel modules would not be
useful in Chrome OS, since it would involve adding an additional set of
keys to our kernel and builds for no good reason: we already trust the
contents of our root filesystem. We don't need to verify those kernel
modules a second time. Having to do signature checking on module loading
would slow us down and be redundant. All we need to know is where a
module is coming from so we can say yes/no to loading it.

If a file descriptor is used as the source of a kernel module, many more
things can be reasoned about. In Chrome OS's case, we could enforce that
the module lives on the filesystem we expect it to live on.  In the case
of IMA (or other LSMs), it would be possible, for example, to examine
extended attributes that may contain signatures over the contents of
the module.

This introduces a new syscall (on x86), similar to init_module, that has
only two arguments. The first argument is used as a file descriptor to
the module and the second argument is a pointer to the NULL terminated
string of module arguments.

Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> (merge fixes)

5 files changed, 223 insertions, 148 deletions

diff --git a/arch/x86/syscalls/syscall_32.tbl b/arch/x86/syscalls/syscall_32.tbl
index a47103fbc692..83b3838417ed 100644
--- a/arch/x86/syscalls/syscall_32.tbl
+++ b/arch/x86/syscalls/syscall_32.tbl
@@ -356,3 +356,4 @@
 347     i386    process_vm_readv        sys_process_vm_readv            compat_sys_process_vm_readv
 348     i386    process_vm_writev       sys_process_vm_writev           compat_sys_process_vm_writev
 349     i386    kcmp                    sys_kcmp
+350     i386    finit_module            sys_finit_module
diff --git a/arch/x86/syscalls/syscall_64.tbl b/arch/x86/syscalls/syscall_64.tbl
index a582bfed95bb..7c58c84b7bc8 100644
--- a/arch/x86/syscalls/syscall_64.tbl
+++ b/arch/x86/syscalls/syscall_64.tbl
@@ -319,6 +319,7 @@
 310     64      process_vm_readv        sys_process_vm_readv
 311     64      process_vm_writev       sys_process_vm_writev
 312     common  kcmp                    sys_kcmp
+313     common  finit_module            sys_finit_module
 
 #
 # x32-specific system call numbers start at 512 to avoid cache impact
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 727f0cd73921..32bc035bcd68 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -868,4 +868,5 @@ asmlinkage long sys_process_vm_writev(pid_t pid,
 
 asmlinkage long sys_kcmp(pid_t pid1, pid_t pid2, int type,
                          unsigned long idx1, unsigned long idx2);
+asmlinkage long sys_finit_module(int fd, const char __user *uargs);
 #endif
diff --git a/kernel/module.c b/kernel/module.c
index 6e48c3a43599..6d2c4e4ca1f5 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -21,6 +21,7 @@
 #include <linux/ftrace_event.h>
 #include <linux/init.h>
 #include <linux/kallsyms.h>
+#include <linux/file.h>
 #include <linux/fs.h>
 #include <linux/sysfs.h>
 #include <linux/kernel.h>
@@ -2425,18 +2426,17 @@ static inline void kmemleak_load_module(const struct module *mod,
 #endif
 
 #ifdef CONFIG_MODULE_SIG
-static int module_sig_check(struct load_info *info,
-                            const void *mod, unsigned long *_len)
+static int module_sig_check(struct load_info *info)
 {
         int err = -ENOKEY;
-        unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
-        unsigned long len = *_len;
+        const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
+        const void *mod = info->hdr;
 
-        if (len > markerlen &&
-            memcmp(mod + len - markerlen, MODULE_SIG_STRING, markerlen) == 0) {
+        if (info->len > markerlen &&
+            memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) == 0) {
                 /* We truncate the module to discard the signature */
-                *_len -= markerlen;
-                err = mod_verify_sig(mod, _len);
+                info->len -= markerlen;
+                err = mod_verify_sig(mod, &info->len);
         }
 
         if (!err) {
@@ -2454,59 +2454,97 @@ static int module_sig_check(struct load_info *info,
         return err;
 }
 #else /* !CONFIG_MODULE_SIG */
-static int module_sig_check(struct load_info *info,
-                            void *mod, unsigned long *len)
+static int module_sig_check(struct load_info *info)
 {
         return 0;
 }
 #endif /* !CONFIG_MODULE_SIG */
 
-/* Sets info->hdr, info->len and info->sig_ok. */
-static int copy_and_check(struct load_info *info,
-                          const void __user *umod, unsigned long len,
-                          const char __user *uargs)
+/* Sanity checks against invalid binaries, wrong arch, weird elf version. */
+static int elf_header_check(struct load_info *info)
 {
-        int err;
-        Elf_Ehdr *hdr;
+        if (info->len < sizeof(*(info->hdr)))
+                return -ENOEXEC;
+
+        if (memcmp(info->hdr->e_ident, ELFMAG, SELFMAG) != 0
+            || info->hdr->e_type != ET_REL
+            || !elf_check_arch(info->hdr)
+            || info->hdr->e_shentsize != sizeof(Elf_Shdr))
+                return -ENOEXEC;
+
+        if (info->hdr->e_shoff >= info->len
+            || (info->hdr->e_shnum * sizeof(Elf_Shdr) >
+                info->len - info->hdr->e_shoff))
+                return -ENOEXEC;
 
-        if (len < sizeof(*hdr))
+        return 0;
+}
+
+/* Sets info->hdr and info->len. */
+static int copy_module_from_user(const void __user *umod, unsigned long len,
+                                  struct load_info *info)
+{
+        info->len = len;
+        if (info->len < sizeof(*(info->hdr)))
                 return -ENOEXEC;
 
         /* Suck in entire file: we'll want most of it. */
-        if ((hdr = vmalloc(len)) == NULL)
+        info->hdr = vmalloc(info->len);
+        if (!info->hdr)
                 return -ENOMEM;
 
-        if (copy_from_user(hdr, umod, len) != 0) {
-                err = -EFAULT;
-                goto free_hdr;
+        if (copy_from_user(info->hdr, umod, info->len) != 0) {
+                vfree(info->hdr);
+                return -EFAULT;
         }
 
-        err = module_sig_check(info, hdr, &len);
+        return 0;
+}
+
+/* Sets info->hdr and info->len. */
+static int copy_module_from_fd(int fd, struct load_info *info)
+{
+        struct file *file;
+        int err;
+        struct kstat stat;
+        loff_t pos;
+        ssize_t bytes = 0;
+
+        file = fget(fd);
+        if (!file)
+                return -ENOEXEC;
+
+        err = vfs_getattr(file->f_vfsmnt, file->f_dentry, &stat);
         if (err)
-                goto free_hdr;
+                goto out;
 
-        /* Sanity checks against insmoding binaries or wrong arch,
-           weird elf version */
-        if (memcmp(hdr->e_ident, ELFMAG, SELFMAG) != 0
-            || hdr->e_type != ET_REL
-            || !elf_check_arch(hdr)
-            || hdr->e_shentsize != sizeof(Elf_Shdr)) {
-                err = -ENOEXEC;
-                goto free_hdr;
+        if (stat.size > INT_MAX) {
+                err = -EFBIG;
+                goto out;
         }
-
-        if (hdr->e_shoff >= len ||
-            hdr->e_shnum * sizeof(Elf_Shdr) > len - hdr->e_shoff) {
-                err = -ENOEXEC;
-                goto free_hdr;
+        info->hdr = vmalloc(stat.size);
+        if (!info->hdr) {
+                err = -ENOMEM;
+                goto out;
         }
 
-        info->hdr = hdr;
-        info->len = len;
-        return 0;
+        pos = 0;
+        while (pos < stat.size) {
+                bytes = kernel_read(file, pos, (char *)(info->hdr) + pos,
+                                    stat.size - pos);
+                if (bytes < 0) {