From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751926AbcFNJ7v (ORCPT ); Tue, 14 Jun 2016 05:59:51 -0400 Received: from mx1.redhat.com ([209.132.183.28]:52068 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751618AbcFNJ7t (ORCPT ); Tue, 14 Jun 2016 05:59:49 -0400 Message-ID: <575FD592.2020303@redhat.com> Date: Tue, 14 Jun 2016 05:59:46 -0400 From: Prarit Bhargava User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Rasmus Villemoes 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 References: <1465820959-13808-1-git-send-email-prarit@redhat.com> <87bn34ls9w.fsf@rasmusvillemoes.dk> In-Reply-To: <87bn34ls9w.fsf@rasmusvillemoes.dk> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Tue, 14 Jun 2016 09:59:48 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06/13/2016 04:59 PM, Rasmus Villemoes wrote: > 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? 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 >> 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 >