Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751212AbdFAXXI (ORCPT ); Thu, 1 Jun 2017 19:23:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33046 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751116AbdFAXXH (ORCPT ); Thu, 1 Jun 2017 19:23:07 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com A5B3FC049D59 Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=jeyu@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com A5B3FC049D59 Date: Thu, 1 Jun 2017 16:23:05 -0700 From: Jessica Yu To: Wanlong Gao Cc: Xie XiuQi , akpm@linux-foundation.org, linux-kernel@vger.kernel.org, rusty@rustcorp.com.au, john.wanghui@huawei.com, wencongyang2@huawei.com, guijianfeng@huawei.com Subject: Re: [PATCH] modpost: abort if a module name is too long Message-ID: <20170601232303.qtc6eldvl6xh2ln6@jeyu> References: <1495266381-14755-1-git-send-email-xiexiuqi@huawei.com> <20170529091007.e3zhsasmcxhzexx6@jeyu> <20170531033042.mtrmcj6odvn2bmxs@jeyu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: X-OS: Linux jeyu 4.12.0-rc2+ x86_64 User-Agent: NeoMutt/20161126 (1.7.1) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Thu, 01 Jun 2017 23:23:06 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4494 Lines: 131 +++ Wanlong Gao [31/05/17 11:48 +0800]: > > >On 2017/5/31 11:30, Jessica Yu wrote: >> +++ Wanlong Gao [31/05/17 10:23 +0800]: >>> Hi Jessica, >>> >>> On 2017/5/29 17:10, Jessica Yu wrote: >>>> +++ Xie XiuQi [20/05/17 15:46 +0800]: >>>>> From: Wanlong Gao >>>>> >>>>> Module name has a limited length, but currently the build system >>>>> allows the build finishing even if the module name is too long. >>>>> >>>>> CC /root/kprobe_example/abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz.mod.o >>>>> /root/kprobe_example/abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz.mod.c:9:2: >>>>> warning: initializer-string for array of chars is too long [enabled by default] >>>>> .name = KBUILD_MODNAME, >>>>> ^ >>>>> >>>>> but it's merely a warning. >>>>> >>>>> This patch adds the check of the module name length in modpost and stops >>>>> the build properly. >>>>> >>>>> Signed-off-by: Wanlong Gao >>>>> Signed-off-by: Xie XiuQi >>>>> --- >>>>> scripts/mod/modpost.c | 11 +++++++++++ >>>>> 1 file changed, 11 insertions(+) >>>>> >>>>> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c >>>>> index 30d752a..db11c57 100644 >>>>> --- a/scripts/mod/modpost.c >>>>> +++ b/scripts/mod/modpost.c >>>>> @@ -2166,6 +2166,17 @@ static int add_versions(struct buffer *b, struct module *mod) >>>>> { >>>>> struct symbol *s, *exp; >>>>> int err = 0; >>>>> + const char *mod_name; >>>>> + >>>>> + mod_name = strrchr(mod->name, '/'); >>>>> + if (mod_name == NULL) >>>>> + mod_name = mod->name; >>>>> + else >>>>> + mod_name++; >>>>> + if (strlen(mod_name) >= MODULE_NAME_LEN) { >>>>> + merror("module name is too long [%s.ko]\n", mod->name); >>>>> + return 1; >>>>> + } >>>> >>>> Hi Xie, >>>> >>>> This check shouldn't be in add_versions() (which does something else entirely), >>>> it should probably be put in a separate helper function called from main. But >>>> I'm not a big fan of the extra string manipulation to do something this simple. >>>> >>>> I think this check can be vastly simplified, how about something like the >>>> following? >>> >>> This looks better, would you apply your following patch? >>> >>> Reviewed-by: Wanlong Gao >>> Tested-by: Wanlong Gao >> >> Sure, thanks for testing. I'll go ahead and format this into a proper >> patch and resend. > >Please wait, I just found that this patch makes the built module can't >be inserted by the following error: > ># insmod abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabc.ko >insmod: ERROR: could not insert module abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabc.ko: Invalid parameters > ># dmesg >abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabc: Unknown symbol __fentry__ (err -22) Hm, I am unable to reproduce this. It looks like __fentry__ is missing from your kernel, you may have a mismatch between the kernel config that you're running and the config you are using to build the module. In other words, it seems like you might have built the module with CONFIG_FTRACE but built the kernel without. Few questions - What is the output of running `grep __fentry__ /proc/kallsyms`? Does your module correspond to the running kernel version? Do you have CONFIG_FTRACE/FUNCTION_TRACER enabled in your running kernel? Is that the full dmesg output (are there any other error messages)? Thanks, Jessica >>>> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c >>>> index 48397fe..bb09fc7 100644 >>>> --- a/scripts/mod/modpost.c >>>> +++ b/scripts/mod/modpost.c >>>> @@ -2139,6 +2139,9 @@ static void add_header(struct buffer *b, struct module *mod) >>>> "#endif\n"); >>>> buf_printf(b, "\t.arch = MODULE_ARCH_INIT,\n"); >>>> buf_printf(b, "};\n"); >>>> + buf_printf(b, "\n"); >>>> + buf_printf(b, "static void __attribute__((section(\".discard\"), used)) __modname_test(void)\n"); >>>> + buf_printf(b, "{ BUILD_BUG_ON(sizeof(KBUILD_MODNAME) > MODULE_NAME_LEN); }\n"); >>>> } >>>> >>>> static void add_intree_flag(struct buffer *b, int is_intree) >>>> >>>> This simply checks if KBUILD_MODNAME > MODULE_NAME_LEN and breaks the build if >>>> it does. >>>> >>>> Jessica >>>> >>>>> for (s = mod->unres; s; s = s->next) { >>>>> exp = find_symbol(s->name); >>>>> -- >>>>> 1.8.3.1 >>>>> >>>> >>>> . >>>> >>> >> >> . >> >