Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755958AbbDOCNg (ORCPT ); Tue, 14 Apr 2015 22:13:36 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53574 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755572AbbDOCN2 (ORCPT ); Tue, 14 Apr 2015 22:13:28 -0400 Date: Wed, 15 Apr 2015 10:15:58 +0800 From: Minfei Huang To: Petr Mladek Cc: Minfei Huang , Josh Poimboeuf , sjenning@redhat.com, jkosina@suse.cz, vojtech@suse.cz, live-patching@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 2/2] livepatch: Fix the bug if the function name is larger than KSYM_NAME_LEN-1 Message-ID: <20150415021558.GA21518@dhcp-128-1.nay.redhat.com> References: <20150414051113.GH4412@treble.hsd1.ky.comcast.net> <20150414053247.GI4412@treble.hsd1.ky.comcast.net> <20150414151110.GL4412@treble.hsd1.ky.comcast.net> <20150414162718.GC21044@dhcp128.suse.cz> <20150414170139.GA30740@dhcp-128-1.nay.redhat.com> <20150414184121.GD21044@dhcp128.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150414184121.GD21044@dhcp128.suse.cz> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5032 Lines: 120 On 04/14/15 at 08:41pm, Petr Mladek wrote: > On Wed 2015-04-15 01:01:39, Minfei Huang wrote: > > On 04/14/15 at 06:27pm, Petr Mladek wrote: > > > On Tue 2015-04-14 23:55:36, Minfei Huang wrote: > > > > On 04/14/15 at 10:11P, Josh Poimboeuf wrote: > > > > > On Tue, Apr 14, 2015 at 01:45:49PM +0800, Minfei Huang wrote: > > > > > > On 04/14/15 at 12:32P, Josh Poimboeuf wrote: > > > > > > > On Tue, Apr 14, 2015 at 01:29:50PM +0800, Minfei Huang wrote: > > > > > > > > > > > > > > > > For end user, they may know litter about restriction of kallsyms and > > > > > > > > livepatch. How can they know the restriction that function name is > > > > > > > > limited to 127? > > > > > > > > > > > > > > As I mentioned above, I think kallsyms.c should fail the build if it > > > > > > > encounters a symbol longer than KSYM_NAME_LEN. > > > > > > > > > > > > > > > > > > > I dont think it is a good idea to handle this case like that. The > > > > > > function name is only for human recognization. Why the compiler fails > > > > > > to build it? > > > > > > > > > > Well, the function name isn't only for human recognition. kpatch-build > > > > > generates patch modules automatically. It assumes that the compiled > > > > > function name matches the kallsyms name. And I'd guess that a lot of > > > > > other code (both in-kernel and user space tools) make the same > > > > > assumption. > > > > > > > > > > Not to mention that most humans would also make the same assumption... > > > > > > > > Yes. The assumption is correct for most case. > > > > > > > > It is significance for livepatch to support extra module, because in my > > > > opinion kernel is more stable than the third module. > > > > > > > > So it is more important, if the livepatch can patch all sorts of patch. > > > > For dynamic function name, I think it is simple to avoid it. > > > > > > Do you have some really existing module with such a crazy long > > > function names or is this debate pure theoretical, please? > > > > > > > No, I do not have such running module which function name is exceed to > > 127. > > > > Again, we can not predict what end user do to name the function name. I > > think the overlength function name is valid for linux kernel, if the > > module can be installed. > > My position on this is that using >127 length function names is > insane. I would be scared to use such a module on a production system. > If we refuse patching, we actually do a favor for the user. > Instead of fixing live patch for such a scenario, we should suggest > the user to use more trustful modules. Yes, the function name can be changed, before the extra module is installed to the production system. We discuss around and around, there are still some confusion with it. 1) How does end user know that livepatch can _not_ support the function which length is larger than 127. We can not enforce the end user to know the livepatch and kallsyms code in detail. 2) How does end user use livepatch to patch running extra module, once the module is running in the production system, if the function name is insane. 3) The error message is ambiguity, if we try to patch the overlength function. We can give the error message clearly, once the function name is overlength. I think it is better that we can take more time on the people who will use livepatch frequently. Attaching a patch to make error message explictly for the overlength function name. >From d46a230499303657a914d6939c3afbeff906796c Mon Sep 17 00:00:00 2001 From: Minfei Huang Date: Wed, 15 Apr 2015 10:02:43 +0800 Subject: [PATCH] livepatch: Make error message explicitly for the overlength function name For not, livepatch do not support the function which name is larger than KSYM_NAME_LEN-1. It may be confusion user with error message "livepatch: symbol 'xxx(function name)' not found in symbol table". Make error message explicitly for overlength issue. Signed-off-by: Minfei Huang --- kernel/livepatch/core.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c index 3f9f1d6..d1f2404 100644 --- a/kernel/livepatch/core.c +++ b/kernel/livepatch/core.c @@ -789,6 +789,12 @@ static int klp_init_object(struct klp_patch *patch, struct klp_object *obj) return -ENOMEM; for (func = obj->funcs; func->old_name; func++) { + if (strlen(func->old_name) > (KSYM_NAME_LEN-1)) { + pr_err("%s is overlength, the max to be supported is %d\n", + func->old_name, KSYM_NAME_LEN-1); + ret = -EINVAL; + goto free; + } ret = klp_init_func(obj, func); if (ret) goto free; -- 2.2.2 > > Best Regards, > Petr -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/