Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932731AbdIHUuQ (ORCPT ); Fri, 8 Sep 2017 16:50:16 -0400 Received: from mout.gmx.net ([212.227.15.15]:64773 "EHLO mout.gmx.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757009AbdIHUuP (ORCPT ); Fri, 8 Sep 2017 16:50:15 -0400 Subject: Re: [PATCH 00/14] Fix wrong %pF and %pS printk format specifier usages To: Sergey Senozhatsky , "Luck, Tony" Cc: "linux-kernel@vger.kernel.org" , Sergey Senozhatsky , Petr Mladek , Andrew Morton , "Yu, Fenghua" , Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman References: <1504729681-3504-1-git-send-email-deller@gmx.de> <20170907004522.GA3885@jagdpanzerIV.localdomain> <8b93f9ca-95f6-4e40-1cc8-d1a65833abff@gmx.de> <20170907075653.GA533@jagdpanzerIV.localdomain> <20170907083207.GC533@jagdpanzerIV.localdomain> <667b8849-fb60-a312-2483-505252ff737e@gmx.de> <20170907093631.GD533@jagdpanzerIV.localdomain> <20170907095119.GE533@jagdpanzerIV.localdomain> <0604f27e-24ab-625b-9013-c6c0f4f6acc1@gmx.de> <3908561D78D1C84285E8C5FCA982C28F6136C2ED@ORSMSX114.amr.corp.intel.com> <20170908061830.GA496@jagdpanzerIV.localdomain> From: Helge Deller Message-ID: <6fdd62aa-e9e7-8954-da6b-6fa5e73983c5@gmx.de> Date: Fri, 8 Sep 2017 22:49:51 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20170908061830.GA496@jagdpanzerIV.localdomain> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Provags-ID: V03:K0:gKu5wqNx6FFNhZexeYR4dRCcXZsQ8KdwT+HUvzy5L99QnrEPuDq fkaG3fxxrlCu4KCGsVyC7Uvh8OuNe/Un6ZtCs4PLnqOdU47vTfb1X0k6LALmQZ/b9QVHfjk l4ee9KioGhMzvr0vyA7l6bIBQS4v0z0BWAN5xK2UK7KtZ8Rs077IFkFFgG2n/mj3wyJkZk+ JSTAXMeJxsONOtjTaeP7g== X-UI-Out-Filterresults: notjunk:1;V01:K0:3cMOZ+matVA=:koMZrEpU110lpLiXgrHDQT 3HmxQFd3v/gK3kC8RCQTzVsT0j+AzdHuM1zQZyx977UoYxkN+uN03WS8WYCPyVZDLvHMA1Fur Uinv6tZWdTqb9UhMCFs274AEiSBn036/0UNuvl9wke8e0EHbyhylhtwFhgPWTU54/2E/f+QH5 fpyiZ9XbMRz9dNOzgW0bUUb73KaIeCbXJwaK847P0Ek3cUt1x42fU4BK6kWcK17c/UE10fTky kwfCakkuPANRRFZWf5wp0egdUHOnisKN/ph1fg3mgrBfJC8H7lcddPpAd0ecrOkG4fGWbdBFG RkYaCeXmMFnWxfas+QRjYMKrmYp+bho/4Q5upln4KeCB/+qjCMkOa0EnOp0HoMdV7SH7HINZI rxJ5S4bH18/o/HfwMoHYZTSNx3igYsa1QR6XGA+r2EWKOqRXooRtUpCBZeptTj/k5xi4K9zhr fOdoSrHDfMURV9TSEk+QYZm0Ave+0PnG3lQk8OE7agcL4LGYRF3CKX0YeITBjmqfCW1pZJyF+ r/Y3qJlxEzy+czGTsHYy9ULdgJ++D/kaZtxQZ6rWTqLPoXwDeT7R+nGt6eB8qFDyNkbzqT64t vJLO0i/sOPvQWBtuxVVoEZO4Lp5DzZkZHkfWZp/rVy1JfAAazbFQLhdX5ZmFoOC6pXXd340Hr iXT1wyZ2uvgP8TlEy8NaW3+CehXTAlZVkFlgRT7DC51IcJt+0cIpnN3avGTSFPdluhaAhKjuU EMd3uLQj5trx3f7aKf9NfemNan2yYuU5h0m5YETtTO33Z/39jTWFf9jMBOqQjlFEv3okc6rti CByrLIPf5RMeMqWIgP2RuqOJK0im4OgDJT8nxyf/EMvmIAA388= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4352 Lines: 132 On 08.09.2017 08:18, Sergey Senozhatsky wrote: > On (09/07/17 16:05), Luck, Tony wrote: > [..] >>>> if (not_a_function_descriptor(ptr)) >>>> return ptr; >>> >>> I'm not sure if it's possible on ia64/ppc64/parisc64 >>> to reliably detect if it's a function descriptor or not. >> >> Agreed. I don't know how to write this test (without changing the compiler to >> put the pointers in a separate section ... and then changing the module loader >> to keep a list of all these sections). > > let me try one more time :) > > so below is a number of assumptions, let me know if anything is wrong > there.... and let's try to fix the "wrong bits" ;) > > > RFC > > > 1) function descriptor table is in .data, not in .text > correct? > > 2) symbol resolution consists of 3 steps: > > a) we check if this is a kernel symbol and resolve it if so > b) we check if the addr belongs to any module and resolve the addr > if so > c) we check if the addr is bpf and resolve it if so. let's skip this part. > > > so, for (a) we probably can do something like below. can't we? > // not tested, as usual. > > > --- > > diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c > index 127e7cfafa55..4807e204428e 100644 > --- a/kernel/kallsyms.c > +++ b/kernel/kallsyms.c > @@ -319,6 +319,16 @@ const char *kallsyms_lookup(unsigned long addr, > namebuf[KSYM_NAME_LEN - 1] = 0; > namebuf[0] = 0; > > +#if defined(CONFIG_IA64) || defined(CONFIG_PPC64) || defined(CONFIG_PARISC) > + if (!is_ksym_addr(addr)) { > + unsigned long deref_addr; > + > + deref_addr = dereference_function_descriptor(addr); > + if (is_ksym_addr(deref_addr)) > + addr = deref_addr; > + } > +#endif > + > if (is_ksym_addr(addr)) { > unsigned long pos; > > > ---- > > if the addr is not in kernel .text, then try dereferencing it and check > if the dereferenced addr is in kernel .text. > > > > now, for (b) we can do something like below... probably. > > if the addr is not module .text (not .data), then check if dereferenced > address is module .text (not .data). > > > --- > > diff --git a/kernel/module.c b/kernel/module.c > index de66ec825992..f81c67b745ff 100644 > --- a/kernel/module.c > +++ b/kernel/module.c > @@ -3865,6 +3865,16 @@ static inline int within(unsigned long addr, void *start, unsigned long size) > return ((void *)addr >= start && (void *)addr < start + size); > } > > +static inline bool __mod_text_address(struct module *mod, > + unsigned long addr) > +{ > + /* Make sure it's within the text section. */ > + if (!within(addr, mod->init_layout.base, mod->init_layout.text_size) > + && !within(addr, mod->core_layout.base, mod->core_layout.text_size)) > + return false; > + return true; > +} > + > #ifdef CONFIG_KALLSYMS > /* > * This ignores the intensely annoying "mapping symbols" found > @@ -3942,6 +3952,14 @@ const char *module_address_lookup(unsigned long addr, > preempt_disable(); > mod = __module_address(addr); > if (mod) { > +#if defined(CONFIG_IA64) || defined(CONFIG_PPC64) || defined(CONFIG_PARISC) > + unsigned long deref_addr; > + > + if (!__mod_text_address(mod, addr)) > + deref_addr = dereference_function_descriptor(addr); > + if (__mod_text_address(mod, deref_addr)) > + addr = deref_addr; > +#endif > if (modname) > *modname = mod->name; > ret = get_ksymbol(mod, addr, size, offset); > > --- > > so there are probably some broken parts there. like... > I don't know. something. > > so - what is broken, and how can we fix/tweak it? help me out. Sergey, I'm sure there is a way how you can get it somehow to work the way you describe above, but even then nobody can guarantee you that it will work in 100% of the cases. It's somehow like "we have %lu and %c specifiers, and it's basically the same, so let's try to figure out at runtime which one should be used based on analysis of what was given as argument". It may work somehow, but not always. What about the idea of a %luS specifier (or something other) ? Helge