From: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
To: Helge Deller <deller@gmx.de>
Cc: "Luck, Tony" <tony.luck@intel.com>,
Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
Petr Mladek <pmladek@suse.com>,
Andrew Morton <akpm@linux-foundation.org>,
"Yu, Fenghua" <fenghua.yu@intel.com>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Paul Mackerras <paulus@samba.org>,
Michael Ellerman <mpe@ellerman.id.au>
Subject: Re: [PATCH 00/14] Fix wrong %pF and %pS printk format specifier usages
Date: Thu, 14 Sep 2017 16:40:44 +0900 [thread overview]
Message-ID: <20170914074044.GE599@jagdpanzerIV.localdomain> (raw)
In-Reply-To: <67a0aad8-5412-60f8-6481-562d37995eb2@gmx.de>
On (09/08/17 20:28), Helge Deller wrote:
[..]
> I don't like this kind of trying to figure out at runtime at all.
> It's too much guessing in here IMHO.
well, may be we can avoid any guessing by checking that the
pointer belongs to .opd section.
for kernel we can add 2 new unsigned longs - __opd_start __opd_end -- and
tweak the corresponding arch/${FOO}/kernel/vmlinux.lds.S file to set those
two, the same way text, unwinding, etc. are set.
.... wait a second.
arch/ia64/kernel/vmlinux.lds.S already handles .opd section
.opd : AT(ADDR(.opd) - LOAD_OFFSET) {
*(.opd)
}
it just doesn't save start/end addresses. so all we need to to
is
.opd : AT(ADDR(.opd) - LOAD_OFFSET) {
+ __opd_start = .;
*(.opd)
+ __opd_end = .;
}
and tweak symbol dereference
static inline void *dereference_function_descriptor(void *ptr)
{
struct fdesc *desc = ptr;
void *p;
+ if (prt < (void *)__start_opd || (void *)__end_opd < ptr)
+ return ptr;
+
if (!probe_kernel_address(&desc->ip, p))
ptr = p;
return ptr;
now, the modules.
module_frob_arch_sections() has the following lines
else if (strcmp(".opd", secstrings + s->sh_name) == 0)
mod->arch.opd = s;
so, once, again, we keep the .opd section info in memory. and we
also have the size of that section
mod->arch.opd->sh_size = fdescs * sizeof(struct fdesc);
so it seems that we've got what we need. need to provide arch callback
(same way as we do with dereference_function_descriptor() to properly
dereference modules' symbols).
so I think we almost have what we need to make ps/pS smart enough
on ppc64/ia64/parisc.
powerpc and parisc handle kernel .opd section as well:
arch/powerpc/kernel/vmlinux.lds.S: .opd
arch/parisc/kernel/vmlinux.lds.S: .opd
need to check more.
> What about this idea:
> For %pF we always have pointers to functions, e.g.:
> printk("Going to call: %pF\n", gettimeofday);
> printk("Going to call: %pF\n", p->func);
>
> and for %pS most (if not all) usages use some kind of casting
> from "unsigned long" to "void *", e.g.:
> printk("%s: called from %pS\n", __func__, (void *)_RET_IP_);
> printk("%s: called from %pS\n", __func__, (void *)__builtin_return_address(0));
> printk("Faulted at %pS\n", (void *)regs->ip);
>
> So, what if we for the %pS case simply take the type as it is
> (unsigned long) and introduce a new printk-format, e.g. "%luS" ?
> The %pS examples above then become:
> printk("%s: called from %luS\n", __func__, _RET_IP_);
> printk("%s: called from %luS\n", __func__, __builtin_return_address(0));
> printk("Faulted at %luS\n", regs->ip);
>
> That way we don't need type-casting, gain compile-time type
> checks from the compiler, and we could add a checkpatch (or occinelle)
> check which checks for the combination of %pF/%pS and "void*" keyword
> and suggest to use %luS.
>
> Opinions?
hm. sounds interesting. but I'm afraid people won't be so happy
to learn a new printk format specifier.
-ss
next prev parent reply other threads:[~2017-09-14 7:40 UTC|newest]
Thread overview: 62+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-06 20:27 [PATCH 00/14] Fix wrong %pF and %pS printk format specifier usages Helge Deller
2017-09-06 20:27 ` [PATCH 01/14] arm: Use %pS printk format for symbols from direct addresses Helge Deller
2017-09-06 20:27 ` [PATCH 02/14] um: " Helge Deller
2017-09-12 12:10 ` Petr Mladek
2017-09-21 20:13 ` Richard Weinberger
2017-09-06 20:27 ` [PATCH 03/14] x86: " Helge Deller
2017-09-06 20:27 ` [PATCH 04/14] ti_sci: Use %pS printk format for " Helge Deller
2017-09-08 23:30 ` Nishanth Menon
2017-09-09 0:30 ` Santosh Shilimkar
2017-09-06 20:27 ` [PATCH 05/14] i915: " Helge Deller
2017-09-27 12:24 ` [Intel-gfx] " Daniel Vetter
2017-09-06 20:27 ` [PATCH 06/14] md/bcache: " Helge Deller
2017-09-07 4:50 ` Coly Li
2017-09-07 7:42 ` Helge Deller
2017-09-07 7:49 ` Coly Li
2017-09-07 8:05 ` Sergey Senozhatsky
2017-09-06 20:27 ` [PATCH 07/14] power/avs: " Helge Deller
2017-09-08 23:37 ` Nishanth Menon
2017-09-06 20:27 ` [PATCH 08/14] fs/f2fs: " Helge Deller
2017-09-06 20:27 ` [PATCH 09/14] fs/pstore: " Helge Deller
2018-11-29 23:26 ` Kees Cook
2018-11-29 23:49 ` Luck, Tony
2018-11-30 0:40 ` Kees Cook
2017-09-06 20:27 ` [PATCH 10/14] fs/xfs: " Helge Deller
2017-09-08 7:38 ` Christoph Hellwig
2017-09-18 18:37 ` Darrick J. Wong
2017-09-06 20:27 ` [PATCH 11/14] smp: Use %pF printk format specifier for function pointers Helge Deller
2017-09-06 20:27 ` [PATCH 12/14] mm/memblock: Use %pS printk format for direct addresses Helge Deller
2017-09-06 20:28 ` [PATCH 13/14] netfilter/ipvs: " Helge Deller
2017-10-09 5:52 ` Simon Horman
2017-11-06 13:46 ` Pablo Neira Ayuso
2017-09-06 20:28 ` [PATCH 14/14] sound/core: " Helge Deller
2017-09-07 8:36 ` Takashi Iwai
2017-09-07 0:45 ` [PATCH 00/14] Fix wrong %pF and %pS printk format specifier usages Sergey Senozhatsky
2017-09-07 6:01 ` Helge Deller
2017-09-07 7:56 ` Sergey Senozhatsky
2017-09-07 8:32 ` Sergey Senozhatsky
2017-09-07 9:12 ` Helge Deller
2017-09-07 9:36 ` Sergey Senozhatsky
2017-09-07 9:51 ` Sergey Senozhatsky
2017-09-07 12:38 ` Helge Deller
2017-09-07 16:05 ` Luck, Tony
2017-09-08 6:18 ` Sergey Senozhatsky
2017-09-08 17:25 ` Luck, Tony
2017-09-08 18:28 ` Helge Deller
2017-09-14 7:40 ` Sergey Senozhatsky [this message]
2017-09-14 8:03 ` Sergey Senozhatsky
2017-09-14 8:39 ` Helge Deller
2017-09-14 9:27 ` Sergey Senozhatsky
2017-09-14 9:47 ` Helge Deller
2017-09-14 16:01 ` Luck, Tony
2017-09-18 7:03 ` Sergey Senozhatsky
2017-09-14 6:53 ` Sergey Senozhatsky
2017-09-08 20:49 ` Helge Deller
2017-09-12 11:18 ` Petr Mladek
2017-09-14 6:44 ` Sergey Senozhatsky
2017-09-08 22:23 ` Yu, Fenghua
2017-09-14 6:35 ` Sergey Senozhatsky
2017-09-07 16:50 ` Joe Perches
2017-09-08 6:23 ` Sergey Senozhatsky
2017-09-08 20:39 ` Helge Deller
2017-09-12 12:23 ` Petr Mladek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170914074044.GE599@jagdpanzerIV.localdomain \
--to=sergey.senozhatsky.work@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=benh@kernel.crashing.org \
--cc=deller@gmx.de \
--cc=fenghua.yu@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mpe@ellerman.id.au \
--cc=paulus@samba.org \
--cc=pmladek@suse.com \
--cc=sergey.senozhatsky@gmail.com \
--cc=tony.luck@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).