Messages in this thread | | | Date | Tue, 2 Jun 2015 11:15:47 +0200 (CEST) | From | Miroslav Benes <> | Subject | Re: [PATCH] livepatch: add module locking around kallsyms calls |
| |
On Tue, 2 Jun 2015, Minfei Huang wrote:
> On Mon, Jun 1, 2015 at 11:48 PM, Miroslav Benes <mbenes@suse.cz> wrote: > > The list of loaded modules is walked through in > > module_kallsyms_on_each_symbol (called by kallsyms_on_each_symbol). The > > module_mutex lock should be acquired to prevent potential corruptions > > in the list. > > > > This was uncovered with new lockdep asserts in module code introduced by > > the commit 0be964be0d45 ("module: Sanitize RCU usage and locking") in > > recent next- trees. > > > > Signed-off-by: Miroslav Benes <mbenes@suse.cz> > > --- > > kernel/livepatch/core.c | 18 +++++++++++++----- > > 1 file changed, 13 insertions(+), 5 deletions(-) > > > > diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c > > index e6c8d54..c40ebcc 100644 > > --- a/kernel/livepatch/core.c > > +++ b/kernel/livepatch/core.c > > @@ -179,7 +179,9 @@ static int klp_find_object_symbol(const char *objname, const char *name, > > .count = 0 > > }; > > > > + mutex_lock(&module_mutex); > > kallsyms_on_each_symbol(klp_find_callback, &args); > > + mutex_unlock(&module_mutex); > > > > if (args.count == 0) > > pr_err("symbol '%s' not found in symbol table\n", name); > > @@ -219,13 +221,19 @@ static int klp_verify_vmlinux_symbol(const char *name, unsigned long addr) > > .name = name, > > .addr = addr, > > }; > > + int ret; > > > > - if (kallsyms_on_each_symbol(klp_verify_callback, &args)) > > - return 0; > > + mutex_lock(&module_mutex); > > + ret = kallsyms_on_each_symbol(klp_verify_callback, &args); > > + mutex_unlock(&module_mutex); > > > > Hi. > In livepatch code path, returning value 0 may represent the right, but > sometime represent wrong, like the above function. > > Is it possible that we can wrap such function and return the unified > value? Thus we can not confuse the returning value any more.
Hi,
I must admit I do not understand. Both klp_find_object_symbol and klp_verify_vmlinux_symbol return 0 on success or -EINVAL. It is true that kallsyms_on_each_symbol and module_kallsyms_on_each symbol are different. That is why our kallsyms callbacks are different. See the implementation of those. But that is the API. Is this what you are worried about?
> Otherwise annotation is appreciate.
Thanks, Miroslav
| |