Re: [PATCH 05/13] kallsyms: refactor {,module_}kallsyms_on_each_symbol

[Petr Mladek <pmladek@suse.com>]

On Thu 2021-01-28 19:14:13, Christoph Hellwig wrote:
> Require an explicit call to module_kallsyms_on_each_symbol to look
> for symbols in modules instead of the call from kallsyms_on_each_symbol,
> and acquire module_mutex inside of module_kallsyms_on_each_symbol instead
> of leaving that up to the caller.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  kernel/kallsyms.c       | 6 +++++-
>  kernel/livepatch/core.c | 6 +-----
>  kernel/module.c         | 8 ++++----
>  3 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
> index fe9de067771c34..a0d3f0865916f9 100644
> --- a/kernel/kallsyms.c
> +++ b/kernel/kallsyms.c
> @@ -177,6 +177,10 @@ unsigned long kallsyms_lookup_name(const char *name)
>  	return module_kallsyms_lookup_name(name);
>  }
>  
> +/*
> + * Iterate over all symbols in vmlinux.  For symbols from modules use
> + * module_kallsyms_on_each_symbol instead.
> + */
>  int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
>  				      unsigned long),
>  			    void *data)
> @@ -192,7 +196,7 @@ int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
>  		if (ret != 0)
>  			return ret;
>  	}
> -	return module_kallsyms_on_each_symbol(fn, data);
> +	return 0;
>  }
>  
>  static unsigned long get_symbol_pos(unsigned long addr,
> diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
> index 262cd9b003b9f0..f591dac5e86ef4 100644
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -164,12 +164,8 @@ static int klp_find_object_symbol(const char *objname, const char *name,
>  		.pos = sympos,
>  	};
>  
> -	mutex_lock(&module_mutex);
> -	if (objname)
> +	if (objname || !kallsyms_on_each_symbol(klp_find_callback, &args))
>  		module_kallsyms_on_each_symbol(klp_find_callback, &args);
> -	else
> -		kallsyms_on_each_symbol(klp_find_callback, &args);
> -	mutex_unlock(&module_mutex);

This change is not needed. (objname == NULL) means that we are
interested only in symbols in "vmlinux".

module_kallsyms_on_each_symbol(klp_find_callback, &args)
will always fail when objname == NULL.

Best Regards,
Petr