Re: [PATCH 00/14] Fix wrong %pF and %pS printk format specifier usages

From: Helge Deller <deller@gmx.de>
To: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Cc: linux-kernel@vger.kernel.org,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Petr Mladek <pmladek@suse.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Tony Luck <tony.luck@intel.com>,
	Fenghua Yu <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, 7 Sep 2017 14:38:21 +0200	[thread overview]
Message-ID: <0604f27e-24ab-625b-9013-c6c0f4f6acc1@gmx.de> (raw)
In-Reply-To: <20170907095119.GE533@jagdpanzerIV.localdomain>

On 07.09.2017 11:51, Sergey Senozhatsky wrote:
> On (09/07/17 18:36), Sergey Senozhatsky wrote:
> [..]
>>> I can look into adding such check-code, but even then the warning will
>>> only show up if you run on ia64, ppc64 and parisc64.
> 
> sorry, not sure I understand the "warning" part.

I was thinking about adding code which warns at runtime if %pF/%pS is
presumable used wrongly.
You are thinking about code to work around the complexity by some
kind of autodetection.

> what I'm thinking about is:
> 
> - every platform that needs descriptor dereference defines its own
>   function. otherwise dereference_descriptor(p) is just (p).
> 
> - so it's something like
> 
>   arch/platform_abc/include/asm/sections.h
> 
> #undef dereference_function_descriptor
> static inline void *dereference_function_descriptor(void *ptr)
> {
> 	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.

> 	if (!probe_kernel_address(....))
> 		return function_ip;
> 	return ptr;
> }
> 
> - so then in lib/vsprintf.c we can do unconditionally
> 
>   case F:
>   case f:
>   case S:
>   case s:
>   case B:
> 	ptr = dereference_function_descriptor(ptr);
> 	return symbol_string(....);
> 
>   because platforms will take care of proper descriptor dereference,
>   when needed.

Ok, but...

> - and ideally we even can drop %pF-%pf. because there won't
>   be any difference between `S' and `F'.
> something like this.
> let's see if this is possible.
> any thoughts?

I see your idea, nevertheless, there *is* a difference between
a "pointer to some assembler statement" (%pS), and a 
"pointer to a function" (%pF) on some architectures.
That's why %pF and %pS printk specifiers were introduced in 2008
by Linus in commit 0fe1ef24f7bd0020f29ffe287dfdb9ead33ca0b2.

People will probably get it wrong sometimes, and to try to avoid this
by some magic autodetection is IMHO the wrong solution.

Instead, maybe adding some checks to scripts/checkpatch.pl can help?
E.g. warn if %pF is used in combination with the keywords like 
_builtin_return_address, _RET_IP_, and similar.

Helge