From 34e1169d996ab148490c01b65b4ee371cf8ffba2 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Tue, 16 Oct 2012 07:31:07 +1030 Subject: 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 Cc: Andrew Morton Signed-off-by: Rusty Russell (merge fixes) --- arch/x86/syscalls/syscall_32.tbl | 1 + arch/x86/syscalls/syscall_64.tbl | 1 + 2 files changed, 2 insertions(+) (limited to 'arch') 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 -- cgit v1.2.3 From 4926f65224e49f253feb0320b03814ea630b7541 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Tue, 16 Oct 2012 12:38:47 +1030 Subject: ARM: add finit_module syscall to ARM Add finit_module syscall to the ARM syscall list. Signed-off-by: Kees Cook Cc: Russell King Signed-off-by: Rusty Russell --- arch/arm/include/uapi/asm/unistd.h | 1 + arch/arm/kernel/calls.S | 1 + 2 files changed, 2 insertions(+) (limited to 'arch') diff --git a/arch/arm/include/uapi/asm/unistd.h b/arch/arm/include/uapi/asm/unistd.h index ac03bdb4ae44..4da7cde70b5d 100644 --- a/arch/arm/include/uapi/asm/unistd.h +++ b/arch/arm/include/uapi/asm/unistd.h @@ -405,6 +405,7 @@ #define __NR_process_vm_readv (__NR_SYSCALL_BASE+376) #define __NR_process_vm_writev (__NR_SYSCALL_BASE+377) /* 378 for kcmp */ +#define __NR_finit_module (__NR_SYSCALL_BASE+379) /* * This may need to be greater than __NR_last_syscall+1 in order to diff --git a/arch/arm/kernel/calls.S b/arch/arm/kernel/calls.S index 831cd38c8d99..36e27df01b47 100644 --- a/arch/arm/kernel/calls.S +++ b/arch/arm/kernel/calls.S @@ -388,6 +388,7 @@ CALL(sys_process_vm_readv) CALL(sys_process_vm_writev) CALL(sys_ni_syscall) /* reserved for sys_kcmp */ + CALL(sys_finit_module) #ifndef syscalls_counted .equ syscalls_padding, ((NR_syscalls + 3) & ~3) - NR_syscalls #define syscalls_counted -- cgit v1.2.3 From 71eac70257b469bd43737232bce0fd960caebee1 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 2 Nov 2012 14:57:20 +1030 Subject: powerpc: add finit_module syscall. (This is just for Acks: this won't work without the actual syscall patches, sitting in my tree for -next at the moment). Acked-by: Benjamin Herrenschmidt Signed-off-by: Rusty Russell --- arch/powerpc/include/asm/systbl.h | 1 + arch/powerpc/include/asm/unistd.h | 2 +- arch/powerpc/include/uapi/asm/unistd.h | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) (limited to 'arch') diff --git a/arch/powerpc/include/asm/systbl.h b/arch/powerpc/include/asm/systbl.h index 840838769853..d0b27f8b47fb 100644 --- a/arch/powerpc/include/asm/systbl.h +++ b/arch/powerpc/include/asm/systbl.h @@ -356,3 +356,4 @@ COMPAT_SYS_SPU(sendmmsg) SYSCALL_SPU(setns) COMPAT_SYS(process_vm_readv) COMPAT_SYS(process_vm_writev) +SYSCALL(finit_module) diff --git a/arch/powerpc/include/asm/unistd.h b/arch/powerpc/include/asm/unistd.h index 921dce6d8445..8d219a014c3b 100644 --- a/arch/powerpc/include/asm/unistd.h +++ b/arch/powerpc/include/asm/unistd.h @@ -12,7 +12,7 @@ #include -#define __NR_syscalls 353 +#define __NR_syscalls 354 #define __NR__exit __NR_exit #define NR_syscalls __NR_syscalls diff --git a/arch/powerpc/include/uapi/asm/unistd.h b/arch/powerpc/include/uapi/asm/unistd.h index 380b5d37a904..8c478c6c6b1e 100644 --- a/arch/powerpc/include/uapi/asm/unistd.h +++ b/arch/powerpc/include/uapi/asm/unistd.h @@ -375,6 +375,7 @@ #define __NR_setns 350 #define __NR_process_vm_readv 351 #define __NR_process_vm_writev 352 +#define __NR_finit_module 353 #endif /* _UAPI_ASM_POWERPC_UNISTD_H_ */ -- cgit v1.2.3 From 82fab442f5322b016f72891c0db2436c6a6c20b7 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 11 Dec 2012 09:38:33 +1030 Subject: modules: don't hand 0 to vmalloc. In commit d0a21265dfb5fa8a David Rientjes unified various archs' module_alloc implementation (including x86) and removed the graduitous shortcut for size == 0. Then, in commit de7d2b567d040e3b, Joe Perches added a warning for zero-length vmallocs, which can happen without kallsyms on modules with no init sections (eg. zlib_deflate). Fix this once and for all; the module code has to handle zero length anyway, so get it right at the caller and remove the now-gratuitous checks within the arch-specific module_alloc implementations. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=42608 Reported-by: Conrad Kostecki Cc: David Rientjes Cc: Joe Perches Signed-off-by: Rusty Russell --- arch/cris/kernel/module.c | 2 -- arch/parisc/kernel/module.c | 2 -- arch/sparc/kernel/module.c | 4 ---- arch/tile/kernel/module.c | 2 -- arch/unicore32/kernel/module.c | 3 --- kernel/module.c | 33 ++++++++++++++++++--------------- 6 files changed, 18 insertions(+), 28 deletions(-) (limited to 'arch') diff --git a/arch/cris/kernel/module.c b/arch/cris/kernel/module.c index 37400f5869e6..51123f985eb5 100644 --- a/arch/cris/kernel/module.c +++ b/arch/cris/kernel/module.c @@ -32,8 +32,6 @@ #ifdef CONFIG_ETRAX_KMALLOCED_MODULES void *module_alloc(unsigned long size) { - if (size == 0) - return NULL; return kmalloc(size, GFP_KERNEL); } diff --git a/arch/parisc/kernel/module.c b/arch/parisc/kernel/module.c index 5e34ccf39a49..2a625fb063e1 100644 --- a/arch/parisc/kernel/module.c +++ b/arch/parisc/kernel/module.c @@ -214,8 +214,6 @@ static inline int reassemble_22(int as22) void *module_alloc(unsigned long size) { - if (size == 0) - return NULL; /* using RWX means less protection for modules, but it's * easier than trying to map the text, data, init_text and * init_data correctly */ diff --git a/arch/sparc/kernel/module.c b/arch/sparc/kernel/module.c index f1ddc0d23679..4435488ebe25 100644 --- a/arch/sparc/kernel/module.c +++ b/arch/sparc/kernel/module.c @@ -43,10 +43,6 @@ void *module_alloc(unsigned long size) { void *ret; - /* We handle the zero case fine, unlike vmalloc */ - if (size == 0) - return NULL; - ret = module_map(size); if (ret) memset(ret, 0, size); diff --git a/arch/tile/kernel/module.c b/arch/tile/kernel/module.c index 243ffebe38d6..4918d91bc3a6 100644 --- a/arch/tile/kernel/module.c +++ b/arch/tile/kernel/module.c @@ -42,8 +42,6 @@ void *module_alloc(unsigned long size) int i = 0; int npages; - if (size == 0) - return NULL; npages = (size + PAGE_SIZE - 1) / PAGE_SIZE; pages = kmalloc(npages * sizeof(struct page *), GFP_KERNEL); if (pages == NULL) diff --git a/arch/unicore32/kernel/module.c b/arch/unicore32/kernel/module.c index 8fbe8577f5e6..16bd1495b934 100644 --- a/arch/unicore32/kernel/module.c +++ b/arch/unicore32/kernel/module.c @@ -27,9 +27,6 @@ void *module_alloc(unsigned long size) struct vm_struct *area; size = PAGE_ALIGN(size); - if (!size) - return NULL; - area = __get_vm_area(size, VM_ALLOC, MODULES_VADDR, MODULES_END); if (!area) return NULL; diff --git a/kernel/module.c b/kernel/module.c index 79a526dd1b11..4542ff3f7ef9 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -2377,7 +2377,7 @@ static void dynamic_debug_remove(struct _ddebug *debug) void * __weak module_alloc(unsigned long size) { - return size == 0 ? NULL : vmalloc_exec(size); + return vmalloc_exec(size); } static void *module_alloc_update_bounds(unsigned long size) @@ -2793,20 +2793,23 @@ static int move_module(struct module *mod, struct load_info *info) memset(ptr, 0, mod->core_size); mod->module_core = ptr; - ptr = module_alloc_update_bounds(mod->init_size); - /* - * The pointer to this block is stored in the module structure - * which is inside the block. This block doesn't need to be - * scanned as it contains data and code that will be freed - * after the module is initialized. - */ - kmemleak_ignore(ptr); - if (!ptr && mod->init_size) { - module_free(mod, mod->module_core); - return -ENOMEM; - } - memset(ptr, 0, mod->init_size); - mod->module_init = ptr; + if (mod->init_size) { + ptr = module_alloc_update_bounds(mod->init_size); + /* + * The pointer to this block is stored in the module structure + * which is inside the block. This block doesn't need to be + * scanned as it contains data and code that will be freed + * after the module is initialized. + */ + kmemleak_ignore(ptr); + if (!ptr) { + module_free(mod, mod->module_core); + return -ENOMEM; + } + memset(ptr, 0, mod->init_size); + mod->module_init = ptr; + } else + mod->module_init = NULL; /* Transfer each section which specifies SHF_ALLOC */ pr_debug("final section addresses:\n"); -- cgit v1.2.3