Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754583AbdIHGVZ (ORCPT ); Fri, 8 Sep 2017 02:21:25 -0400 Received: from mail-pf0-f193.google.com ([209.85.192.193]:38481 "EHLO mail-pf0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754287AbdIHGVX (ORCPT ); Fri, 8 Sep 2017 02:21:23 -0400 X-Google-Smtp-Source: ADKCNb6RnNkahjDTmqOgfPsmI24spsg9y3cRk3K8hB4vhDblAnKo0rUIcQSJBEdMqH07RNnPSTyk4A== Date: Fri, 8 Sep 2017 15:18:30 +0900 From: Sergey Senozhatsky To: Helge Deller , "Luck, Tony" Cc: Sergey Senozhatsky , "linux-kernel@vger.kernel.org" , Sergey Senozhatsky , Petr Mladek , Andrew Morton , "Yu, Fenghua" , Benjamin Herrenschmidt , Paul Mackerras , Michael Ellerman Subject: Re: [PATCH 00/14] Fix wrong %pF and %pS printk format specifier usages Message-ID: <20170908061830.GA496@jagdpanzerIV.localdomain> 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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <3908561D78D1C84285E8C5FCA982C28F6136C2ED@ORSMSX114.amr.corp.intel.com> User-Agent: Mutt/1.9.0 (2017-09-02) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3824 Lines: 124 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. btw, get_ksymbol() is actually interesting. it scans module's sections, so if we are able to distinguish descriptor ELF sections, then we can dereference addr only if it belong to descriptor table ELF section. -ss