Messages in this thread | | | Date | Tue, 28 Jul 2015 14:52:33 +0200 | From | Peter Zijlstra <> | Subject | Re: [PATCH] module: Fix missing to hold module_mutex lock in module_kallsyms_lookup_name |
| |
On Tue, Jul 28, 2015 at 10:01:35AM +0930, Rusty Russell wrote: > Peter?
> module: weaken locking assertion for oops path. > > We don't actually hold the module_mutex when calling find_module_all > from module_kallsyms_lookup_name: that's because it's used by the oops > code and we don't want to deadlock. > > However, access to the list read-only is safe if preempt is disabled, > so we can weaken the assertion. Keep a strong version for external > callers though. > > Fixes: 0be964be0d45 ("module: Sanitize RCU usage and locking") > Reported-by: He Kuang <hekuang@huawei.com> > Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> > > diff --git a/kernel/module.c b/kernel/module.c > index 4d2b82e610e2..b86b7bf1be38 100644 > --- a/kernel/module.c > +++ b/kernel/module.c > @@ -602,13 +602,16 @@ const struct kernel_symbol *find_symbol(const char *name, > } > EXPORT_SYMBOL_GPL(find_symbol); > > -/* Search for module by name: must hold module_mutex. */ > +/* > + * Search for module by name: must hold module_mutex (or preempt disabled > + * for read-only access). > + */ > static struct module *find_module_all(const char *name, size_t len, > bool even_unformed) > { > struct module *mod; > > - module_assert_mutex(); > + module_assert_mutex_or_preempt();
Yeah, that should be fine indeed, I went by that comment you just expanded.
The operation itself does indeed not modify data at all, so the preempt_disable is perfectly adequate.
Thanks!
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
| |