Messages in this thread | | | Date | Tue, 14 Jun 2016 05:59:46 -0400 | From | Prarit Bhargava <> | Subject | Re: [PATCH] init, fix initcall blacklist for modules |
| |
On 06/13/2016 04:59 PM, Rasmus Villemoes wrote: > 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?
Oops. I can see how you read that that way. No, this isn't caused by or "Fixes:" c8cdd2be21. At some point "%pF" changed its behavior and blacklisting module_init() functions stopped working.
P.
> >> 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 >
| |