Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752694AbbDNFaH (ORCPT ); Tue, 14 Apr 2015 01:30:07 -0400 Received: from blu004-omc4s8.hotmail.com ([65.55.111.147]:60583 "EHLO BLU004-OMC4S8.hotmail.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751369AbbDNF34 (ORCPT ); Tue, 14 Apr 2015 01:29:56 -0400 X-TMN: [h6UI56hh7f/COKtgYQnrJyhdjyCRk3a0] X-Originating-Email: [minfei.huang@hotmail.com] Message-ID: Date: Tue, 14 Apr 2015 13:29:50 +0800 From: Minfei Huang To: Josh Poimboeuf CC: 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 References: <1428844554-4015-1-git-send-email-minfei.huang@hotmail.com> <20150413231305.GD4412@treble.hsd1.ky.comcast.net> <20150414045739.GG4412@treble.hsd1.ky.comcast.net> <20150414051113.GH4412@treble.hsd1.ky.comcast.net> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20150414051113.GH4412@treble.hsd1.ky.comcast.net> User-Agent: Mutt/1.5.23 (2014-03-12) X-OriginalArrivalTime: 14 Apr 2015 05:29:54.0222 (UTC) FILETIME=[098C54E0:01D07674] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5305 Lines: 117 On 04/14/15 at 12:11P, Josh Poimboeuf wrote: > On Tue, Apr 14, 2015 at 01:03:48PM +0800, Minfei Huang wrote: > > On 04/13/15 at 11:57P, Josh Poimboeuf wrote: > > > On Tue, Apr 14, 2015 at 08:26:29AM +0800, Minfei Huang wrote: > > > > On 04/13/15 at 06:13P, Josh Poimboeuf wrote: > > > > > On Sun, Apr 12, 2015 at 09:15:54PM +0800, Minfei Huang wrote: > > > > > > For now, the kallsyms will only store the first (KSYM_NAME_LEN-1). The > > > > > > kallsyms name is same for the function which first (KSYM_NAME_LEN-1) is > > > > > > same, but the rest is not. > > > > > > > > > > > > Then function will never be patched, although function name and address > > > > > > are provided both. The reason caused this bug is livepatch cannt > > > > > > recognize the function name. > > > > > > > > > > > > Now, livepatch will verify the function name with first (KSYM_NAME_LEN-1) > > > > > > and address, if provided. Once they are matched, we can confirm that the > > > > > > patched function is found. > > > > > > > > > > From scripts/kallsyms.c: > > > > > > > > > > if (strlen(str) > KSYM_NAME_LEN) { > > > > > fprintf(stderr, "Symbol %s too long for kallsyms (%zu vs %d).\n" > > > > > "Please increase KSYM_NAME_LEN both in kernel and kallsyms.c\n", > > > > > str, strlen(str), KSYM_NAME_LEN); > > > > > return -1; > > > > > } > > > > > > > > > > So I think such a long symbol name wouldn't be added to the kallsyms > > > > > database in the first place. > > > > > > > > > > > > > Actually, kernel allows overlength function name to be used. Following > > > > is my testing module. > > > > > > > > We can got the address in /proc/kallsyms. > > > > $ cat /proc/kallsyms | grep sysfs_print > > > > ffffffffa0000000 t sys_print_print_print_print_print_print_print_print_print_print_print_print_print_print_print_print_print_print_print_print_pri [sysfs_print] > > > > ffffffffa0000010 t kobj_release [sysfs_print] > > > > ffffffffa0000020 t sys_print_print_print_print_print_print_print_print_print_print_print_print_print_print_print_print_print_print_print_print_pri [sysfs_print] > > > > ffffffffa00004e0 b root_kobj [sysfs_print] > > > > ffffffffa0000200 d print_ktype [sysfs_print] > > > > ffffffffa00004a0 b print_kobj [sysfs_print] > > > > ffffffffa000004c t sys_print_exit [sysfs_print] > > > > ffffffffa0000144 r __func__.14514 [sysfs_print] > > > > ffffffffa0000230 d kobj_attrs [sysfs_print] > > > > ffffffffa0000240 d sys_print_kobj_attr [sysfs_print] > > > > ffffffffa0000260 d __this_module [sysfs_print] > > > > ffffffffa000004c t cleanup_module [sysfs_print] > > > > > > > > Code: > > > > > > > > static ssize_t sys_print_print_print_print_print_print_print_print_print_print_print_print_print_print_print_print_print_print_print_print_print_store(struct kobject *kobj, s > > > > const char *buf, size_t count) > > > > { > > > > return count; > > > > } > > > > > > > > static ssize_t sys_print_print_print_print_print_print_print_print_print_print_print_print_print_print_print_print_print_print_print_print_print_show(struct kobject *kobj, > > > > struct kobj_attribute *attr, char *buf) > > > > { > > > > return snprintf(buf, PAGE_SIZE-1, "%s\n", "This is printed by module"); > > > > } > > > > > > > > static struct kobj_attribute sys_print_kobj_attr = __ATTR_RW(sys_print_print_print_print_print_print_print_print_print_print_print_print_print_print_print_print_print_print_p > > > > static struct attribute *kobj_attrs[] = { > > > > &sys_print_kobj_attr.attr, > > > > NULL > > > > }; > > > > > > > > > > Hm, this seems like a kallsyms bug. IMO it should either fail the build > > > or omit the symbol from the kallsyms db. Truncating it seems dangerous > > > and counterintuitive. > > > > > > > Kallsyms will record all of the function name, without truncating it. > > But the kallsyms will return the truncated function name which is max to > > 127. > > > > > But regardless I really don't see a good reason to encourage this kind > > > of insanity in the livepatch code. > > > > > > > Yes, the above code is terrible, but we cannt stop user composing like > > that. > > > > Once the function name is like above, user will never have chance to use > > livepatch. > > Again, this seems like a kallsyms bug. Fix the bug and the real world > need for this patch set goes away. The user will be forced to either > shorten their function name or increase KSYM_NAME_LEN. > kallsyms bug? I donot think increasing the KSYM_NAME_LEN is a good idea. 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? It is significant that livepatch supports to running specified kernel and extra module, not only for deliverying kernel. Thanks Minfei > -- > Josh > -- > To unsubscribe from this list: send the line "unsubscribe live-patching" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- 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/