lkml.org 
[lkml]   [2016]   [Jun]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] init, fix initcall blacklist for modules
Date
On Mon, Jun 13 2016, Prarit Bhargava <prarit@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 <prarit@redhat.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Yang Shi <yang.shi@linaro.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Mel Gorman <mgorman@suse.de>
> Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Yaowei Bai <baiyaowei@cmss.chinamobile.com>
> Cc: Andrey Ryabinin <aryabinin@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

\
 
 \ /
  Last update: 2016-06-13 23:41    [W:0.071 / U:0.176 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site