Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751886AbbDNE54 (ORCPT ); Tue, 14 Apr 2015 00:57:56 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47643 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751002AbbDNE5s (ORCPT ); Tue, 14 Apr 2015 00:57:48 -0400 Date: Mon, 13 Apr 2015 23:57:39 -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: <20150414045739.GG4412@treble.hsd1.ky.comcast.net> References: <1428844554-4015-1-git-send-email-minfei.huang@hotmail.com> <20150413231305.GD4412@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: 3565 Lines: 81 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. But regardless I really don't see a good reason to encourage this kind of insanity in the livepatch code. -- 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/