From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1424245AbcFMU7R (ORCPT ); Mon, 13 Jun 2016 16:59:17 -0400 Received: from mail-lf0-f52.google.com ([209.85.215.52]:33842 "EHLO mail-lf0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1422719AbcFMU7L (ORCPT ); Mon, 13 Jun 2016 16:59:11 -0400 From: Rasmus Villemoes To: Prarit Bhargava Cc: linux-kernel@vger.kernel.org, Andrew Morton , Thomas Gleixner , Yang Shi , Ingo Molnar , Mel Gorman , Kees Cook , Yaowei Bai , Andrey Ryabinin Subject: Re: [PATCH] init, fix initcall blacklist for modules Organization: D03 References: <1465820959-13808-1-git-send-email-prarit@redhat.com> X-Hashcash: 1:20:160613:yang.shi@linaro.org::5Nid/dOybbQOdZuT:0000000000000000000000000000000000000000000NGJ X-Hashcash: 1:20:160613:baiyaowei@cmss.chinamobile.com::F7LXlCmwujBn/Ntm:00000000000000000000000000000000NHZ X-Hashcash: 1:20:160613:linux-kernel@vger.kernel.org::i6OiHvsNRSfTmotf:0000000000000000000000000000000001Obx X-Hashcash: 1:20:160613:tglx@linutronix.de::yGs3UQtGg1sftPD1:00000000000000000000000000000000000000000001VH1 X-Hashcash: 1:20:160613:keescook@chromium.org::6dyfJVvIyb7iZwWk:00000000000000000000000000000000000000001X19 X-Hashcash: 1:20:160613:aryabinin@virtuozzo.com::FaYPRPygOOvIom5o:000000000000000000000000000000000000002JE5 X-Hashcash: 1:20:160613:akpm@linux-foundation.org::giXe5vpWa1ECNNDt:0000000000000000000000000000000000003POZ X-Hashcash: 1:20:160613:prarit@redhat.com::6qwBf2MsKhPCFv/M:000000000000000000000000000000000000000000005bku X-Hashcash: 1:20:160613:mingo@kernel.org::oyk+as3njMY5W1Ev:08Tou X-Hashcash: 1:20:160613:mgorman@suse.de::gqPLIX73TIjKvsuJ:00HAdI Date: Mon, 13 Jun 2016 22:59:07 +0200 In-Reply-To: <1465820959-13808-1-git-send-email-prarit@redhat.com> (Prarit Bhargava's message of "Mon, 13 Jun 2016 08:29:19 -0400") Message-ID: <87bn34ls9w.fsf@rasmusvillemoes.dk> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jun 13 2016, Prarit Bhargava 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 > Cc: Andrew Morton > Cc: Thomas Gleixner > Cc: Yang Shi > Cc: Ingo Molnar > Cc: Mel Gorman > Cc: Rasmus Villemoes > Cc: Kees Cook > Cc: Yaowei Bai > Cc: Andrey Ryabinin > --- > 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