Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752173AbbDNFLY (ORCPT ); Tue, 14 Apr 2015 01:11:24 -0400 Received: from mx1.redhat.com ([209.132.183.28]:36510 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751745AbbDNFLR (ORCPT ); Tue, 14 Apr 2015 01:11:17 -0400 Date: Tue, 14 Apr 2015 00:11:13 -0500 From: Josh Poimboeuf To: Minfei Huang 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 Message-ID: <20150414051113.GH4412@treble.hsd1.ky.comcast.net> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23.1-rc1 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4502 Lines: 99 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. -- Josh -- 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/