Messages in this thread | | | From | Rasmus Villemoes <> | Subject | Re: [PATCH] init, fix initcall blacklist for modules | Date | Mon, 13 Jun 2016 22:59:07 +0200 |
| |
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
| |