Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754515Ab2H2VaH (ORCPT ); Wed, 29 Aug 2012 17:30:07 -0400 Received: from smtp.outflux.net ([198.145.64.163]:34513 "EHLO smtp.outflux.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753598Ab2H2VaF (ORCPT ); Wed, 29 Aug 2012 17:30:05 -0400 From: Kees Cook To: linux-kernel@vger.kernel.org Cc: Rusty Russell , Serge Hallyn , James Morris , Al Viro , Eric Paris , Kees Cook , Jiri Kosina , linux-security-module@vger.kernel.org Subject: [PATCH 1/2] module: allow loading module from fd Date: Wed, 29 Aug 2012 14:29:06 -0700 Message-Id: <1346275747-8936-1-git-send-email-keescook@chromium.org> X-Mailer: git-send-email 1.7.0.4 X-HELO: www.outflux.net Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4199 Lines: 164 Instead of (or in addition to) kernel module signing, being able to reason about the origin of a kernel module would be valuable in situations where an OS already trusts a specific file system, file, etc, due to things like security labels or an existing root of trust to a partition through things like dm-verity. This changes the init_module syscall so that when the first argument (blob address) is NULL, the second argument is used as a file descriptor to the module (instead of length). The third argument (module arguments) remains unchanged. Some alternatives to overloading the existing syscall are: - write a new syscall (seemed unnecessary) - add an fd ioctl (awful) - enhance the ELF binfmt loader (complex) It seemed most sensible to avoid introducing new or crazy interfaces or further complicating the ELF loader. Instead, just use the existing syscall in a new way. Tools using the fd argument style can trivially downgrade to the blob argument style when they see an EFAULT error. Signed-off-by: Kees Cook --- kernel/module.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++------ 1 files changed, 87 insertions(+), 10 deletions(-) diff --git a/kernel/module.c b/kernel/module.c index 4edbd9c..0be8c11 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -2399,23 +2400,99 @@ static inline void kmemleak_load_module(const struct module *mod, } #endif -/* Sets info->hdr and info->len. */ -static int copy_and_check(struct load_info *info, - const void __user *umod, unsigned long len, - const char __user *uargs) +static Elf_Ehdr *copy_module_from_user(const void __user *umod, + unsigned long len) { - int err; Elf_Ehdr *hdr; if (len < sizeof(*hdr)) - return -ENOEXEC; + return ERR_PTR(-ENOEXEC); /* Suck in entire file: we'll want most of it. */ - if ((hdr = vmalloc(len)) == NULL) - return -ENOMEM; + hdr = vmalloc(len); + if (!hdr) + return ERR_PTR(-ENOMEM); if (copy_from_user(hdr, umod, len) != 0) { - err = -EFAULT; + vfree(hdr); + return ERR_PTR(-EFAULT); + } + + return hdr; +} + +static Elf_Ehdr *copy_module_from_fd(unsigned int fd, unsigned long *len) +{ + struct file *file; + int err; + Elf_Ehdr *hdr; + struct kstat stat; + unsigned long size; + off_t pos; + ssize_t bytes = 0; + + file = fget(fd); + if (!file) + return ERR_PTR(-ENOEXEC); + + err = vfs_getattr(file->f_vfsmnt, file->f_dentry, &stat); + if (err) { + hdr = ERR_PTR(err); + goto out; + } + + if (stat.size > INT_MAX) { + hdr = ERR_PTR(-ENOMEM); + goto out; + } + size = stat.size; + + hdr = vmalloc(size); + if (!hdr) { + hdr = ERR_PTR(-ENOMEM); + goto out; + } + + pos = 0; + while (pos < size) { + bytes = kernel_read(file, pos, (char *)hdr + pos, size - pos); + if (bytes < 0) { + vfree(hdr); + hdr = ERR_PTR(bytes); + goto out; + } + if (bytes == 0) + break; + pos += bytes; + } + *len = pos; + +out: + fput(file); + return hdr; +} + +/* Sets info->hdr and info->len. */ +static int copy_and_check(struct load_info *info, + const void __user *umod, unsigned long len) +{ + int err; + Elf_Ehdr *hdr; + + if (umod == NULL) { + unsigned int fd; + + if (len < 0 || len > INT_MAX) + return -ENOEXEC; + fd = len; + + hdr = copy_module_from_fd(fd, &len); + } else + hdr = copy_module_from_user(umod, len); + if (IS_ERR(hdr)) + return PTR_ERR(hdr); + if (len < sizeof(*hdr)) { + err = -ENOEXEC; goto free_hdr; } @@ -2875,7 +2952,7 @@ static struct module *load_module(void __user *umod, umod, len, uargs); /* Copy in the blobs from userspace, check they are vaguely sane. */ - err = copy_and_check(&info, umod, len, uargs); + err = copy_and_check(&info, umod, len); if (err) return ERR_PTR(err); -- 1.7.0.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/