Commit 193934123c84fa168d0326aa6ab8d58cd173b32a
Exists in
ti-lsk-linux-4.1.y
and in
10 other branches
Merge tag 'fixes-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux
Pull module and param fixes from Rusty Russell: "Surprising number of fixes this merge window :( The first two are minor fallout from the param rework which went in this merge window. The next three are a series which fixes a longstanding (but never previously reported and unlikely , so no CC stable) race between kallsyms and freeing the init section. Finally, a minor cleanup as our module refcount will now be -1 during unload" * tag 'fixes-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux: module: make module_refcount() a signed integer. module: fix race in kallsyms resolution during module load success. module: remove mod arg from module_free, rename module_memfree(). module_arch_freeing_init(): new hook for archs before module->module_init freed. param: fix uninitialized read with CONFIG_DEBUG_LOCK_ALLOC param: initialize store function to NULL if not available.
Showing 18 changed files Side-by-side Diff
- arch/avr32/kernel/module.c
- arch/cris/kernel/module.c
- arch/ia64/kernel/module.c
- arch/mips/net/bpf_jit.c
- arch/nios2/kernel/module.c
- arch/parisc/kernel/module.c
- arch/powerpc/net/bpf_jit_comp.c
- arch/s390/kernel/module.c
- arch/sparc/net/bpf_jit_comp.c
- arch/tile/kernel/module.c
- arch/x86/kernel/ftrace.c
- include/linux/module.h
- include/linux/moduleloader.h
- kernel/bpf/core.c
- kernel/debug/kdb/kdb_main.c
- kernel/kprobes.c
- kernel/module.c
- kernel/params.c
arch/avr32/kernel/module.c
... | ... | @@ -19,12 +19,10 @@ |
19 | 19 | #include <linux/moduleloader.h> |
20 | 20 | #include <linux/vmalloc.h> |
21 | 21 | |
22 | -void module_free(struct module *mod, void *module_region) | |
22 | +void module_arch_freeing_init(struct module *mod) | |
23 | 23 | { |
24 | 24 | vfree(mod->arch.syminfo); |
25 | 25 | mod->arch.syminfo = NULL; |
26 | - | |
27 | - vfree(module_region); | |
28 | 26 | } |
29 | 27 | |
30 | 28 | static inline int check_rela(Elf32_Rela *rela, struct module *module, |
... | ... | @@ -290,14 +288,5 @@ |
290 | 288 | } |
291 | 289 | |
292 | 290 | return ret; |
293 | -} | |
294 | - | |
295 | -int module_finalize(const Elf_Ehdr *hdr, const Elf_Shdr *sechdrs, | |
296 | - struct module *module) | |
297 | -{ | |
298 | - vfree(module->arch.syminfo); | |
299 | - module->arch.syminfo = NULL; | |
300 | - | |
301 | - return 0; | |
302 | 291 | } |
arch/cris/kernel/module.c
arch/ia64/kernel/module.c
... | ... | @@ -305,14 +305,12 @@ |
305 | 305 | #endif /* !USE_BRL */ |
306 | 306 | |
307 | 307 | void |
308 | -module_free (struct module *mod, void *module_region) | |
308 | +module_arch_freeing_init (struct module *mod) | |
309 | 309 | { |
310 | - if (mod && mod->arch.init_unw_table && | |
311 | - module_region == mod->module_init) { | |
310 | + if (mod->arch.init_unw_table) { | |
312 | 311 | unw_remove_unwind_table(mod->arch.init_unw_table); |
313 | 312 | mod->arch.init_unw_table = NULL; |
314 | 313 | } |
315 | - vfree(module_region); | |
316 | 314 | } |
317 | 315 | |
318 | 316 | /* Have we already seen one of these relocations? */ |
arch/mips/net/bpf_jit.c
arch/nios2/kernel/module.c
arch/parisc/kernel/module.c
... | ... | @@ -298,14 +298,10 @@ |
298 | 298 | } |
299 | 299 | #endif |
300 | 300 | |
301 | - | |
302 | -/* Free memory returned from module_alloc */ | |
303 | -void module_free(struct module *mod, void *module_region) | |
301 | +void module_arch_freeing_init(struct module *mod) | |
304 | 302 | { |
305 | 303 | kfree(mod->arch.section); |
306 | 304 | mod->arch.section = NULL; |
307 | - | |
308 | - vfree(module_region); | |
309 | 305 | } |
310 | 306 | |
311 | 307 | /* Additional bytes needed in front of individual sections */ |
arch/powerpc/net/bpf_jit_comp.c
arch/s390/kernel/module.c
... | ... | @@ -55,14 +55,10 @@ |
55 | 55 | } |
56 | 56 | #endif |
57 | 57 | |
58 | -/* Free memory returned from module_alloc */ | |
59 | -void module_free(struct module *mod, void *module_region) | |
58 | +void module_arch_freeing_init(struct module *mod) | |
60 | 59 | { |
61 | - if (mod) { | |
62 | - vfree(mod->arch.syminfo); | |
63 | - mod->arch.syminfo = NULL; | |
64 | - } | |
65 | - vfree(module_region); | |
60 | + vfree(mod->arch.syminfo); | |
61 | + mod->arch.syminfo = NULL; | |
66 | 62 | } |
67 | 63 | |
68 | 64 | static void check_rela(Elf_Rela *rela, struct module *me) |
arch/sparc/net/bpf_jit_comp.c
... | ... | @@ -776,7 +776,7 @@ |
776 | 776 | if (unlikely(proglen + ilen > oldproglen)) { |
777 | 777 | pr_err("bpb_jit_compile fatal error\n"); |
778 | 778 | kfree(addrs); |
779 | - module_free(NULL, image); | |
779 | + module_memfree(image); | |
780 | 780 | return; |
781 | 781 | } |
782 | 782 | memcpy(image + proglen, temp, ilen); |
... | ... | @@ -822,7 +822,7 @@ |
822 | 822 | void bpf_jit_free(struct bpf_prog *fp) |
823 | 823 | { |
824 | 824 | if (fp->jited) |
825 | - module_free(NULL, fp->bpf_func); | |
825 | + module_memfree(fp->bpf_func); | |
826 | 826 | |
827 | 827 | bpf_prog_unlock_free(fp); |
828 | 828 | } |
arch/tile/kernel/module.c
... | ... | @@ -74,7 +74,7 @@ |
74 | 74 | |
75 | 75 | |
76 | 76 | /* Free memory returned from module_alloc */ |
77 | -void module_free(struct module *mod, void *module_region) | |
77 | +void module_memfree(void *module_region) | |
78 | 78 | { |
79 | 79 | vfree(module_region); |
80 | 80 | |
... | ... | @@ -83,7 +83,7 @@ |
83 | 83 | 0, 0, 0, NULL, NULL, 0); |
84 | 84 | |
85 | 85 | /* |
86 | - * FIXME: If module_region == mod->module_init, trim exception | |
86 | + * FIXME: Add module_arch_freeing_init to trim exception | |
87 | 87 | * table entries. |
88 | 88 | */ |
89 | 89 | } |
arch/x86/kernel/ftrace.c
include/linux/module.h
... | ... | @@ -444,7 +444,7 @@ |
444 | 444 | #define module_put_and_exit(code) __module_put_and_exit(THIS_MODULE, code) |
445 | 445 | |
446 | 446 | #ifdef CONFIG_MODULE_UNLOAD |
447 | -unsigned long module_refcount(struct module *mod); | |
447 | +int module_refcount(struct module *mod); | |
448 | 448 | void __symbol_put(const char *symbol); |
449 | 449 | #define symbol_put(x) __symbol_put(VMLINUX_SYMBOL_STR(x)) |
450 | 450 | void symbol_put_addr(void *addr); |
include/linux/moduleloader.h
... | ... | @@ -26,7 +26,7 @@ |
26 | 26 | void *module_alloc(unsigned long size); |
27 | 27 | |
28 | 28 | /* Free memory returned from module_alloc. */ |
29 | -void module_free(struct module *mod, void *module_region); | |
29 | +void module_memfree(void *module_region); | |
30 | 30 | |
31 | 31 | /* |
32 | 32 | * Apply the given relocation to the (simplified) ELF. Return -error |
... | ... | @@ -82,5 +82,7 @@ |
82 | 82 | /* Any cleanup needed when module leaves. */ |
83 | 83 | void module_arch_cleanup(struct module *mod); |
84 | 84 | |
85 | +/* Any cleanup before freeing mod->module_init */ | |
86 | +void module_arch_freeing_init(struct module *mod); | |
85 | 87 | #endif |
kernel/bpf/core.c
kernel/debug/kdb/kdb_main.c
... | ... | @@ -2023,7 +2023,7 @@ |
2023 | 2023 | kdb_printf("%-20s%8u 0x%p ", mod->name, |
2024 | 2024 | mod->core_size, (void *)mod); |
2025 | 2025 | #ifdef CONFIG_MODULE_UNLOAD |
2026 | - kdb_printf("%4ld ", module_refcount(mod)); | |
2026 | + kdb_printf("%4d ", module_refcount(mod)); | |
2027 | 2027 | #endif |
2028 | 2028 | if (mod->state == MODULE_STATE_GOING) |
2029 | 2029 | kdb_printf(" (Unloading)"); |
kernel/kprobes.c
kernel/module.c
... | ... | @@ -772,9 +772,18 @@ |
772 | 772 | return 0; |
773 | 773 | } |
774 | 774 | |
775 | -unsigned long module_refcount(struct module *mod) | |
775 | +/** | |
776 | + * module_refcount - return the refcount or -1 if unloading | |
777 | + * | |
778 | + * @mod: the module we're checking | |
779 | + * | |
780 | + * Returns: | |
781 | + * -1 if the module is in the process of unloading | |
782 | + * otherwise the number of references in the kernel to the module | |
783 | + */ | |
784 | +int module_refcount(struct module *mod) | |
776 | 785 | { |
777 | - return (unsigned long)atomic_read(&mod->refcnt) - MODULE_REF_BASE; | |
786 | + return atomic_read(&mod->refcnt) - MODULE_REF_BASE; | |
778 | 787 | } |
779 | 788 | EXPORT_SYMBOL(module_refcount); |
780 | 789 | |
... | ... | @@ -856,7 +865,7 @@ |
856 | 865 | struct module_use *use; |
857 | 866 | int printed_something = 0; |
858 | 867 | |
859 | - seq_printf(m, " %lu ", module_refcount(mod)); | |
868 | + seq_printf(m, " %i ", module_refcount(mod)); | |
860 | 869 | |
861 | 870 | /* |
862 | 871 | * Always include a trailing , so userspace can differentiate |
... | ... | @@ -908,7 +917,7 @@ |
908 | 917 | static ssize_t show_refcnt(struct module_attribute *mattr, |
909 | 918 | struct module_kobject *mk, char *buffer) |
910 | 919 | { |
911 | - return sprintf(buffer, "%lu\n", module_refcount(mk->mod)); | |
920 | + return sprintf(buffer, "%i\n", module_refcount(mk->mod)); | |
912 | 921 | } |
913 | 922 | |
914 | 923 | static struct module_attribute modinfo_refcnt = |
... | ... | @@ -1795,7 +1804,7 @@ |
1795 | 1804 | static void unset_module_init_ro_nx(struct module *mod) { } |
1796 | 1805 | #endif |
1797 | 1806 | |
1798 | -void __weak module_free(struct module *mod, void *module_region) | |
1807 | +void __weak module_memfree(void *module_region) | |
1799 | 1808 | { |
1800 | 1809 | vfree(module_region); |
1801 | 1810 | } |
... | ... | @@ -1804,6 +1813,10 @@ |
1804 | 1813 | { |
1805 | 1814 | } |
1806 | 1815 | |
1816 | +void __weak module_arch_freeing_init(struct module *mod) | |
1817 | +{ | |
1818 | +} | |
1819 | + | |
1807 | 1820 | /* Free a module, remove from lists, etc. */ |
1808 | 1821 | static void free_module(struct module *mod) |
1809 | 1822 | { |
... | ... | @@ -1841,7 +1854,8 @@ |
1841 | 1854 | |
1842 | 1855 | /* This may be NULL, but that's OK */ |
1843 | 1856 | unset_module_init_ro_nx(mod); |
1844 | - module_free(mod, mod->module_init); | |
1857 | + module_arch_freeing_init(mod); | |
1858 | + module_memfree(mod->module_init); | |
1845 | 1859 | kfree(mod->args); |
1846 | 1860 | percpu_modfree(mod); |
1847 | 1861 | |
... | ... | @@ -1850,7 +1864,7 @@ |
1850 | 1864 | |
1851 | 1865 | /* Finally, free the core (containing the module structure) */ |
1852 | 1866 | unset_module_core_ro_nx(mod); |
1853 | - module_free(mod, mod->module_core); | |
1867 | + module_memfree(mod->module_core); | |
1854 | 1868 | |
1855 | 1869 | #ifdef CONFIG_MPU |
1856 | 1870 | update_protections(current->mm); |
... | ... | @@ -2785,7 +2799,7 @@ |
2785 | 2799 | */ |
2786 | 2800 | kmemleak_ignore(ptr); |
2787 | 2801 | if (!ptr) { |
2788 | - module_free(mod, mod->module_core); | |
2802 | + module_memfree(mod->module_core); | |
2789 | 2803 | return -ENOMEM; |
2790 | 2804 | } |
2791 | 2805 | memset(ptr, 0, mod->init_size); |
... | ... | @@ -2930,8 +2944,9 @@ |
2930 | 2944 | static void module_deallocate(struct module *mod, struct load_info *info) |
2931 | 2945 | { |
2932 | 2946 | percpu_modfree(mod); |
2933 | - module_free(mod, mod->module_init); | |
2934 | - module_free(mod, mod->module_core); | |
2947 | + module_arch_freeing_init(mod); | |
2948 | + module_memfree(mod->module_init); | |
2949 | + module_memfree(mod->module_core); | |
2935 | 2950 | } |
2936 | 2951 | |
2937 | 2952 | int __weak module_finalize(const Elf_Ehdr *hdr, |
2938 | 2953 | |
2939 | 2954 | |
... | ... | @@ -2983,11 +2998,32 @@ |
2983 | 2998 | #endif |
2984 | 2999 | } |
2985 | 3000 | |
3001 | +/* For freeing module_init on success, in case kallsyms traversing */ | |
3002 | +struct mod_initfree { | |
3003 | + struct rcu_head rcu; | |
3004 | + void *module_init; | |
3005 | +}; | |
3006 | + | |
3007 | +static void do_free_init(struct rcu_head *head) | |
3008 | +{ | |
3009 | + struct mod_initfree *m = container_of(head, struct mod_initfree, rcu); | |
3010 | + module_memfree(m->module_init); | |
3011 | + kfree(m); | |
3012 | +} | |
3013 | + | |
2986 | 3014 | /* This is where the real work happens */ |
2987 | 3015 | static int do_init_module(struct module *mod) |
2988 | 3016 | { |
2989 | 3017 | int ret = 0; |
3018 | + struct mod_initfree *freeinit; | |
2990 | 3019 | |
3020 | + freeinit = kmalloc(sizeof(*freeinit), GFP_KERNEL); | |
3021 | + if (!freeinit) { | |
3022 | + ret = -ENOMEM; | |
3023 | + goto fail; | |
3024 | + } | |
3025 | + freeinit->module_init = mod->module_init; | |
3026 | + | |
2991 | 3027 | /* |
2992 | 3028 | * We want to find out whether @mod uses async during init. Clear |
2993 | 3029 | * PF_USED_ASYNC. async_schedule*() will set it. |
... | ... | @@ -2999,18 +3035,7 @@ |
2999 | 3035 | if (mod->init != NULL) |
3000 | 3036 | ret = do_one_initcall(mod->init); |
3001 | 3037 | if (ret < 0) { |
3002 | - /* | |
3003 | - * Init routine failed: abort. Try to protect us from | |
3004 | - * buggy refcounters. | |
3005 | - */ | |
3006 | - mod->state = MODULE_STATE_GOING; | |
3007 | - synchronize_sched(); | |
3008 | - module_put(mod); | |
3009 | - blocking_notifier_call_chain(&module_notify_list, | |
3010 | - MODULE_STATE_GOING, mod); | |
3011 | - free_module(mod); | |
3012 | - wake_up_all(&module_wq); | |
3013 | - return ret; | |
3038 | + goto fail_free_freeinit; | |
3014 | 3039 | } |
3015 | 3040 | if (ret > 0) { |
3016 | 3041 | pr_warn("%s: '%s'->init suspiciously returned %d, it should " |
3017 | 3042 | |
3018 | 3043 | |
... | ... | @@ -3055,15 +3080,35 @@ |
3055 | 3080 | mod->strtab = mod->core_strtab; |
3056 | 3081 | #endif |
3057 | 3082 | unset_module_init_ro_nx(mod); |
3058 | - module_free(mod, mod->module_init); | |
3083 | + module_arch_freeing_init(mod); | |
3059 | 3084 | mod->module_init = NULL; |
3060 | 3085 | mod->init_size = 0; |
3061 | 3086 | mod->init_ro_size = 0; |
3062 | 3087 | mod->init_text_size = 0; |
3088 | + /* | |
3089 | + * We want to free module_init, but be aware that kallsyms may be | |
3090 | + * walking this with preempt disabled. In all the failure paths, | |
3091 | + * we call synchronize_rcu/synchronize_sched, but we don't want | |
3092 | + * to slow down the success path, so use actual RCU here. | |
3093 | + */ | |
3094 | + call_rcu(&freeinit->rcu, do_free_init); | |
3063 | 3095 | mutex_unlock(&module_mutex); |
3064 | 3096 | wake_up_all(&module_wq); |
3065 | 3097 | |
3066 | 3098 | return 0; |
3099 | + | |
3100 | +fail_free_freeinit: | |
3101 | + kfree(freeinit); | |
3102 | +fail: | |
3103 | + /* Try to protect us from buggy refcounters. */ | |
3104 | + mod->state = MODULE_STATE_GOING; | |
3105 | + synchronize_sched(); | |
3106 | + module_put(mod); | |
3107 | + blocking_notifier_call_chain(&module_notify_list, | |
3108 | + MODULE_STATE_GOING, mod); | |
3109 | + free_module(mod); | |
3110 | + wake_up_all(&module_wq); | |
3111 | + return ret; | |
3067 | 3112 | } |
3068 | 3113 | |
3069 | 3114 | static int may_init_module(void) |
kernel/params.c
... | ... | @@ -642,12 +642,15 @@ |
642 | 642 | mk->mp->grp.attrs = new_attrs; |
643 | 643 | |
644 | 644 | /* Tack new one on the end. */ |
645 | + memset(&mk->mp->attrs[mk->mp->num], 0, sizeof(mk->mp->attrs[0])); | |
645 | 646 | sysfs_attr_init(&mk->mp->attrs[mk->mp->num].mattr.attr); |
646 | 647 | mk->mp->attrs[mk->mp->num].param = kp; |
647 | 648 | mk->mp->attrs[mk->mp->num].mattr.show = param_attr_show; |
648 | 649 | /* Do not allow runtime DAC changes to make param writable. */ |
649 | 650 | if ((kp->perm & (S_IWUSR | S_IWGRP | S_IWOTH)) != 0) |
650 | 651 | mk->mp->attrs[mk->mp->num].mattr.store = param_attr_store; |
652 | + else | |
653 | + mk->mp->attrs[mk->mp->num].mattr.store = NULL; | |
651 | 654 | mk->mp->attrs[mk->mp->num].mattr.attr.name = (char *)name; |
652 | 655 | mk->mp->attrs[mk->mp->num].mattr.attr.mode = kp->perm; |
653 | 656 | mk->mp->num++; |