From b4bc842802db3314f9a657094da0450a903ea619 Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Mon, 7 Feb 2011 16:02:25 -0800 Subject: module: deal with alignment issues in built-in module versions On m68k natural alignment is 2-byte boundary but we are trying to align structures in __modver section on sizeof(void *) boundary. This causes trouble when we try to access elements in this section in array-like fashion when create "version" attributes for built-in modules. Moreover, as DaveM said, we can't reliably put structures into independent objects, put them into a special section, and then expect array access over them (via the section boundaries) after linking the objects together to just "work" due to variable alignment choices in different situations. The only solution that seems to work reliably is to make an array of plain pointers to the objects in question and put those pointers in the special section. Reported-by: Geert Uytterhoeven Signed-off-by: Dmitry Torokhov Signed-off-by: Rusty Russell --- include/linux/module.h | 10 +++++----- kernel/params.c | 9 ++++++--- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/include/linux/module.h b/include/linux/module.h index 5de42043dff..4cfebc21169 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -174,10 +174,7 @@ extern struct module __this_module; #define MODULE_VERSION(_version) \ extern ssize_t __modver_version_show(struct module_attribute *, \ struct module *, char *); \ - static struct module_version_attribute __modver_version_attr \ - __used \ - __attribute__ ((__section__ ("__modver"),aligned(sizeof(void *)))) \ - = { \ + static struct module_version_attribute ___modver_attr = { \ .mattr = { \ .attr = { \ .name = "version", \ @@ -187,7 +184,10 @@ extern struct module __this_module; }, \ .module_name = KBUILD_MODNAME, \ .version = _version, \ - } + }; \ + static const struct module_version_attribute \ + __used __attribute__ ((__section__ ("__modver"))) \ + * __moduleparam_const __modver_attr = &___modver_attr #endif /* Optional firmware file (or files) needed by the module diff --git a/kernel/params.c b/kernel/params.c index 7ab388a48a2..28c5d5c83f6 100644 --- a/kernel/params.c +++ b/kernel/params.c @@ -821,15 +821,18 @@ ssize_t __modver_version_show(struct module_attribute *mattr, return sprintf(buf, "%s\n", vattr->version); } -extern struct module_version_attribute __start___modver[], __stop___modver[]; +extern const struct module_version_attribute *__start___modver[]; +extern const struct module_version_attribute *__stop___modver[]; static void __init version_sysfs_builtin(void) { - const struct module_version_attribute *vattr; + const struct module_version_attribute **p; struct module_kobject *mk; int err; - for (vattr = __start___modver; vattr < __stop___modver; vattr++) { + for (p = __start___modver; p < __stop___modver; p++) { + const struct module_version_attribute *vattr = *p; + mk = locate_module_kobject(vattr->module_name); if (mk) { err = sysfs_create_file(&mk->kobj, &vattr->mattr.attr); -- cgit v1.2.3-70-g09d2 From 9b73a5840c7d5f77e5766626716df13787cb258c Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Mon, 7 Feb 2011 16:02:27 -0800 Subject: module: do not hide __modver_version_show declaration behind ifdef Doing so prevents the following warning from sparse: CHECK kernel/params.c kernel/params.c:817:9: warning: symbol '__modver_version_show' was not declared. Should it be static? since kernel/params.c is never compiled with MODULE being set. Signed-off-by: Dmitry Torokhov Signed-off-by: Rusty Russell --- include/linux/module.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/include/linux/module.h b/include/linux/module.h index 4cfebc21169..23996ad147e 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -64,6 +64,9 @@ struct module_version_attribute { const char *version; } __attribute__ ((__aligned__(sizeof(void *)))); +extern ssize_t __modver_version_show(struct module_attribute *, + struct module *, char *); + struct module_kobject { struct kobject kobj; @@ -172,8 +175,6 @@ extern struct module __this_module; #define MODULE_VERSION(_version) MODULE_INFO(version, _version) #else #define MODULE_VERSION(_version) \ - extern ssize_t __modver_version_show(struct module_attribute *, \ - struct module *, char *); \ static struct module_version_attribute ___modver_attr = { \ .mattr = { \ .attr = { \ -- cgit v1.2.3-70-g09d2 From a288bd651f4180c224cfddf837a0416157a36661 Mon Sep 17 00:00:00 2001 From: Richard Kennedy Date: Thu, 19 May 2011 16:55:25 -0600 Subject: module: remove 64 bit alignment padding from struct module with CONFIG_TRACE* Reorder struct module to remove 24 bytes of alignment padding on 64 bit builds when the CONFIG_TRACE options are selected. This allows the structure to fit into one fewer cache lines, and its size drops from 592 to 568 on x86_64. Signed-off-by: Richard Kennedy Signed-off-by: Rusty Russell --- include/linux/module.h | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/include/linux/module.h b/include/linux/module.h index 23996ad147e..65cc6cc73ca 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -368,34 +368,35 @@ struct module struct module_notes_attrs *notes_attrs; #endif + /* The command line arguments (may be mangled). People like + keeping pointers to this stuff */ + char *args; + #ifdef CONFIG_SMP /* Per-cpu data. */ void __percpu *percpu; unsigned int percpu_size; #endif - /* The command line arguments (may be mangled). People like - keeping pointers to this stuff */ - char *args; #ifdef CONFIG_TRACEPOINTS - struct tracepoint * const *tracepoints_ptrs; unsigned int num_tracepoints; + struct tracepoint * const *tracepoints_ptrs; #endif #ifdef HAVE_JUMP_LABEL struct jump_entry *jump_entries; unsigned int num_jump_entries; #endif #ifdef CONFIG_TRACING - const char **trace_bprintk_fmt_start; unsigned int num_trace_bprintk_fmt; + const char **trace_bprintk_fmt_start; #endif #ifdef CONFIG_EVENT_TRACING struct ftrace_event_call **trace_events; unsigned int num_trace_events; #endif #ifdef CONFIG_FTRACE_MCOUNT_RECORD - unsigned long *ftrace_callsites; unsigned int num_ftrace_callsites; + unsigned long *ftrace_callsites; #endif #ifdef CONFIG_MODULE_UNLOAD -- cgit v1.2.3-70-g09d2 From c5be0b2eb1ca05e0cd747f9c0ba552c6ee8827a0 Mon Sep 17 00:00:00 2001 From: Richard Kennedy Date: Thu, 19 May 2011 16:55:25 -0600 Subject: module: reorder kparam_array to remove alignment padding on 64 bit builds Reorder structure kparam_array to remove 8 bytes of alignment padding on 64 bit builds, dropping its size from 40 to 32 bytes. Also update the macro module_param_array_named to initialise the structure using its member names to allow it to be changed without touching all its call sites. 'git grep' finds module_param_array in 1037 places so this patch will save a small amount of data space across many modules. Signed-off-by: Richard Kennedy Signed-off-by: Rusty Russell --- include/linux/moduleparam.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/include/linux/moduleparam.h b/include/linux/moduleparam.h index 07b41951e3f..ddaae98c53f 100644 --- a/include/linux/moduleparam.h +++ b/include/linux/moduleparam.h @@ -67,9 +67,9 @@ struct kparam_string { struct kparam_array { unsigned int max; + unsigned int elemsize; unsigned int *num; const struct kernel_param_ops *ops; - unsigned int elemsize; void *elem; }; @@ -371,8 +371,9 @@ extern int param_get_invbool(char *buffer, const struct kernel_param *kp); */ #define module_param_array_named(name, array, type, nump, perm) \ static const struct kparam_array __param_arr_##name \ - = { ARRAY_SIZE(array), nump, ¶m_ops_##type, \ - sizeof(array[0]), array }; \ + = { .max = ARRAY_SIZE(array), .num = nump, \ + .ops = ¶m_ops_##type, \ + .elemsize = sizeof(array[0]), .elem = array }; \ __module_param_call(MODULE_PARAM_PREFIX, name, \ ¶m_array_ops, \ .arr = &__param_arr_##name, \ -- cgit v1.2.3-70-g09d2 From 5d05c70849f760ac8f4ed3ebfeefb92689858834 Mon Sep 17 00:00:00 2001 From: Daniel J Blueman Date: Tue, 8 Mar 2011 22:01:47 +0800 Subject: minor ANSI prototype sparse fix Fix function prototype to be ANSI-C compliant, consistent with other function prototypes, addressing a sparse warning. Signed-off-by: Daniel J Blueman Signed-off-by: Rusty Russell --- kernel/module.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kernel/module.c b/kernel/module.c index d5938a5c19c..523c40b7177 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1627,7 +1627,7 @@ void unset_section_ro_nx(struct module *mod, void *module_region) } /* Iterate through all modules and set each module's text as RW */ -void set_all_modules_text_rw() +void set_all_modules_text_rw(void) { struct module *mod; @@ -1648,7 +1648,7 @@ void set_all_modules_text_rw() } /* Iterate through all modules and set each module's text as RO */ -void set_all_modules_text_ro() +void set_all_modules_text_ro(void) { struct module *mod; -- cgit v1.2.3-70-g09d2 From 4d10380e720a3ce19dbe88d0133f66ded07b6a8f Mon Sep 17 00:00:00 2001 From: Jan Glauber Date: Thu, 19 May 2011 16:55:25 -0600 Subject: module: zero mod->init_ro_size after init is freed. Reset mod->init_ro_size to zero after the init part of a module is unloaded. Otherwise we need to check if module->init is NULL in the unprotect functions in the next patch. Signed-off-by: Jan Glauber Signed-off-by: Rusty Russell --- kernel/module.c | 1 + 1 file changed, 1 insertion(+) diff --git a/kernel/module.c b/kernel/module.c index 523c40b7177..92112c91b7e 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -2935,6 +2935,7 @@ SYSCALL_DEFINE3(init_module, void __user *, umod, module_free(mod, mod->module_init); mod->module_init = NULL; mod->init_size = 0; + mod->init_ro_size = 0; mod->init_text_size = 0; mutex_unlock(&module_mutex); -- cgit v1.2.3-70-g09d2 From 448694a1d50432be63aafccb42d6f54d8cf3d02c Mon Sep 17 00:00:00 2001 From: Jan Glauber Date: Thu, 19 May 2011 16:55:26 -0600 Subject: module: undo module RONX protection correctly. While debugging I stumbled over two problems in the code that protects module pages. First issue is that disabling the protection before freeing init or unload of a module is not symmetric with the enablement. For instance, if pages are set to RO the page range from module_core to module_core + core_ro_size is protected. If a module is unloaded the page range from module_core to module_core + core_size is set back to RW. So pages that were not set to RO are also changed to RW. This is not critical but IMHO it should be symmetric. Second issue is that while set_memory_rw & set_memory_ro are used for RO/RW changes only set_memory_nx is involved for NX/X. One would await that the inverse function is called when the NX protection should be removed, which is not the case here, unless I'm missing something. Signed-off-by: Jan Glauber Signed-off-by: Rusty Russell --- arch/s390/include/asm/cacheflush.h | 1 + arch/s390/mm/pageattr.c | 5 +++++ kernel/module.c | 25 +++++++++++++------------ 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/arch/s390/include/asm/cacheflush.h b/arch/s390/include/asm/cacheflush.h index 43a5c78046d..3e20383d092 100644 --- a/arch/s390/include/asm/cacheflush.h +++ b/arch/s390/include/asm/cacheflush.h @@ -11,5 +11,6 @@ void kernel_map_pages(struct page *page, int numpages, int enable); int set_memory_ro(unsigned long addr, int numpages); int set_memory_rw(unsigned long addr, int numpages); int set_memory_nx(unsigned long addr, int numpages); +int set_memory_x(unsigned long addr, int numpages); #endif /* _S390_CACHEFLUSH_H */ diff --git a/arch/s390/mm/pageattr.c b/arch/s390/mm/pageattr.c index 0607e4b14b2..f05edcc3bef 100644 --- a/arch/s390/mm/pageattr.c +++ b/arch/s390/mm/pageattr.c @@ -54,3 +54,8 @@ int set_memory_nx(unsigned long addr, int numpages) return 0; } EXPORT_SYMBOL_GPL(set_memory_nx); + +int set_memory_x(unsigned long addr, int numpages) +{ + return 0; +} diff --git a/kernel/module.c b/kernel/module.c index 92112c91b7e..b99dcebc980 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1607,22 +1607,23 @@ static void set_section_ro_nx(void *base, } } -/* Setting memory back to RW+NX before releasing it */ +/* Setting memory back to W+X before releasing it */ void unset_section_ro_nx(struct module *mod, void *module_region) { - unsigned long total_pages; - if (mod->module_core == module_region) { - /* Set core as NX+RW */ - total_pages = MOD_NUMBER_OF_PAGES(mod->module_core, mod->core_size); - set_memory_nx((unsigned long)mod->module_core, total_pages); - set_memory_rw((unsigned long)mod->module_core, total_pages); - + set_page_attributes(mod->module_core + mod->core_text_size, + mod->module_core + mod->core_size, + set_memory_x); + set_page_attributes(mod->module_core, + mod->module_core + mod->core_ro_size, + set_memory_rw); } else if (mod->module_init == module_region) { - /* Set init as NX+RW */ - total_pages = MOD_NUMBER_OF_PAGES(mod->module_init, mod->init_size); - set_memory_nx((unsigned long)mod->module_init, total_pages); - set_memory_rw((unsigned long)mod->module_init, total_pages); + set_page_attributes(mod->module_init + mod->init_text_size, + mod->module_init + mod->init_size, + set_memory_x); + set_page_attributes(mod->module_init, + mod->module_init + mod->init_ro_size, + set_memory_rw); } } -- cgit v1.2.3-70-g09d2 From 01526ed0830643bd53a8434c3068e4c077e1b09d Mon Sep 17 00:00:00 2001 From: Jan Glauber Date: Thu, 19 May 2011 16:55:26 -0600 Subject: module: split unset_section_ro_nx function. Split the unprotect function into a function per section to make the code more readable and add the missing static declaration. Signed-off-by: Jan Glauber Signed-off-by: Rusty Russell --- kernel/module.c | 45 +++++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/kernel/module.c b/kernel/module.c index b99dcebc980..0e6f97f43c8 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1607,24 +1607,24 @@ static void set_section_ro_nx(void *base, } } -/* Setting memory back to W+X before releasing it */ -void unset_section_ro_nx(struct module *mod, void *module_region) -{ - if (mod->module_core == module_region) { - set_page_attributes(mod->module_core + mod->core_text_size, - mod->module_core + mod->core_size, - set_memory_x); - set_page_attributes(mod->module_core, - mod->module_core + mod->core_ro_size, - set_memory_rw); - } else if (mod->module_init == module_region) { - set_page_attributes(mod->module_init + mod->init_text_size, - mod->module_init + mod->init_size, - set_memory_x); - set_page_attributes(mod->module_init, - mod->module_init + mod->init_ro_size, - set_memory_rw); - } +static void unset_module_core_ro_nx(struct module *mod) +{ + set_page_attributes(mod->module_core + mod->core_text_size, + mod->module_core + mod->core_size, + set_memory_x); + set_page_attributes(mod->module_core, + mod->module_core + mod->core_ro_size, + set_memory_rw); +} + +static void unset_module_init_ro_nx(struct module *mod) +{ + set_page_attributes(mod->module_init + mod->init_text_size, + mod->module_init + mod->init_size, + set_memory_x); + set_page_attributes(mod->module_init, + mod->module_init + mod->init_ro_size, + set_memory_rw); } /* Iterate through all modules and set each module's text as RW */ @@ -1670,7 +1670,8 @@ void set_all_modules_text_ro(void) } #else static inline void set_section_ro_nx(void *base, unsigned long text_size, unsigned long ro_size, unsigned long total_size) { } -static inline void unset_section_ro_nx(struct module *mod, void *module_region) { } +static void unset_module_core_ro_nx(struct module *mod) { } +static void unset_module_init_ro_nx(struct module *mod) { } #endif /* Free a module, remove from lists, etc. */ @@ -1697,7 +1698,7 @@ static void free_module(struct module *mod) destroy_params(mod->kp, mod->num_kp); /* This may be NULL, but that's OK */ - unset_section_ro_nx(mod, mod->module_init); + unset_module_init_ro_nx(mod); module_free(mod, mod->module_init); kfree(mod->args); percpu_modfree(mod); @@ -1706,7 +1707,7 @@ static void free_module(struct module *mod) lockdep_free_key_range(mod->module_core, mod->core_size); /* Finally, free the core (containing the module structure) */ - unset_section_ro_nx(mod, mod->module_core); + unset_module_core_ro_nx(mod); module_free(mod, mod->module_core); #ifdef CONFIG_MPU @@ -2932,7 +2933,7 @@ SYSCALL_DEFINE3(init_module, void __user *, umod, mod->symtab = mod->core_symtab; mod->strtab = mod->core_strtab; #endif - unset_section_ro_nx(mod, mod->module_init); + unset_module_init_ro_nx(mod); module_free(mod, mod->module_init); mod->module_init = NULL; mod->init_size = 0; -- cgit v1.2.3-70-g09d2 From de4d8d53465483168d6a627d409ee2d09d8e3308 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 19 Apr 2011 21:49:58 +0200 Subject: module: each_symbol_section instead of each_symbol Instead of having a callback function for each symbol in the kernel, have a callback for each array of symbols. This eases the logic when we move to sorted symbols and binary search. Signed-off-by: Rusty Russell Signed-off-by: Alessio Igor Bogani --- include/linux/module.h | 5 +++-- kernel/module.c | 42 +++++++++++++++++++++++++++--------------- 2 files changed, 30 insertions(+), 17 deletions(-) diff --git a/include/linux/module.h b/include/linux/module.h index 65cc6cc73ca..49f4ad0ddec 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -477,8 +477,9 @@ const struct kernel_symbol *find_symbol(const char *name, bool warn); /* Walk the exported symbol table */ -bool each_symbol(bool (*fn)(const struct symsearch *arr, struct module *owner, - unsigned int symnum, void *data), void *data); +bool each_symbol_section(bool (*fn)(const struct symsearch *arr, + struct module *owner, + void *data), void *data); /* Returns 0 and fills in value, defined and namebuf, or -ERANGE if symnum out of range. */ diff --git a/kernel/module.c b/kernel/module.c index 0e6f97f43c8..e8aa462301e 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -240,23 +240,24 @@ static bool each_symbol_in_section(const struct symsearch *arr, struct module *owner, bool (*fn)(const struct symsearch *syms, struct module *owner, - unsigned int symnum, void *data), + void *data), void *data) { - unsigned int i, j; + unsigned int j; for (j = 0; j < arrsize; j++) { - for (i = 0; i < arr[j].stop - arr[j].start; i++) - if (fn(&arr[j], owner, i, data)) - return true; + if (fn(&arr[j], owner, data)) + return true; } return false; } /* Returns true as soon as fn returns true, otherwise false. */ -bool each_symbol(bool (*fn)(const struct symsearch *arr, struct module *owner, - unsigned int symnum, void *data), void *data) +bool each_symbol_section(bool (*fn)(const struct symsearch *arr, + struct module *owner, + void *data), + void *data) { struct module *mod; static const struct symsearch arr[] = { @@ -309,7 +310,7 @@ bool each_symbol(bool (*fn)(const struct symsearch *arr, struct module *owner, } return false; } -EXPORT_SYMBOL_GPL(each_symbol); +EXPORT_SYMBOL_GPL(each_symbol_section); struct find_symbol_arg { /* Input */ @@ -323,15 +324,12 @@ struct find_symbol_arg { const struct kernel_symbol *sym; }; -static bool find_symbol_in_section(const struct symsearch *syms, - struct module *owner, - unsigned int symnum, void *data) +static bool check_symbol(const struct symsearch *syms, + struct module *owner, + unsigned int symnum, void *data) { struct find_symbol_arg *fsa = data; - if (strcmp(syms->start[symnum].name, fsa->name) != 0) - return false; - if (!fsa->gplok) { if (syms->licence == GPL_ONLY) return false; @@ -365,6 +363,20 @@ static bool find_symbol_in_section(const struct symsearch *syms, return true; } +static bool find_symbol_in_section(const struct symsearch *syms, + struct module *owner, + void *data) +{ + struct find_symbol_arg *fsa = data; + unsigned int i; + + for (i = 0; i < syms->stop - syms->start; i++) { + if (strcmp(syms->start[i].name, fsa->name) == 0) + return check_symbol(syms, owner, i, data); + } + return false; +} + /* Find a symbol and return it, along with, (optional) crc and * (optional) module which owns it. Needs preempt disabled or module_mutex. */ const struct kernel_symbol *find_symbol(const char *name, @@ -379,7 +391,7 @@ const struct kernel_symbol *find_symbol(const char *name, fsa.gplok = gplok; fsa.warn = warn; - if (each_symbol(find_symbol_in_section, &fsa)) { + if (each_symbol_section(find_symbol_in_section, &fsa)) { if (owner) *owner = fsa.owner; if (crc) -- cgit v1.2.3-70-g09d2 From f02e8a6596b7dc9b2171f7ff5654039ef0950cdc Mon Sep 17 00:00:00 2001 From: Alessio Igor Bogani Date: Thu, 14 Apr 2011 14:59:39 +0200 Subject: module: Sort exported symbols This patch places every exported symbol in its own section (i.e. "___ksymtab+printk"). Thus the linker will use its SORT() directive to sort and finally merge all symbol in the right and final section (i.e. "__ksymtab"). The symbol prefixed archs use an underscore as prefix for symbols. To avoid collision we use a different character to create the temporary section names. This work was supported by a hardware donation from the CE Linux Forum. Signed-off-by: Alessio Igor Bogani Signed-off-by: Rusty Russell (folded in '+' fixup) Tested-by: Dirk Behme --- include/asm-generic/vmlinux.lds.h | 20 ++++++++++---------- include/linux/module.h | 4 ++-- scripts/module-common.lds | 11 +++++++++++ 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index bd297a20ab9..b27445e00b6 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -274,70 +274,70 @@ /* Kernel symbol table: Normal symbols */ \ __ksymtab : AT(ADDR(__ksymtab) - LOAD_OFFSET) { \ VMLINUX_SYMBOL(__start___ksymtab) = .; \ - *(__ksymtab) \ + *(SORT(___ksymtab+*)) \ VMLINUX_SYMBOL(__stop___ksymtab) = .; \ } \ \ /* Kernel symbol table: GPL-only symbols */ \ __ksymtab_gpl : AT(ADDR(__ksymtab_gpl) - LOAD_OFFSET) { \ VMLINUX_SYMBOL(__start___ksymtab_gpl) = .; \ - *(__ksymtab_gpl) \ + *(SORT(___ksymtab_gpl+*)) \ VMLINUX_SYMBOL(__stop___ksymtab_gpl) = .; \ } \ \ /* Kernel symbol table: Normal unused symbols */ \ __ksymtab_unused : AT(ADDR(__ksymtab_unused) - LOAD_OFFSET) { \ VMLINUX_SYMBOL(__start___ksymtab_unused) = .; \ - *(__ksymtab_unused) \ + *(SORT(___ksymtab_unused+*)) \ VMLINUX_SYMBOL(__stop___ksymtab_unused) = .; \ } \ \ /* Kernel symbol table: GPL-only unused symbols */ \ __ksymtab_unused_gpl : AT(ADDR(__ksymtab_unused_gpl) - LOAD_OFFSET) { \ VMLINUX_SYMBOL(__start___ksymtab_unused_gpl) = .; \ - *(__ksymtab_unused_gpl) \ + *(SORT(___ksymtab_unused_gpl+*)) \ VMLINUX_SYMBOL(__stop___ksymtab_unused_gpl) = .; \ } \ \ /* Kernel symbol table: GPL-future-only symbols */ \ __ksymtab_gpl_future : AT(ADDR(__ksymtab_gpl_future) - LOAD_OFFSET) { \ VMLINUX_SYMBOL(__start___ksymtab_gpl_future) = .; \ - *(__ksymtab_gpl_future) \ + *(SORT(___ksymtab_gpl_future+*)) \ VMLINUX_SYMBOL(__stop___ksymtab_gpl_future) = .; \ } \ \ /* Kernel symbol table: Normal symbols */ \ __kcrctab : AT(ADDR(__kcrctab) - LOAD_OFFSET) { \ VMLINUX_SYMBOL(__start___kcrctab) = .; \ - *(__kcrctab) \ + *(SORT(___kcrctab+*)) \ VMLINUX_SYMBOL(__stop___kcrctab) = .; \ } \ \ /* Kernel symbol table: GPL-only symbols */ \ __kcrctab_gpl : AT(ADDR(__kcrctab_gpl) - LOAD_OFFSET) { \ VMLINUX_SYMBOL(__start___kcrctab_gpl) = .; \ - *(__kcrctab_gpl) \ + *(SORT(___kcrctab_gpl+*)) \ VMLINUX_SYMBOL(__stop___kcrctab_gpl) = .; \ } \ \ /* Kernel symbol table: Normal unused symbols */ \ __kcrctab_unused : AT(ADDR(__kcrctab_unused) - LOAD_OFFSET) { \ VMLINUX_SYMBOL(__start___kcrctab_unused) = .; \ - *(__kcrctab_unused) \ + *(SORT(___kcrctab_unused+*)) \ VMLINUX_SYMBOL(__stop___kcrctab_unused) = .; \ } \ \ /* Kernel symbol table: GPL-only unused symbols */ \ __kcrctab_unused_gpl : AT(ADDR(__kcrctab_unused_gpl) - LOAD_OFFSET) { \ VMLINUX_SYMBOL(__start___kcrctab_unused_gpl) = .; \ - *(__kcrctab_unused_gpl) \ + *(SORT(___kcrctab_unused_gpl+*)) \ VMLINUX_SYMBOL(__stop___kcrctab_unused_gpl) = .; \ } \ \ /* Kernel symbol table: GPL-future-only symbols */ \ __kcrctab_gpl_future : AT(ADDR(__kcrctab_gpl_future) - LOAD_OFFSET) { \ VMLINUX_SYMBOL(__start___kcrctab_gpl_future) = .; \ - *(__kcrctab_gpl_future) \ + *(SORT(___kcrctab_gpl_future+*)) \ VMLINUX_SYMBOL(__stop___kcrctab_gpl_future) = .; \ } \ \ diff --git a/include/linux/module.h b/include/linux/module.h index 49f4ad0ddec..d9ca2d5dc6d 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -224,7 +224,7 @@ struct module_use { extern void *__crc_##sym __attribute__((weak)); \ static const unsigned long __kcrctab_##sym \ __used \ - __attribute__((section("__kcrctab" sec), unused)) \ + __attribute__((section("___kcrctab" sec "+" #sym), unused)) \ = (unsigned long) &__crc_##sym; #else #define __CRC_SYMBOL(sym, sec) @@ -239,7 +239,7 @@ struct module_use { = MODULE_SYMBOL_PREFIX #sym; \ static const struct kernel_symbol __ksymtab_##sym \ __used \ - __attribute__((section("__ksymtab" sec), unused)) \ + __attribute__((section("___ksymtab" sec "+" #sym), unused)) \ = { (unsigned long)&sym, __kstrtab_##sym } #define EXPORT_SYMBOL(sym) \ diff --git a/scripts/module-common.lds b/scripts/module-common.lds index 47a1f9ae0ed..0865b3e752b 100644 --- a/scripts/module-common.lds +++ b/scripts/module-common.lds @@ -5,4 +5,15 @@ */ SECTIONS { /DISCARD/ : { *(.discard) } + + __ksymtab : { *(SORT(___ksymtab+*)) } + __ksymtab_gpl : { *(SORT(___ksymtab_gpl+*)) } + __ksymtab_unused : { *(SORT(___ksymtab_unused+*)) } + __ksymtab_unused_gpl : { *(SORT(___ksymtab_unused_gpl+*)) } + __ksymtab_gpl_future : { *(SORT(___ksymtab_gpl_future+*)) } + __kcrctab : { *(SORT(___kcrctab+*)) } + __kcrctab_gpl : { *(SORT(___kcrctab_gpl+*)) } + __kcrctab_unused : { *(SORT(___kcrctab_unused+*)) } + __kcrctab_unused_gpl : { *(SORT(___kcrctab_unused_gpl+*)) } + __kcrctab_gpl_future : { *(SORT(___kcrctab_gpl_future+*)) } } -- cgit v1.2.3-70-g09d2 From 1a94dc35bc5c166d89913dc01a49d27a3c21a455 Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Thu, 14 Apr 2011 20:00:19 +0200 Subject: lib: Add generic binary search function to the kernel. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There a large number hand-coded binary searches in the kernel (run "git grep search | grep binary" to find many of them). Since in my experience, hand-coding binary searches can be error-prone, it seems worth cleaning this up by providing a generic binary search function. This generic binary search implementation comes from Ksplice. It has the same basic API as the C library bsearch() function. Ksplice uses it in half a dozen places with 4 different comparison functions, and I think our code is substantially cleaner because of this. Signed-off-by: Tim Abbott Extra-bikeshedding-by: Alan Jenkins Extra-bikeshedding-by: AndrĂ© Goddard Rosa Extra-bikeshedding-by: Rusty Russell Signed-off-by: Rusty Russell Signed-off-by: Alessio Igor Bogani Signed-off-by: Rusty Russell --- include/linux/bsearch.h | 9 +++++++++ lib/Makefile | 3 ++- lib/bsearch.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 1 deletion(-) create mode 100644 include/linux/bsearch.h create mode 100644 lib/bsearch.c diff --git a/include/linux/bsearch.h b/include/linux/bsearch.h new file mode 100644 index 00000000000..90b1aa86722 --- /dev/null +++ b/include/linux/bsearch.h @@ -0,0 +1,9 @@ +#ifndef _LINUX_BSEARCH_H +#define _LINUX_BSEARCH_H + +#include + +void *bsearch(const void *key, const void *base, size_t num, size_t size, + int (*cmp)(const void *key, const void *elt)); + +#endif /* _LINUX_BSEARCH_H */ diff --git a/lib/Makefile b/lib/Makefile index ef0f2857115..4b49a249064 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -21,7 +21,8 @@ lib-y += kobject.o kref.o klist.o obj-y += bcd.o div64.o sort.o parser.o halfmd4.o debug_locks.o random32.o \ bust_spinlocks.o hexdump.o kasprintf.o bitmap.o scatterlist.o \ - string_helpers.o gcd.o lcm.o list_sort.o uuid.o flex_array.o + string_helpers.o gcd.o lcm.o list_sort.o uuid.o flex_array.o \ + bsearch.o obj-y += kstrtox.o obj-$(CONFIG_TEST_KSTRTOX) += test-kstrtox.o diff --git a/lib/bsearch.c b/lib/bsearch.c new file mode 100644 index 00000000000..5b54758e2af --- /dev/null +++ b/lib/bsearch.c @@ -0,0 +1,53 @@ +/* + * A generic implementation of binary search for the Linux kernel + * + * Copyright (C) 2008-2009 Ksplice, Inc. + * Author: Tim Abbott + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; version 2. + */ + +#include +#include + +/* + * bsearch - binary search an array of elements + * @key: pointer to item being searched for + * @base: pointer to first element to search + * @num: number of elements + * @size: size of each element + * @cmp: pointer to comparison function + * + * This function does a binary search on the given array. The + * contents of the array should already be in ascending sorted order + * under the provided comparison function. + * + * Note that the key need not have the same type as the elements in + * the array, e.g. key could be a string and the comparison function + * could compare the string with the struct's name field. However, if + * the key and elements in the array are of the same type, you can use + * the same comparison function for both sort() and bsearch(). + */ +void *bsearch(const void *key, const void *base, size_t num, size_t size, + int (*cmp)(const void *key, const void *elt)) +{ + size_t start = 0, end = num; + int result; + + while (start < end) { + size_t mid = start + (end - start) / 2; + + result = cmp(key, base + mid * size); + if (result < 0) + end = mid; + else if (result > 0) + start = mid + 1; + else + return (void *)base + mid * size; + } + + return NULL; +} +EXPORT_SYMBOL(bsearch); -- cgit v1.2.3-70-g09d2 From 403ed27846aa126ecf0b842b5b179c506b9d989c Mon Sep 17 00:00:00 2001 From: Alessio Igor Bogani Date: Wed, 20 Apr 2011 11:10:52 +0200 Subject: module: Use the binary search for symbols resolution Takes advantage of the order and locates symbols using binary search. This work was supported by a hardware donation from the CE Linux Forum. Signed-off-by: Alessio Igor Bogani Signed-off-by: Rusty Russell Tested-by: Dirk Behme --- kernel/module.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/kernel/module.c b/kernel/module.c index e8aa462301e..d1db8eb56ad 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -57,6 +57,7 @@ #include #include #include +#include #define CREATE_TRACE_POINTS #include @@ -363,17 +364,27 @@ static bool check_symbol(const struct symsearch *syms, return true; } +static int cmp_name(const void *va, const void *vb) +{ + const char *a; + const struct kernel_symbol *b; + a = va; b = vb; + return strcmp(a, b->name); +} + static bool find_symbol_in_section(const struct symsearch *syms, struct module *owner, void *data) { struct find_symbol_arg *fsa = data; - unsigned int i; + struct kernel_symbol *sym; + + sym = bsearch(fsa->name, syms->start, syms->stop - syms->start, + sizeof(struct kernel_symbol), cmp_name); + + if (sym != NULL && check_symbol(syms, owner, sym - syms->start, data)) + return true; - for (i = 0; i < syms->stop - syms->start; i++) { - if (strcmp(syms->start[i].name, fsa->name) == 0) - return check_symbol(syms, owner, i, data); - } return false; } -- cgit v1.2.3-70-g09d2 From 9d63487f86115b1d3ef69670043bcf2b83c4d227 Mon Sep 17 00:00:00 2001 From: Alessio Igor Bogani Date: Wed, 18 May 2011 22:35:59 +0200 Subject: module: Use binary search in lookup_symbol() The function is_exported() with its helper function lookup_symbol() are used to verify if a provided symbol is effectively exported by the kernel or by the modules. Now that both have their symbols sorted we can replace a linear search with a binary search which provide a considerably speed-up. This work was supported by a hardware donation from the CE Linux Forum. Signed-off-by: Alessio Igor Bogani Acked-by: Greg Kroah-Hartman Signed-off-by: Rusty Russell --- kernel/module.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/kernel/module.c b/kernel/module.c index d1db8eb56ad..22879725678 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -2055,11 +2055,8 @@ static const struct kernel_symbol *lookup_symbol(const char *name, const struct kernel_symbol *start, const struct kernel_symbol *stop) { - const struct kernel_symbol *ks = start; - for (; ks < stop; ks++) - if (strcmp(ks->name, name) == 0) - return ks; - return NULL; + return bsearch(name, start, stop - start, + sizeof(struct kernel_symbol), cmp_name); } static int is_exported(const char *name, unsigned long value, -- cgit v1.2.3-70-g09d2 From 6845756b29e4c4e7db41e2d75cafa9d091bc1c07 Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Thu, 19 May 2011 16:55:27 -0600 Subject: modpost: Update 64k section support for binutils 2.18.50 Binutils 2.18.50 made a backwards-incompatible change in the way it writes ELF objects with over 65280 sections, to improve conformance with the ELF specification and interoperability with other ELF tools. Specifically, it no longer adds 256 to section indices SHN_LORESERVE and higher to skip over the reserved range SHN_LORESERVE through SHN_HIRESERVE; those values are only considered special in the st_shndx field, and not in other places where section indices are stored. See: http://sourceware.org/bugzilla/show_bug.cgi?id=5900 http://groups.google.com/group/generic-abi/browse_thread/thread/e8bb63714b072e67/6c63738f12cc8a17 Signed-off-by: Anders Kaseorg Signed-off-by: Rusty Russell --- scripts/mod/modpost.c | 16 ++++++---------- scripts/mod/modpost.h | 27 ++++++++------------------- 2 files changed, 14 insertions(+), 29 deletions(-) diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index cd104afcc5f..413c53693e6 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -420,11 +420,10 @@ static int parse_elf(struct elf_info *info, const char *filename) return 0; } - if (hdr->e_shnum == 0) { + if (hdr->e_shnum == SHN_UNDEF) { /* * There are more than 64k sections, * read count from .sh_size. - * note: it doesn't need shndx2secindex() */ info->num_sections = TO_NATIVE(sechdrs[0].sh_size); } @@ -432,8 +431,7 @@ static int parse_elf(struct elf_info *info, const char *filename) info->num_sections = hdr->e_shnum; } if (hdr->e_shstrndx == SHN_XINDEX) { - info->secindex_strings = - shndx2secindex(TO_NATIVE(sechdrs[0].sh_link)); + info->secindex_strings = TO_NATIVE(sechdrs[0].sh_link); } else { info->secindex_strings = hdr->e_shstrndx; @@ -489,7 +487,7 @@ static int parse_elf(struct elf_info *info, const char *filename) sechdrs[i].sh_offset; info->symtab_stop = (void *)hdr + sechdrs[i].sh_offset + sechdrs[i].sh_size; - sh_link_idx = shndx2secindex(sechdrs[i].sh_link); + sh_link_idx = sechdrs[i].sh_link; info->strtab = (void *)hdr + sechdrs[sh_link_idx].sh_offset; } @@ -516,11 +514,9 @@ static int parse_elf(struct elf_info *info, const char *filename) if (symtab_shndx_idx != ~0U) { Elf32_Word *p; - if (symtab_idx != - shndx2secindex(sechdrs[symtab_shndx_idx].sh_link)) + if (symtab_idx != sechdrs[symtab_shndx_idx].sh_link) fatal("%s: SYMTAB_SHNDX has bad sh_link: %u!=%u\n", - filename, - shndx2secindex(sechdrs[symtab_shndx_idx].sh_link), + filename, sechdrs[symtab_shndx_idx].sh_link, symtab_idx); /* Fix endianness */ for (p = info->symtab_shndx_start; p < info->symtab_shndx_stop; @@ -1446,7 +1442,7 @@ static unsigned int *reloc_location(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r) { Elf_Shdr *sechdrs = elf->sechdrs; - int section = shndx2secindex(sechdr->sh_info); + int section = sechdr->sh_info; return (void *)elf->hdr + sechdrs[section].sh_offset + r->r_offset; diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h index 0388cfccac8..2031119080d 100644 --- a/scripts/mod/modpost.h +++ b/scripts/mod/modpost.h @@ -145,33 +145,22 @@ static inline int is_shndx_special(unsigned int i) return i != SHN_XINDEX && i >= SHN_LORESERVE && i <= SHN_HIRESERVE; } -/* shndx is in [0..SHN_LORESERVE) U (SHN_HIRESERVE, 0xfffffff], thus: - * shndx == 0 <=> sechdrs[0] - * ...... - * shndx == SHN_LORESERVE-1 <=> sechdrs[SHN_LORESERVE-1] - * shndx == SHN_HIRESERVE+1 <=> sechdrs[SHN_LORESERVE] - * shndx == SHN_HIRESERVE+2 <=> sechdrs[SHN_LORESERVE+1] - * ...... - * fyi: sym->st_shndx is uint16, SHN_LORESERVE = ff00, SHN_HIRESERVE = ffff, - * so basically we map 0000..feff -> 0000..feff - * ff00..ffff -> (you are a bad boy, dont do it) - * 10000..xxxx -> ff00..(xxxx-0x100) +/* + * Move reserved section indices SHN_LORESERVE..SHN_HIRESERVE out of + * the way to -256..-1, to avoid conflicting with real section + * indices. */ -static inline unsigned int shndx2secindex(unsigned int i) -{ - if (i <= SHN_HIRESERVE) - return i; - return i - (SHN_HIRESERVE + 1 - SHN_LORESERVE); -} +#define SPECIAL(i) ((i) - (SHN_HIRESERVE + 1)) /* Accessor for sym->st_shndx, hides ugliness of "64k sections" */ static inline unsigned int get_secindex(const struct elf_info *info, const Elf_Sym *sym) { + if (is_shndx_special(sym->st_shndx)) + return SPECIAL(sym->st_shndx); if (sym->st_shndx != SHN_XINDEX) return sym->st_shndx; - return shndx2secindex(info->symtab_shndx_start[sym - - info->symtab_start]); + return info->symtab_shndx_start[sym - info->symtab_start]; } /* file2alias.c */ -- cgit v1.2.3-70-g09d2 From d0f1fed29e6e73d9d17f4c91a5896a4ce3938d45 Mon Sep 17 00:00:00 2001 From: Jonathan Cameron Date: Tue, 19 Apr 2011 12:43:45 +0100 Subject: Add a strtobool function matching semantics of existing in kernel equivalents This is a rename of the usr_strtobool proposal, which was a renamed, relocated and fixed version of previous kstrtobool RFC Signed-off-by: Jonathan Cameron Signed-off-by: Rusty Russell --- include/linux/string.h | 1 + lib/string.c | 29 +++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/include/linux/string.h b/include/linux/string.h index a716ee2a8ad..a176db2f2c8 100644 --- a/include/linux/string.h +++ b/include/linux/string.h @@ -123,6 +123,7 @@ extern char **argv_split(gfp_t gfp, const char *str, int *argcp); extern void argv_free(char **argv); extern bool sysfs_streq(const char *s1, const char *s2); +extern int strtobool(const char *s, bool *res); #ifdef CONFIG_BINARY_PRINTF int vbin_printf(u32 *bin_buf, size_t size, const char *fmt, va_list args); diff --git a/lib/string.c b/lib/string.c index f71bead1be3..01fad9b203e 100644 --- a/lib/string.c +++ b/lib/string.c @@ -535,6 +535,35 @@ bool sysfs_streq(const char *s1, const char *s2) } EXPORT_SYMBOL(sysfs_streq); +/** + * strtobool - convert common user inputs into boolean values + * @s: input string + * @res: result + * + * This routine returns 0 iff the first character is one of 'Yy1Nn0'. + * Otherwise it will return -EINVAL. Value pointed to by res is + * updated upon finding a match. + */ +int strtobool(const char *s, bool *res) +{ + switch (s[0]) { + case 'y': + case 'Y': + case '1': + *res = true; + break; + case 'n': + case 'N': + case '0': + *res = false; + break; + default: + return -EINVAL; + } + return 0; +} +EXPORT_SYMBOL(strtobool); + #ifndef __HAVE_ARCH_MEMSET /** * memset - Fill a region of memory with the given value -- cgit v1.2.3-70-g09d2 From a0374396375d06398c419ebb6857fb5809cff81f Mon Sep 17 00:00:00 2001 From: Jonathan Cameron Date: Tue, 19 Apr 2011 12:43:46 +0100 Subject: debugfs: move to new strtobool No functional changes requires that we eat errors from strtobool. If people want to not do this, then it should be fixed at a later date. V2: Simplification suggested by Rusty Russell removes the need for additional variable ret. Signed-off-by: Jonathan Cameron Signed-off-by: Rusty Russell --- fs/debugfs/file.c | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c index 89d394d8fe2..568304d058a 100644 --- a/fs/debugfs/file.c +++ b/fs/debugfs/file.c @@ -429,25 +429,16 @@ static ssize_t write_file_bool(struct file *file, const char __user *user_buf, { char buf[32]; int buf_size; + bool bv; u32 *val = file->private_data; buf_size = min(count, (sizeof(buf)-1)); if (copy_from_user(buf, user_buf, buf_size)) return -EFAULT; - switch (buf[0]) { - case 'y': - case 'Y': - case '1': - *val = 1; - break; - case 'n': - case 'N': - case '0': - *val = 0; - break; - } - + if (strtobool(buf, &bv) == 0) + *val = bv; + return count; } -- cgit v1.2.3-70-g09d2 From f721a465cddbe7f03e6cd2272008da558cf93818 Mon Sep 17 00:00:00 2001 From: Jonathan Cameron Date: Tue, 19 Apr 2011 12:43:47 +0100 Subject: params.c: Use new strtobool function to process boolean inputs Signed-off-by: Jonathan Cameron Signed-off-by: Rusty Russell --- kernel/params.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/kernel/params.c b/kernel/params.c index 28c5d5c83f6..ed72e133086 100644 --- a/kernel/params.c +++ b/kernel/params.c @@ -297,21 +297,15 @@ EXPORT_SYMBOL(param_ops_charp); int param_set_bool(const char *val, const struct kernel_param *kp) { bool v; + int ret; /* No equals means "set"... */ if (!val) val = "1"; /* One of =[yYnN01] */ - switch (val[0]) { - case 'y': case 'Y': case '1': - v = true; - break; - case 'n': case 'N': case '0': - v = false; - break; - default: - return -EINVAL; - } + ret = strtobool(val, &v); + if (ret) + return ret; if (kp->flags & KPARAM_ISBOOL) *(bool *)kp->arg = v; -- cgit v1.2.3-70-g09d2