diff --git a/arch/xtensa/platforms/iss/simdisk.c b/arch/xtensa/platforms/iss/simdisk.c index 7c7a2aa..a2ddb49 100644 --- a/arch/xtensa/platforms/iss/simdisk.c +++ b/arch/xtensa/platforms/iss/simdisk.c @@ -41,7 +41,7 @@ module_param(simdisk_count, int, S_IRUGO); MODULE_PARM_DESC(simdisk_count, "Number of simdisk units."); static int n_files; -static const char *filename[MAX_SIMDISK_COUNT] = { +static char *filename[MAX_SIMDISK_COUNT] = { #ifdef CONFIG_SIMDISK0_FILENAME CONFIG_SIMDISK0_FILENAME, #ifdef CONFIG_SIMDISK1_FILENAME @@ -50,20 +50,48 @@ static const char *filename[MAX_SIMDISK_COUNT] = { #endif }; +/* + * The simdisk code can be built either into the kernel or as a loadable module. + * When built-in, CONFIG_SIMDISK{0,1}_FILENAME can be used to specify the + * initial simdisk filenames and additional filenames can be provided on the + * kernel command line. These arguments are parsed during early boot when slab + * is not yet available, but the command line itself is preserved for the + * lifetime of the kernel, so the incoming pointer is stored directly. + * When built as a loadable module, each value is copied with kstrdup() and all + * allocated memory is freed in simdisk_param_free_filename() when the module is + * unloaded. + */ static int simdisk_param_set_filename(const char *val, const struct kernel_param *kp) { - if (n_files < ARRAY_SIZE(filename)) - filename[n_files++] = val; - else + char *str; + + if (n_files >= ARRAY_SIZE(filename)) return -EINVAL; + +#ifdef MODULE + str = kstrdup(val, GFP_KERNEL); + if (!str) + return -ENOMEM; +#else + str = (char *)val; +#endif + + filename[n_files++] = str; return 0; } +static void simdisk_param_free_filename(void *arg) +{ + for (int i = 0; i < n_files; i++) + kfree(filename[i]); +} + static const struct kernel_param_ops simdisk_param_ops_filename = { .set = simdisk_param_set_filename, + .free = simdisk_param_free_filename, }; -module_param_cb(filename, &simdisk_param_ops_filename, &n_files, 0); +module_param_cb(filename, &simdisk_param_ops_filename, NULL, 0); MODULE_PARM_DESC(filename, "Backing storage filename."); static int simdisk_major = SIMDISK_MAJOR; diff --git a/include/linux/module.h b/include/linux/module.h index 7566815..96cc985 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -477,10 +477,6 @@ 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; diff --git a/kernel/module/dups.c b/kernel/module/dups.c index 1d720a5..c9be9df 100644 --- a/kernel/module/dups.c +++ b/kernel/module/dups.c @@ -7,29 +7,22 @@ #define pr_fmt(fmt) "module: " fmt +#include +#include +#include +#include +#include +#include #include +#include +#include +#include +#include +#include #include -#include -#include -#include -#include -#include #include -#include -#include -#include +#include #include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include #include "internal.h" @@ -38,32 +31,42 @@ static bool enable_dups_trace = IS_ENABLED(CONFIG_MODULE_DEBUG_AUTOLOAD_DUPS_TRACE); module_param(enable_dups_trace, bool_enable_only, 0644); -/* - * Protects dup_kmod_reqs list, adds / removals with RCU. - */ +/* A mutex-protected list of active kmod requests. */ static DEFINE_MUTEX(kmod_dup_mutex); static LIST_HEAD(dup_kmod_reqs); struct kmod_dup_req { + refcount_t refcount; struct list_head list; char name[MODULE_NAME_LEN]; struct completion first_req_done; - struct work_struct complete_work; struct delayed_work delete_work; int dup_ret; }; +static void get_kmod_req(struct kmod_dup_req *kmod_req) +{ + refcount_inc(&kmod_req->refcount); +} + +static void put_kmod_req(struct kmod_dup_req *kmod_req) +{ + if (refcount_dec_and_test(&kmod_req->refcount)) + kfree(kmod_req); +} + +DEFINE_FREE(put_kmod_req, struct kmod_dup_req *, if (_T) put_kmod_req(_T)) + static struct kmod_dup_req *kmod_dup_request_lookup(char *module_name) { struct kmod_dup_req *kmod_req; - list_for_each_entry_rcu(kmod_req, &dup_kmod_reqs, list, - lockdep_is_held(&kmod_dup_mutex)) { - if (strlen(kmod_req->name) == strlen(module_name) && - !memcmp(kmod_req->name, module_name, strlen(module_name))) { + lockdep_assert_held(&kmod_dup_mutex); + + list_for_each_entry(kmod_req, &dup_kmod_reqs, list) { + if (!strcmp(kmod_req->name, module_name)) return kmod_req; - } - } + } return NULL; } @@ -86,58 +89,40 @@ static void kmod_dup_request_delete(struct work_struct *work) * kmod. The inneficies there are a call to modprobe and modprobe * just returning 0. */ - mutex_lock(&kmod_dup_mutex); - list_del_rcu(&kmod_req->list); - synchronize_rcu(); - mutex_unlock(&kmod_dup_mutex); - kfree(kmod_req); + scoped_guard(mutex, &kmod_dup_mutex) + list_del(&kmod_req->list); + + put_kmod_req(kmod_req); } -static void kmod_dup_request_complete(struct work_struct *work) +static struct kmod_dup_req *alloc_kmod_req(const char *module_name) { - struct kmod_dup_req *kmod_req; + struct kmod_dup_req *kmod_req = kzalloc_obj(*kmod_req); - kmod_req = container_of(work, struct kmod_dup_req, complete_work); - - /* - * This will ensure that the kernel will let all the waiters get - * informed its time to check the return value. It's time to - * go home. - */ - complete_all(&kmod_req->first_req_done); + if (!kmod_req) + return NULL; - /* - * Now that we have allowed prior request_module() calls to go on - * with life, let's schedule deleting this entry. We don't have - * to do it right away, but we *eventually* want to do it so to not - * let this linger forever as this is just a boot optimization for - * possible abuses of vmalloc() incurred by finit_module() thrashing. - */ - queue_delayed_work(system_dfl_wq, &kmod_req->delete_work, 60 * HZ); + refcount_set(&kmod_req->refcount, 1); + strscpy(kmod_req->name, module_name); + INIT_DELAYED_WORK(&kmod_req->delete_work, kmod_dup_request_delete); + init_completion(&kmod_req->first_req_done); + return kmod_req; } bool kmod_dup_request_exists_wait(char *module_name, bool wait, int *dup_ret) { - struct kmod_dup_req *kmod_req, *new_kmod_req; + struct kmod_dup_req *kmod_req __free(put_kmod_req) = NULL; int ret; - /* - * Pre-allocate the entry in case we have to use it later - * to avoid contention with the mutex. - */ - new_kmod_req = kzalloc_obj(*new_kmod_req); - if (!new_kmod_req) - return false; - - memcpy(new_kmod_req->name, module_name, strlen(module_name)); - INIT_WORK(&new_kmod_req->complete_work, kmod_dup_request_complete); - INIT_DELAYED_WORK(&new_kmod_req->delete_work, kmod_dup_request_delete); - init_completion(&new_kmod_req->first_req_done); + scoped_guard(mutex, &kmod_dup_mutex) { + struct kmod_dup_req *new_kmod_req; - mutex_lock(&kmod_dup_mutex); + kmod_req = kmod_dup_request_lookup(module_name); + if (kmod_req) { + get_kmod_req(kmod_req); + break; + } - kmod_req = kmod_dup_request_lookup(module_name); - if (!kmod_req) { /* * If the first request that came through for a module * was with request_module_nowait() we cannot wait for it @@ -150,9 +135,7 @@ bool kmod_dup_request_exists_wait(char *module_name, bool wait, int *dup_ret) * would benefit from duplicate detection. */ if (!wait) { - kfree(new_kmod_req); pr_debug("New request_module_nowait() for %s -- cannot track duplicates for this request\n", module_name); - mutex_unlock(&kmod_dup_mutex); return false; } @@ -161,14 +144,14 @@ bool kmod_dup_request_exists_wait(char *module_name, bool wait, int *dup_ret) * keep tab on duplicates later. */ pr_debug("New request_module() for %s\n", module_name); - list_add_rcu(&new_kmod_req->list, &dup_kmod_reqs); - mutex_unlock(&kmod_dup_mutex); + new_kmod_req = alloc_kmod_req(module_name); + if (!new_kmod_req) + return false; + list_add(&new_kmod_req->list, &dup_kmod_reqs); return false; } - mutex_unlock(&kmod_dup_mutex); /* We are dealing with a duplicate request now */ - kfree(new_kmod_req); /* * To fix these try to use try_then_request_module() instead as that @@ -214,7 +197,6 @@ bool kmod_dup_request_exists_wait(char *module_name, bool wait, int *dup_ret) /* Now the duplicate request has the same exact return value as the first request */ *dup_ret = kmod_req->dup_ret; - return true; } @@ -222,26 +204,29 @@ void kmod_dup_request_announce(char *module_name, int ret) { struct kmod_dup_req *kmod_req; - mutex_lock(&kmod_dup_mutex); + /* + * Look for a kmod_dup_req previously added in + * kmod_dup_request_exists_wait(). Note that a request_module_nowait() + * without its own kmod_dup_req entry can announce a result of + * a concurrent request_module() call. + */ + scoped_guard(mutex, &kmod_dup_mutex) { + kmod_req = kmod_dup_request_lookup(module_name); + if (!kmod_req || completion_done(&kmod_req->first_req_done)) + return; - kmod_req = kmod_dup_request_lookup(module_name); - if (!kmod_req) - goto out; + kmod_req->dup_ret = ret; - kmod_req->dup_ret = ret; + /* Inform all duplicate waiters to check the return value. */ + complete_all(&kmod_req->first_req_done); + } /* - * If we complete() here we may allow duplicate threads - * to continue before the first one that submitted the - * request. We're in no rush also, given that each and - * every bounce back to userspace is slow we avoid that - * with a slight delay here. So queueue up the completion - * and let duplicates suffer, just wait a tad bit longer. - * There is no rush. But we also don't want to hold the - * caller up forever or introduce any boot delays. + * Now that we have allowed prior request_module() calls to go on + * with life, let's schedule deleting this entry. We don't have + * to do it right away, but we *eventually* want to do it so to not + * let this linger forever as this is just a boot optimization for + * possible abuses of vmalloc() incurred by finit_module() thrashing. */ - queue_work(system_dfl_wq, &kmod_req->complete_work); - -out: - mutex_unlock(&kmod_dup_mutex); + queue_delayed_work(system_dfl_wq, &kmod_req->delete_work, 60 * HZ); } diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c index 0fc11e4..f23126d 100644 --- a/kernel/module/kallsyms.c +++ b/kernel/module/kallsyms.c @@ -258,17 +258,25 @@ static const char *find_kallsyms_symbol(struct module *mod, unsigned int i, best = 0; unsigned long nextval, bestval; struct mod_kallsyms *kallsyms = rcu_dereference(mod->kallsyms); - struct module_memory *mod_mem; + struct module_memory *mod_mem = NULL; - /* At worse, next value is at end of module */ - if (within_module_init(addr, mod)) - mod_mem = &mod->mem[MOD_INIT_TEXT]; - else - mod_mem = &mod->mem[MOD_TEXT]; + for_each_mod_mem_type(type) { +#ifndef CONFIG_KALLSYMS_ALL + if (!mod_mem_type_is_text(type)) + continue; +#endif + if (within_module_mem_type(addr, mod, type)) { + mod_mem = &mod->mem[type]; + break; + } + } - nextval = (unsigned long)mod_mem->base + mod_mem->size; + if (!mod_mem) + return NULL; - bestval = kallsyms_symbol_value(&kallsyms->symtab[best]); + /* Initialize bounds within memory region the address belongs to. */ + nextval = (unsigned long)mod_mem->base + mod_mem->size; + bestval = (unsigned long)mod_mem->base - 1; /* * Scan for closest preceding symbol, and next symbol. (ELF diff --git a/kernel/module/main.c b/kernel/module/main.c index 46dd8d2..d0e1e0b 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -1458,7 +1458,6 @@ static void free_module(struct module *mod) /* This may be empty, but that's OK */ module_arch_freeing_init(mod); - kfree(mod->args); percpu_modfree(mod); free_mod_mem(mod); @@ -2011,6 +2010,7 @@ static int elf_validity_cache_sechdrs(struct load_info *info) * Specifically checks: * * * Section name table index is inbounds of section headers + * * Section name table type is SHT_STRTAB * * Section name table is not empty * * Section name table is NUL terminated * * All section name offsets are inbounds of the section @@ -2038,6 +2038,11 @@ static int elf_validity_cache_secstrings(struct load_info *info) strhdr = &info->sechdrs[info->hdr->e_shstrndx]; + if (strhdr->sh_type != SHT_STRTAB) { + pr_err("Invalid ELF section name table type: %u\n", strhdr->sh_type); + return -ENOEXEC; + } + /* * The section name table must be NUL-terminated, as required * by the spec. This makes strcmp and pr_* calls that access @@ -2204,7 +2209,7 @@ static int elf_validity_cache_index_sym(struct load_info *info) * Must have &load_info->index.sym populated. * * Looks at the symbol table's associated string table, makes sure it is - * in-bounds, and caches it. + * in-bounds and of type SHT_STRTAB, and caches it. * * Return: %0 if valid, %-ENOEXEC on failure. */ @@ -2218,6 +2223,12 @@ static int elf_validity_cache_index_str(struct load_info *info) return -ENOEXEC; } + if (info->sechdrs[str_idx].sh_type != SHT_STRTAB) { + pr_err("Invalid ELF symbol string table type: %u\n", + info->sechdrs[str_idx].sh_type); + return -ENOEXEC; + } + info->index.str = str_idx; return 0; } @@ -3425,7 +3436,7 @@ static int load_module(struct load_info *info, const char __user *uargs, struct module *mod; bool module_allocated = false; long err = 0; - char *after_dashes; + char *args = NULL, *after_dashes; /* * Do the signature check (if any) first. All that @@ -3523,9 +3534,9 @@ static int load_module(struct load_info *info, const char __user *uargs, flush_module_icache(mod); /* Now copy in args */ - mod->args = strndup_user(uargs, ~0UL >> 1); - if (IS_ERR(mod->args)) { - err = PTR_ERR(mod->args); + args = strndup_user(uargs, ~0UL >> 1); + if (IS_ERR(args)) { + err = PTR_ERR(args); goto free_arch_cleanup; } @@ -3546,7 +3557,7 @@ static int load_module(struct load_info *info, const char __user *uargs, mod->async_probe_requested = async_probe; /* Module is ready to execute: parsing args may do that. */ - after_dashes = parse_args(mod->name, mod->args, mod->kp, mod->num_kp, + after_dashes = parse_args(mod->name, args, mod->kp, mod->num_kp, -32768, 32767, mod, unknown_module_param_cb); if (IS_ERR(after_dashes)) { @@ -3556,6 +3567,8 @@ static int load_module(struct load_info *info, const char __user *uargs, pr_warn("%s: parameters '%s' after `--' ignored\n", mod->name, after_dashes); } + kfree(args); + args = NULL; /* Link in to sysfs. */ err = mod_sysfs_setup(mod, info, mod->kp, mod->num_kp); @@ -3597,7 +3610,7 @@ static int load_module(struct load_info *info, const char __user *uargs, ddebug_cleanup: ftrace_release_mod(mod); synchronize_rcu(); - kfree(mod->args); + kfree(args); free_arch_cleanup: module_arch_cleanup(mod); free_modinfo: diff --git a/kernel/module/procfs.c b/kernel/module/procfs.c index 0a4841e..90712aa 100644 --- a/kernel/module/procfs.c +++ b/kernel/module/procfs.c @@ -64,7 +64,7 @@ static void m_stop(struct seq_file *m, void *p) static unsigned int module_total_size(struct module *mod) { - int size = 0; + unsigned int size = 0; for_each_mod_mem_type(type) size += mod->mem[type].size; diff --git a/kernel/module/stats.c b/kernel/module/stats.c index 3a9672f..78b59bc 100644 --- a/kernel/module/stats.c +++ b/kernel/module/stats.c @@ -253,7 +253,7 @@ int try_add_failed_module(const char *name, enum fail_dup_mod_reason reason) mod_fail = kzalloc_obj(*mod_fail); if (!mod_fail) return -ENOMEM; - memcpy(mod_fail->name, name, strlen(name)); + strscpy(mod_fail->name, name); __set_bit(reason, &mod_fail->dup_fail_mask); atomic_long_inc(&mod_fail->count); list_add_rcu(&mod_fail->list, &dup_failed_modules); diff --git a/kernel/params.c b/kernel/params.c index a668863..a1ff4bf 100644 --- a/kernel/params.c +++ b/kernel/params.c @@ -261,6 +261,7 @@ EXPORT_SYMBOL_GPL(param_set_uint_minmax); int param_set_charp(const char *val, const struct kernel_param *kp) { + char *tmp; size_t len, maxlen = 1024; len = strnlen(val, maxlen + 1); @@ -269,19 +270,20 @@ int param_set_charp(const char *val, const struct kernel_param *kp) return -ENOSPC; } - maybe_kfree_parameter(*(char **)kp->arg); - /* * This is a hack. We can't kmalloc() in early boot, and we * don't need to; this mangled commandline is preserved. */ if (slab_is_available()) { - *(char **)kp->arg = kmalloc_parameter(len + 1); - if (!*(char **)kp->arg) + tmp = kmalloc_parameter(len + 1); + if (!tmp) return -ENOMEM; - strcpy(*(char **)kp->arg, val); + memcpy(tmp, val, len + 1); } else - *(const char **)kp->arg = val; + tmp = (char *)val; + + maybe_kfree_parameter(*(char **)kp->arg); + *(char **)kp->arg = tmp; return 0; } @@ -538,7 +540,7 @@ const struct kernel_param_ops param_ops_string = { }; EXPORT_SYMBOL(param_ops_string); -/* sysfs output in /sys/modules/XYZ/parameters/ */ +/* sysfs output in /sys/module/XYZ/parameters/ */ #define to_module_attr(n) container_of_const(n, struct module_attribute, attr) #define to_module_kobject(n) container_of(n, struct module_kobject, kobj) diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs index 6541af2..f9a1476 100644 --- a/rust/kernel/module_param.rs +++ b/rust/kernel/module_param.rs @@ -5,7 +5,7 @@ //! C header: [`include/linux/moduleparam.h`](srctree/include/linux/moduleparam.h) use crate::prelude::*; -use crate::str::BStr; +use crate::str::{kstrtobool_bytes, BStr}; use bindings; use kernel::sync::SetOnce; @@ -105,6 +105,12 @@ impl_int_module_param!(u64); impl_int_module_param!(isize); impl_int_module_param!(usize); +impl ModuleParam for bool { + fn try_from_param_arg(arg: &BStr) -> Result { + kstrtobool_bytes(arg) + } +} + /// A wrapper for kernel parameters. /// /// This type is instantiated by the [`module!`] macro when module parameters are @@ -130,10 +136,26 @@ impl ModuleParamAccess { } } + /// Get a copy of the parameter value. + /// + /// Returns the value supplied at module load time, or the default value + /// if the parameter has not been set. + #[inline] + pub fn value(&self) -> T + where + T: Copy, + { + self.value.copy().unwrap_or(self.default) + } + /// Get a shared reference to the parameter value. + /// + /// Returns a reference to the value supplied at module load time, or a + /// reference to the default value if the parameter has not been set. // Note: When sysfs access to parameters are enabled, we have to pass in a // held lock guard here. - pub fn value(&self) -> &T { + #[inline] + pub fn value_ref(&self) -> &T { self.value.as_ref().unwrap_or(&self.default) } @@ -179,3 +201,4 @@ make_param_ops!(PARAM_OPS_I64, i64); make_param_ops!(PARAM_OPS_U64, u64); make_param_ops!(PARAM_OPS_ISIZE, isize); make_param_ops!(PARAM_OPS_USIZE, usize); +make_param_ops!(PARAM_OPS_BOOL, bool); diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs index 37a6643..24f96fe 100644 --- a/rust/macros/lib.rs +++ b/rust/macros/lib.rs @@ -56,6 +56,7 @@ use syn::parse_macro_input; /// - [`u64`] /// - [`isize`] /// - [`usize`] +/// - [`bool`] /// /// C header: [`include/linux/moduleparam.h`](srctree/include/linux/moduleparam.h) /// diff --git a/rust/macros/module.rs b/rust/macros/module.rs index bd69d8d..bc7027f 100644 --- a/rust/macros/module.rs +++ b/rust/macros/module.rs @@ -192,6 +192,7 @@ fn param_ops_path(param_type: &str) -> Path { "u64" => parse_quote!(::kernel::module_param::PARAM_OPS_U64), "isize" => parse_quote!(::kernel::module_param::PARAM_OPS_ISIZE), "usize" => parse_quote!(::kernel::module_param::PARAM_OPS_USIZE), + "bool" => parse_quote!(::kernel::module_param::PARAM_OPS_BOOL), t => panic!("Unsupported parameter type {}", t), } } diff --git a/samples/rust/rust_minimal.rs b/samples/rust/rust_minimal.rs index 8eb9583..8c910d3 100644 --- a/samples/rust/rust_minimal.rs +++ b/samples/rust/rust_minimal.rs @@ -15,6 +15,10 @@ module! { default: 1, description: "This parameter has a default of 1", }, + test_bool_parameter: bool { + default: false, + description: "This boolean parameter defaults to false", + }, }, } @@ -28,7 +32,11 @@ impl kernel::Module for RustMinimal { pr_info!("Am I built-in? {}\n", !cfg!(MODULE)); pr_info!( "test_parameter: {}\n", - *module_parameters::test_parameter.value() + module_parameters::test_parameter.value() + ); + pr_info!( + "test_bool_parameter: {}\n", + module_parameters::test_bool_parameter.value() ); let mut numbers = KVec::new(); diff --git a/scripts/module.lds.S b/scripts/module.lds.S index b626830..d0f2004 100644 --- a/scripts/module.lds.S +++ b/scripts/module.lds.S @@ -3,11 +3,6 @@ * Archs are free to supply their own linker scripts. ld will * combine them automatically. */ -#ifdef CONFIG_UNWIND_TABLES -#define DISCARD_EH_FRAME -#else -#define DISCARD_EH_FRAME *(.eh_frame) -#endif #include