On Mon, Jun 13 2016, Prarit Bhargava <pra...@redhat.com> wrote:
> Sorry ... forgot to cc everyone on the last email.
>
> P.
>
> ----8<----
>
> sprint_symbol_no_offset() returns the string "function_name [module_name]"
> where [module_name] is not printed for built in kernel functions. This
> means that the initcall blacklisting code will now always fail when
I was and am pretty sure that %pf ends up using
sprint_symbol_no_offset(), so I don't see how this is new. But maybe
"now" doesn't refer to c8cdd2be21?
> comparing module_init() function names. This patch resolves the issue by
> comparing to the length of the function_name.
>
> Signed-off-by: Prarit Bhargava <pra...@redhat.com>
> Cc: Andrew Morton <a...@linux-foundation.org>
> Cc: Thomas Gleixner <t...@linutronix.de>
> Cc: Yang Shi <yang....@linaro.org>
> Cc: Ingo Molnar <mi...@kernel.org>
> Cc: Mel Gorman <mgor...@suse.de>
> Cc: Rasmus Villemoes <li...@rasmusvillemoes.dk>
> Cc: Kees Cook <keesc...@chromium.org>
> Cc: Yaowei Bai <baiyao...@cmss.chinamobile.com>
> Cc: Andrey Ryabinin <aryabi...@virtuozzo.com>
> ---
> init/main.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/init/main.c b/init/main.c
> index 4c17fda5c2ff..09a795e91efe 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -708,14 +708,26 @@ static bool __init_or_module
> initcall_blacklisted(initcall_t fn)
> {
> struct blacklist_entry *entry;
> char fn_name[KSYM_SYMBOL_LEN];
> + char *space;
> + int length;
>
> if (list_empty(&blacklisted_initcalls))
> return false;
>
> sprint_symbol_no_offset(fn_name, (unsigned long)fn);
> + /*
> + * fn will be "function_name [module_name]" where [module_name] is not
> + * displayed for built-in initcall functions. Strip off the
> + * [module_name].
> + */
> + space = strchrnul(fn_name, ' ');
> + if (!space)
> + length = strlen(fn_name);
> + else
> + length = space - fn_name;
strchrnul never returns NULL, so this could just be 'length =
strchrnul(fn_name, ' ') - fn_name;'. But I don't think that's what you
want anyway: Suppose one has blacklisted "init_foobar", and the function
pointer resolves to a completely unrelated "init_foo", we'll end up
falsely also blacklisting that since we're just comparing prefixes.
May I suggest
strreplace(fn_name, ' ', '\0');
which also seems to match the comment a little better (and eliminates
the extra variables and the hunk below).
> list_for_each_entry(entry, &blacklisted_initcalls, next) {
> - if (!strcmp(fn_name, entry->buf)) {
> + if (!strncmp(fn_name, entry->buf, length)) {
> pr_debug("initcall %s blacklisted\n", fn_name);
> return true;
> }
Rasmus
