[PATCHv3 0/7] printk/ia64/ppc64/parisc64: let's deprecate %pF/%pf printk specifiers

[PATCHv3 0/7] printk/ia64/ppc64/parisc64: let's deprecate %pF/%pf printk specifiers

[Sergey Senozhatsky]

	Hello


	Petr, could you please pick up the series?

==========================================================================

	On some arches C function pointers are indirect and point to
a function descriptor, which contains the actual pointer to the code.
This mostly doesn't matter, except for cases when people want to print
out function pointers in symbolic format, because the usual '%pS/%ps'
does not work on those arches as expected. That's the reason why we
have '%pF/%pf', but since it's here because of a subtle ABI detail
specific to some arches (ppc64/ia64/parisc64) it's easy to misuse
'%pF/%pf' and '%pS/%ps' (see [1], for example).

	This patch set attempts to move ia64/ppc64/parisc64 C function
pointer ABI details out of printk() to arch code. Function dereference
code now checks if a pointer belongs to a .opd ELF section and dereferences
that pointer only if it does. The kernel and modules have their own .opd
sections that's why I use two different ARCH functions: for kernel and
for module pointer dereference.

	I planned to remove dereference_function_descriptor() entirely,
but then I discovered a bunch other uses cases (kgdbts, init/main.c,
extable, etc.), so I decided to keep dereference_function_descriptor()
around because the main point of this patch set is to deprecate %pF/%pf.
But at the same time, I think I can go further and handle both kernel
and module descriptor dereference in dereference_function_descriptor().
We need a module pointer for module .opd check, so that will come at an
extra cost of module lookup (may be there will some other issues along
the way, haven't checked it).

Right now we've got:

- dereference_function_descriptor(addr)
        a generic (old) function. it simply attempts to dereference
        whatever pointer we give it.

- dereference_kernel_function_descriptor(addr)
        dereferences a kernel pointer if it's within the kernel's .opd
        section.

- dereference_module_function_descriptor(module, addr)
        dereference a module pointer if it's within the module's .opd
        section.

v3:
-- picked up ACKs and Tested-by
-- tweaked checkpatch warning (Joe)
-- updated Documentation

v2:
-- convert dereference_function_descriptor() to unsigned long
-- fix kernel descriptor range checks (Helge)
-- fix parisc module descriptor range check (Helge)
-- fix ppc64 module range check
-- add checkpatch patch

Sergey Senozhatsky (7):
  switch dereference_function_descriptor() to `unsigned long'
  sections: split dereference_function_descriptor()
  ia64: Add .opd based function descriptor dereference
  powerpc64: Add .opd based function descriptor dereference
  parisc64: Add .opd based function descriptor dereference
  symbol lookup: use new kernel and module dereference functions
  checkpatch: add pF/pf deprecation warning

 Documentation/printk-formats.txt          | 20 ++++++++++----------
 arch/ia64/include/asm/sections.h          | 16 ++++++++++++----
 arch/ia64/kernel/module.c                 | 13 +++++++++++++
 arch/ia64/kernel/vmlinux.lds.S            |  2 ++
 arch/parisc/boot/compressed/vmlinux.lds.S |  2 ++
 arch/parisc/include/asm/sections.h        |  4 +++-
 arch/parisc/kernel/module.c               | 17 +++++++++++++++++
 arch/parisc/kernel/process.c              | 15 ++++++++++++---
 arch/parisc/kernel/vmlinux.lds.S          |  2 ++
 arch/parisc/mm/init.c                     |  4 ++--
 arch/powerpc/include/asm/module.h         |  3 +++
 arch/powerpc/include/asm/sections.h       | 17 ++++++++++++++---
 arch/powerpc/kernel/module_64.c           | 16 ++++++++++++++++
 arch/powerpc/kernel/vmlinux.lds.S         |  2 ++
 drivers/misc/kgdbts.c                     |  2 +-
 include/asm-generic/sections.h            |  8 ++++++--
 include/linux/moduleloader.h              |  4 ++++
 init/main.c                               |  2 +-
 kernel/extable.c                          |  2 +-
 kernel/kallsyms.c                         |  1 +
 kernel/module.c                           |  9 ++++++++-
 lib/vsprintf.c                            |  5 +----
 scripts/checkpatch.pl                     | 11 +++++++++--
 23 files changed, 142 insertions(+), 35 deletions(-)

-- 
2.14.2

[PATCHv3 1/7] switch dereference_function_descriptor() to `unsigned long'

[Sergey Senozhatsky]

Convert dereference_function_descriptor() to accept and return
`unsigned long'. There will be two new ARCH function for kernel
and module function pointer dereference, which will work with
`unsigned long', so the patch unifies interfaces.

Besides, dereference_function_descriptor() mostly work with
`unsigned long':

drivers/misc/kgdbts.c:
addr = (unsigned long) dereference_function_descriptor((void *)addr);

init/main.c:
addr = (unsigned long) dereference_function_descriptor(fn);

kernel/extable.c:
addr = (unsigned long) dereference_function_descriptor(ptr);

kernel/module.c:
unsigned long a = (unsigned long)dereference_function_descriptor(addr);

Convert dereference_function_descriptor() users tree-wide.

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Tested-by: Helge Deller <deller@gmx.de> # parisc64
Tested-by: Santosh Sivaraj <santosh@fossix.org> # powerpc64
Acked-by: Michael Ellerman <mpe@ellerman.id.au> # powerpc64
Tested-by: Tony Luck <tony.luck@intel.com> # ia64
---
 arch/ia64/include/asm/sections.h    | 6 +++---
 arch/parisc/include/asm/sections.h  | 2 +-
 arch/parisc/kernel/process.c        | 6 +++---
 arch/parisc/mm/init.c               | 4 ++--
 arch/powerpc/include/asm/sections.h | 6 +++---
 drivers/misc/kgdbts.c               | 2 +-
 init/main.c                         | 2 +-
 kernel/extable.c                    | 2 +-
 kernel/module.c                     | 2 +-
 lib/vsprintf.c                      | 2 +-
 10 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/arch/ia64/include/asm/sections.h b/arch/ia64/include/asm/sections.h
index 2ab2003698ef..de6bfa1ef8fb 100644
--- a/arch/ia64/include/asm/sections.h
+++ b/arch/ia64/include/asm/sections.h
@@ -27,13 +27,13 @@ extern char __start_unwind[], __end_unwind[];
 extern char __start_ivt_text[], __end_ivt_text[];
 
 #undef dereference_function_descriptor
-static inline void *dereference_function_descriptor(void *ptr)
+static inline unsigned long dereference_function_descriptor(unsigned long ptr)
 {
-	struct fdesc *desc = ptr;
+	struct fdesc *desc = (struct fdesc *)ptr;
 	void *p;
 
 	if (!probe_kernel_address(&desc->ip, p))
-		ptr = p;
+		ptr = (unsigned long)p;
 	return ptr;
 }
 
diff --git a/arch/parisc/include/asm/sections.h b/arch/parisc/include/asm/sections.h
index 9d13c3507ad6..59fbe0067112 100644
--- a/arch/parisc/include/asm/sections.h
+++ b/arch/parisc/include/asm/sections.h
@@ -6,7 +6,7 @@
 
 #ifdef CONFIG_64BIT
 #undef dereference_function_descriptor
-void *dereference_function_descriptor(void *);
+unsigned long dereference_function_descriptor(unsigned long);
 #endif
 
 #endif
diff --git a/arch/parisc/kernel/process.c b/arch/parisc/kernel/process.c
index 30f92391a93e..d350aa913acc 100644
--- a/arch/parisc/kernel/process.c
+++ b/arch/parisc/kernel/process.c
@@ -267,13 +267,13 @@ get_wchan(struct task_struct *p)
 }
 
 #ifdef CONFIG_64BIT
-void *dereference_function_descriptor(void *ptr)
+unsigned long dereference_function_descriptor(unsigned long ptr)
 {
-	Elf64_Fdesc *desc = ptr;
+	Elf64_Fdesc *desc = (Elf64_Fdesc *)ptr;
 	void *p;
 
 	if (!probe_kernel_address(&desc->addr, p))
-		ptr = p;
+		ptr = (unsigned long)p;
 	return ptr;
 }
 #endif
diff --git a/arch/parisc/mm/init.c b/arch/parisc/mm/init.c
index 1ca9a2b4239f..06e1b79e2946 100644
--- a/arch/parisc/mm/init.c
+++ b/arch/parisc/mm/init.c
@@ -389,10 +389,10 @@ static void __init setup_bootmem(void)
 static int __init parisc_text_address(unsigned long vaddr)
 {
 	static unsigned long head_ptr __initdata;
+	unsigned long addr = (unsigned long)&parisc_kernel_start;
 
 	if (!head_ptr)
-		head_ptr = PAGE_MASK & (unsigned long)
-			dereference_function_descriptor(&parisc_kernel_start);
+		head_ptr = PAGE_MASK & dereference_function_descriptor(addr);
 
 	return core_kernel_text(vaddr) || vaddr == head_ptr;
 }
diff --git a/arch/powerpc/include/asm/sections.h b/arch/powerpc/include/asm/sections.h
index 7902d6358854..67379b8945e8 100644
--- a/arch/powerpc/include/asm/sections.h
+++ b/arch/powerpc/include/asm/sections.h
@@ -66,13 +66,13 @@ static inline int overlaps_kvm_tmp(unsigned long start, unsigned long end)
 
 #ifdef PPC64_ELF_ABI_v1
 #undef dereference_function_descriptor
-static inline void *dereference_function_descriptor(void *ptr)
+static inline unsigned long dereference_function_descriptor(unsigned long ptr)
 {
-	struct ppc64_opd_entry *desc = ptr;
+	struct ppc64_opd_entry *desc = (struct ppc64_opd_entry *)ptr;
 	void *p;
 
 	if (!probe_kernel_address(&desc->funcaddr, p))
-		ptr = p;
+		ptr = (unsigned long)p;
 	return ptr;
 }
 #endif /* PPC64_ELF_ABI_v1 */
diff --git a/drivers/misc/kgdbts.c b/drivers/misc/kgdbts.c
index fc7efedbc4be..6a5a159dfb75 100644
--- a/drivers/misc/kgdbts.c
+++ b/drivers/misc/kgdbts.c
@@ -225,7 +225,7 @@ static unsigned long lookup_addr(char *arg)
 		addr = (unsigned long)_do_fork;
 	else if (!strcmp(arg, "hw_break_val"))
 		addr = (unsigned long)&hw_break_val;
-	addr = (unsigned long) dereference_function_descriptor((void *)addr);
+	addr = dereference_function_descriptor(addr);
 	return addr;
 }
 
diff --git a/init/main.c b/init/main.c
index 83bdfa4750b1..ac56f7a4501f 100644
--- a/init/main.c
+++ b/init/main.c
@@ -764,7 +764,7 @@ static bool __init_or_module initcall_blacklisted(initcall_t fn)
 	if (list_empty(&blacklisted_initcalls))
 		return false;
 
-	addr = (unsigned long) dereference_function_descriptor(fn);
+	addr = dereference_function_descriptor((unsigned long)fn);
 	sprint_symbol_no_offset(fn_name, addr);
 
 	/*
diff --git a/kernel/extable.c b/kernel/extable.c
index 9aa1cc41ecf7..e48d6ba4ce6c 100644
--- a/kernel/extable.c
+++ b/kernel/extable.c
@@ -167,7 +167,7 @@ int kernel_text_address(unsigned long addr)
 int func_ptr_is_kernel_text(void *ptr)
 {
 	unsigned long addr;
-	addr = (unsigned long) dereference_function_descriptor(ptr);
+	addr = dereference_function_descriptor((unsigned long)ptr);
 	if (core_kernel_text(addr))
 		return 1;
 	return is_module_text_address(addr);
diff --git a/kernel/module.c b/kernel/module.c
index de66ec825992..ea77ab13bead 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1067,7 +1067,7 @@ EXPORT_SYMBOL(__symbol_put);
 void symbol_put_addr(void *addr)
 {
 	struct module *modaddr;
-	unsigned long a = (unsigned long)dereference_function_descriptor(addr);
+	unsigned long a = dereference_function_descriptor((unsigned long)addr);
 
 	if (core_kernel_text(a))
 		return;
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 86c3385b9eb3..bcd906a39010 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1723,7 +1723,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 	switch (*fmt) {
 	case 'F':
 	case 'f':
-		ptr = dereference_function_descriptor(ptr);
+		ptr = (void *)dereference_function_descriptor((unsigned long)ptr);
 		/* Fallthrough */
 	case 'S':
 	case 's':
-- 
2.14.2

[PATCHv3 2/7] sections: split dereference_function_descriptor()

[Sergey Senozhatsky]

There are two format specifiers to print out a pointer in symbolic
format: '%pS/%ps' and '%pF/%pf'. On most architectures, the two
mean exactly the same thing, but some architectures (ia64, ppc64,
parisc64) use an indirect pointer for C function pointers, where
the function pointer points to a function descriptor (which in
turn contains the actual pointer to the code). The '%pF/%pf, when
used appropriately, automatically does the appropriate function
descriptor dereference on such architectures.

The "when used appropriately" part is tricky. Basically this is
a subtle ABI detail, specific to some platforms, that made it to
the API level and people can be unaware of it and miss the whole
"we need to dereference the function" business out. [1] proves
that point (note that it fixes only '%pF' and '%pS', there might
be '%pf' and '%ps' cases as well).

It appears that we can handle everything within the affected
arches and make '%pS/%ps' smart enough to retire '%pF/%pf'.
Function descriptors live in .opd elf section and all affected
arches (ia64, ppc64, parisc64) handle it properly for kernel
and modules. So we, technically, can decide if the dereference
is needed by simply looking at the pointer: if it belongs to
.opd section then we need to dereference it.

The kernel and modules have their own .opd sections, obviously,
that's why we need to split dereference_function_descriptor()
and use separate kernel and module dereference arch callbacks.

This patch does the first step, it
a) adds dereference_kernel_function_descriptor() function.
b) adds a weak alias to dereference_module_function_descriptor()
   function.

So, for the time being, we will have:
1) dereference_function_descriptor()
   A generic function, that simply dereferences the pointer. There is
   bunch of places that call it: kgdbts, init/main.c, extable, etc.

2) dereference_kernel_function_descriptor()
   A function to call on kernel symbols that does kernel .opd section
   address range test.

3) dereference_module_function_descriptor()
   A function to call on modules' symbols that does modules' .opd
   section address range test.

[1] https://marc.info/?l=linux-kernel&m=150472969730573

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Tested-by: Helge Deller <deller@gmx.de> # parisc64
Tested-by: Santosh Sivaraj <santosh@fossix.org> # powerpc64
Acked-by: Michael Ellerman <mpe@ellerman.id.au> # powerpc64
Tested-by: Tony Luck <tony.luck@intel.com> # ia64
---
 include/asm-generic/sections.h | 8 ++++++--
 include/linux/moduleloader.h   | 4 ++++
 kernel/module.c                | 6 ++++++
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index e5da44eddd2f..387f22c41e0d 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -29,6 +29,7 @@
  *	__ctors_start, __ctors_end
  *	__irqentry_text_start, __irqentry_text_end
  *	__softirqentry_text_start, __softirqentry_text_end
+ *	__start_opd, __end_opd
  */
 extern char _text[], _stext[], _etext[];
 extern char _data[], _sdata[], _edata[];
@@ -47,12 +48,15 @@ extern char __softirqentry_text_start[], __softirqentry_text_end[];
 /* Start and end of .ctors section - used for constructor calls. */
 extern char __ctors_start[], __ctors_end[];
 
+/* Start and end of .opd section - used for function descriptors. */
+extern char __start_opd[], __end_opd[];
+
 extern __visible const void __nosave_begin, __nosave_end;
 
-/* function descriptor handling (if any).  Override
- * in asm/sections.h */
+/* Function descriptor handling (if any).  Override in asm/sections.h */
 #ifndef dereference_function_descriptor
 #define dereference_function_descriptor(p) (p)
+#define dereference_kernel_function_descriptor(p) (p)
 #endif
 
 /* random extra sections (if any).  Override
diff --git a/include/linux/moduleloader.h b/include/linux/moduleloader.h
index 4d0cb9bba93e..172904e9cded 100644
--- a/include/linux/moduleloader.h
+++ b/include/linux/moduleloader.h
@@ -85,6 +85,10 @@ void module_arch_cleanup(struct module *mod);
 /* Any cleanup before freeing mod->module_init */
 void module_arch_freeing_init(struct module *mod);
 
+/* Dereference module function descriptor */
+unsigned long dereference_module_function_descriptor(struct module *mod,
+						     unsigned long addr);
+
 #ifdef CONFIG_KASAN
 #include <linux/kasan.h>
 #define MODULE_ALIGN (PAGE_SIZE << KASAN_SHADOW_SCALE_SHIFT)
diff --git a/kernel/module.c b/kernel/module.c
index ea77ab13bead..b792e814150a 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2121,6 +2121,12 @@ void __weak module_arch_freeing_init(struct module *mod)
 {
 }
 
+unsigned long __weak dereference_module_function_descriptor(struct module *mod,
+							    unsigned long addr)
+{
+	return addr;
+}
+
 /* Free a module, remove from lists, etc. */
 static void free_module(struct module *mod)
 {
-- 
2.14.2

[PATCHv3 3/7] ia64: Add .opd based function descriptor dereference

[Sergey Senozhatsky]

We are moving towards separate kernel and module function descriptor
dereference callbacks. This patch enables it for IA64.

For pointers that belong to the kernel
-  Added __start_opd and __end_opd pointers, to track the kernel
   .opd section address range;

-  Added dereference_kernel_function_descriptor(). Now we
   will dereference only function pointers that are within
   [__start_opd, __end_opd];

For pointers that belong to a module
-  Added dereference_module_function_descriptor() to handle module
   function descriptor dereference. Now we will dereference only
   pointers that are within [module->opd.start, module->opd.end].

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Tested-by: Helge Deller <deller@gmx.de> # parisc64
Tested-by: Santosh Sivaraj <santosh@fossix.org> # powerpc64
Acked-by: Michael Ellerman <mpe@ellerman.id.au> # powerpc64
Tested-by: Tony Luck <tony.luck@intel.com> # ia64
---
 arch/ia64/include/asm/sections.h | 10 +++++++++-
 arch/ia64/kernel/module.c        | 13 +++++++++++++
 arch/ia64/kernel/vmlinux.lds.S   |  2 ++
 3 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/arch/ia64/include/asm/sections.h b/arch/ia64/include/asm/sections.h
index de6bfa1ef8fb..3ba7ce9d8bc8 100644
--- a/arch/ia64/include/asm/sections.h
+++ b/arch/ia64/include/asm/sections.h
@@ -37,6 +37,14 @@ static inline unsigned long dereference_function_descriptor(unsigned long ptr)
 	return ptr;
 }
 
+#undef dereference_kernel_function_descriptor
+static inline unsigned long
+dereference_kernel_function_descriptor(unsigned long addr)
+{
+	if (addr < (unsigned long)__start_opd ||
+			addr >= (unsigned long)__end_opd)
+		return addr;
+	return dereference_function_descriptor(addr);
+}
 
 #endif /* _ASM_IA64_SECTIONS_H */
-
diff --git a/arch/ia64/kernel/module.c b/arch/ia64/kernel/module.c
index d1d945c6bd05..0741ae6fa957 100644
--- a/arch/ia64/kernel/module.c
+++ b/arch/ia64/kernel/module.c
@@ -35,6 +35,7 @@
 
 #include <asm/patch.h>
 #include <asm/unaligned.h>
+#include <asm/sections.h>
 
 #define ARCH_MODULE_DEBUG 0
 
@@ -917,3 +918,15 @@ module_arch_cleanup (struct module *mod)
 	if (mod->arch.core_unw_table)
 		unw_remove_unwind_table(mod->arch.core_unw_table);
 }
+
+unsigned long
+dereference_module_function_descriptor(struct module *mod, unsigned long addr)
+{
+	Elf64_Shdr *opd = mod->arch.opd;
+
+	if (addr < opd->sh_addr ||
+			addr >= (opd->sh_addr + opd->sh_size))
+		return addr;
+
+	return dereference_function_descriptor(addr);
+}
diff --git a/arch/ia64/kernel/vmlinux.lds.S b/arch/ia64/kernel/vmlinux.lds.S
index 798026dde52e..f872ba5ff82a 100644
--- a/arch/ia64/kernel/vmlinux.lds.S
+++ b/arch/ia64/kernel/vmlinux.lds.S
@@ -107,7 +107,9 @@ SECTIONS {
 	RODATA
 
 	.opd : AT(ADDR(.opd) - LOAD_OFFSET) {
+		__start_opd = .;
 		*(.opd)
+		__end_opd = .;
 	}
 
 	/*
-- 
2.14.2

[PATCHv3 4/7] powerpc64: Add .opd based function descriptor dereference

[Sergey Senozhatsky]

We are moving towards separate kernel and module function descriptor
dereference callbacks. This patch enables it for powerpc64.

For pointers that belong to the kernel
-  Added __start_opd and __end_opd pointers, to track the kernel
   .opd section address range;

-  Added dereference_kernel_function_descriptor(). Now we
   will dereference only function pointers that are within
   [__start_opd, __end_opd];

For pointers that belong to a module
-  Added dereference_module_function_descriptor() to handle module
   function descriptor dereference. Now we will dereference only
   pointers that are within [module->opd.start, module->opd.end].

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Tested-by: Helge Deller <deller@gmx.de> # parisc64
Tested-by: Santosh Sivaraj <santosh@fossix.org> # powerpc64
Acked-by: Michael Ellerman <mpe@ellerman.id.au> # powerpc64
Tested-by: Tony Luck <tony.luck@intel.com> # ia64
---
 arch/powerpc/include/asm/module.h   |  3 +++
 arch/powerpc/include/asm/sections.h | 11 +++++++++++
 arch/powerpc/kernel/module_64.c     | 16 ++++++++++++++++
 arch/powerpc/kernel/vmlinux.lds.S   |  2 ++
 4 files changed, 32 insertions(+)

diff --git a/arch/powerpc/include/asm/module.h b/arch/powerpc/include/asm/module.h
index 6c0132c7212f..7e28442827f1 100644
--- a/arch/powerpc/include/asm/module.h
+++ b/arch/powerpc/include/asm/module.h
@@ -45,6 +45,9 @@ struct mod_arch_specific {
 	unsigned long tramp;
 #endif
 
+	/* For module function descriptor dereference */
+	unsigned long start_opd;
+	unsigned long end_opd;
 #else /* powerpc64 */
 	/* Indices of PLT sections within module. */
 	unsigned int core_plt_section;
diff --git a/arch/powerpc/include/asm/sections.h b/arch/powerpc/include/asm/sections.h
index 67379b8945e8..6b4ee0d1645f 100644
--- a/arch/powerpc/include/asm/sections.h
+++ b/arch/powerpc/include/asm/sections.h
@@ -75,6 +75,17 @@ static inline unsigned long dereference_function_descriptor(unsigned long ptr)
 		ptr = (unsigned long)p;
 	return ptr;
 }
+
+#undef dereference_kernel_function_descriptor
+static inline unsigned long
+dereference_kernel_function_descriptor(unsigned long addr)
+{
+	if (addr < (unsigned long)__start_opd ||
+			addr >= (unsigned long)__end_opd)
+		return addr;
+
+	return dereference_function_descriptor(addr);
+}
 #endif /* PPC64_ELF_ABI_v1 */
 
 #endif
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 0b0f89685b67..94caec045a90 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -344,6 +344,11 @@ int module_frob_arch_sections(Elf64_Ehdr *hdr,
 		else if (strcmp(secstrings+sechdrs[i].sh_name,"__versions")==0)
 			dedotify_versions((void *)hdr + sechdrs[i].sh_offset,
 					  sechdrs[i].sh_size);
+		else if (!strcmp(secstrings + sechdrs[i].sh_name, ".opd")) {
+			me->arch.start_opd = sechdrs[i].sh_addr;
+			me->arch.end_opd = sechdrs[i].sh_addr +
+					   sechdrs[i].sh_size;
+		}
 
 		/* We don't handle .init for the moment: rename to _init */
 		while ((p = strstr(secstrings + sechdrs[i].sh_name, ".init")))
@@ -712,6 +717,17 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
 	return 0;
 }
 
+#ifdef PPC64_ELF_ABI_v1
+unsigned long dereference_module_function_descriptor(struct module *mod,
+						     unsigned long addr)
+{
+	if (addr < mod->arch.start_opd || addr >= mod->arch.end_opd)
+		return addr;
+
+	return dereference_function_descriptor(addr);
+}
+#endif /* PPC64_ELF_ABI_v1 */
+
 #ifdef CONFIG_DYNAMIC_FTRACE
 
 #ifdef CC_USING_MPROFILE_KERNEL
diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S
index 882628fa6987..70e10251e083 100644
--- a/arch/powerpc/kernel/vmlinux.lds.S
+++ b/arch/powerpc/kernel/vmlinux.lds.S
@@ -277,7 +277,9 @@ SECTIONS
 	}
 
 	.opd : AT(ADDR(.opd) - LOAD_OFFSET) {
+		__start_opd = .;
 		*(.opd)
+		__end_opd = .;
 	}
 
 	. = ALIGN(256);
-- 
2.14.2

[PATCHv3 5/7] parisc64: Add .opd based function descriptor dereference

[Sergey Senozhatsky]

We are moving towards separate kernel and module function descriptor
dereference callbacks. This patch enables it for parisc64.

For pointers that belong to the kernel
-  Added __start_opd and __end_opd pointers, to track the kernel
   .opd section address range;

-  Added dereference_kernel_function_descriptor(). Now we
   will dereference only function pointers that are within
   [__start_opd, __end_opd];

For pointers that belong to a module
-  Added dereference_module_function_descriptor() to handle module
   function descriptor dereference. Now we will dereference only
   pointers that are within [module->opd.start, module->opd.end].

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Signed-off-by: Helge Deller <deller@gmx.de>
Tested-by: Helge Deller <deller@gmx.de> # parisc64
Tested-by: Santosh Sivaraj <santosh@fossix.org> # powerpc64
Acked-by: Michael Ellerman <mpe@ellerman.id.au> # powerpc64
Tested-by: Tony Luck <tony.luck@intel.com> # ia64
---
 arch/parisc/boot/compressed/vmlinux.lds.S |  2 ++
 arch/parisc/include/asm/sections.h        |  2 ++
 arch/parisc/kernel/module.c               | 17 +++++++++++++++++
 arch/parisc/kernel/process.c              |  9 +++++++++
 arch/parisc/kernel/vmlinux.lds.S          |  2 ++
 5 files changed, 32 insertions(+)

diff --git a/arch/parisc/boot/compressed/vmlinux.lds.S b/arch/parisc/boot/compressed/vmlinux.lds.S
index a4ce3314e78e..4ebd4e65524c 100644
--- a/arch/parisc/boot/compressed/vmlinux.lds.S
+++ b/arch/parisc/boot/compressed/vmlinux.lds.S
@@ -29,7 +29,9 @@ SECTIONS
 	. = ALIGN(16);
 	/* Linkage tables */
 	.opd : {
+		__start_opd = .;
 		*(.opd)
+		__end_opd = .;
 	} PROVIDE (__gp = .);
 	.plt : {
 		*(.plt)
diff --git a/arch/parisc/include/asm/sections.h b/arch/parisc/include/asm/sections.h
index 59fbe0067112..845ddc9a3421 100644
--- a/arch/parisc/include/asm/sections.h
+++ b/arch/parisc/include/asm/sections.h
@@ -7,6 +7,8 @@
 #ifdef CONFIG_64BIT
 #undef dereference_function_descriptor
 unsigned long dereference_function_descriptor(unsigned long);
+#undef dereference_kernel_function_descriptor
+unsigned long dereference_kernel_function_descriptor(unsigned long);
 #endif
 
 #endif
diff --git a/arch/parisc/kernel/module.c b/arch/parisc/kernel/module.c
index f1a76935a314..28f89b3dcc11 100644
--- a/arch/parisc/kernel/module.c
+++ b/arch/parisc/kernel/module.c
@@ -66,6 +66,7 @@
 
 #include <asm/pgtable.h>
 #include <asm/unwind.h>
+#include <asm/sections.h>
 
 #if 0
 #define DEBUGP printk
@@ -954,3 +955,19 @@ void module_arch_cleanup(struct module *mod)
 {
 	deregister_unwind_table(mod);
 }
+
+#ifdef CONFIG_64BIT
+unsigned long dereference_module_function_descriptor(struct module *mod,
+						     unsigned long addr)
+{
+	unsigned long start_opd = (Elf64_Addr)mod->core_layout.base +
+				   mod->arch.fdesc_offset;
+	unsigned long end_opd = start_opd +
+				mod->arch.fdesc_count * sizeof(Elf64_Fdesc);
+
+	if (addr < start_opd || addr >= end_opd)
+		return addr;
+
+	return dereference_function_descriptor(addr);
+}
+#endif
diff --git a/arch/parisc/kernel/process.c b/arch/parisc/kernel/process.c
index d350aa913acc..423bbfe90e2b 100644
--- a/arch/parisc/kernel/process.c
+++ b/arch/parisc/kernel/process.c
@@ -276,6 +276,15 @@ unsigned long dereference_function_descriptor(unsigned long ptr)
 		ptr = (unsigned long)p;
 	return ptr;
 }
+
+unsigned long dereference_kernel_function_descriptor(unsigned long addr)
+{
+	if (addr < (unsigned long)__start_opd ||
+			addr >= (unsigned long)__end_opd)
+		return addr;
+
+	return dereference_function_descriptor(addr);
+}
 #endif
 
 static inline unsigned long brk_rnd(void)
diff --git a/arch/parisc/kernel/vmlinux.lds.S b/arch/parisc/kernel/vmlinux.lds.S
index ffe2cbf52d1a..ab030895dd1e 100644
--- a/arch/parisc/kernel/vmlinux.lds.S
+++ b/arch/parisc/kernel/vmlinux.lds.S
@@ -99,7 +99,9 @@ SECTIONS
 	. = ALIGN(16);
 	/* Linkage tables */
 	.opd : {
+		__start_opd = .;
 		*(.opd)
+		__end_opd = .;
 	} PROVIDE (__gp = .);
 	.plt : {
 		*(.plt)
-- 
2.14.2

[PATCHv3 6/7] symbol lookup: use new kernel and module dereference functions

[Sergey Senozhatsky]

Call appropriate function descriptor dereference ARCH callbacks:
- dereference_kernel_function_descriptor() if the pointer is a
  kernel symbol;

- dereference_module_function_descriptor() if the pointer is a
  module symbol.

This patch also removes dereference_function_descriptor() from
'%pF/%pf' vsprintf handler, because it has the same behavior with
'%pS/%ps' now.

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Tested-by: Helge Deller <deller@gmx.de> # parisc64
Tested-by: Santosh Sivaraj <santosh@fossix.org> # powerpc64
Acked-by: Michael Ellerman <mpe@ellerman.id.au> # powerpc64
Tested-by: Tony Luck <tony.luck@intel.com> # ia64
---
 Documentation/printk-formats.txt | 20 ++++++++++----------
 kernel/kallsyms.c                |  1 +
 kernel/module.c                  |  1 +
 lib/vsprintf.c                   |  5 +----
 4 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt
index 361789df51ec..3adbc4fdd482 100644
--- a/Documentation/printk-formats.txt
+++ b/Documentation/printk-formats.txt
@@ -50,26 +50,28 @@ Symbols/Function Pointers
 
 ::
 
+	%pS	versatile_init+0x0/0x110
+	%ps	versatile_init
 	%pF	versatile_init+0x0/0x110
 	%pf	versatile_init
-	%pS	versatile_init+0x0/0x110
 	%pSR	versatile_init+0x9/0x110
 		(with __builtin_extract_return_addr() translation)
-	%ps	versatile_init
 	%pB	prev_fn_of_versatile_init+0x88/0x88
 
-The ``F`` and ``f`` specifiers are for printing function pointers,
-for example, f->func, &gettimeofday. They have the same result as
-``S`` and ``s`` specifiers. But they do an extra conversion on
-ia64, ppc64 and parisc64 architectures where the function pointers
-are actually function descriptors.
-
 The ``S`` and ``s`` specifiers can be used for printing symbols
 from direct addresses, for example, __builtin_return_address(0),
 (void *)regs->ip. They result in the symbol name with (``S``) or
 without (``s``) offsets. If KALLSYMS are disabled then the symbol
 address is printed instead.
 
+Note, that the ``F`` and ``f`` specifiers are identical to ``S`` (``s``)
+and thus deprecated. We have ``F`` and ``f`` because on ia64, ppc64 and
+parisc64 function pointers are indirect and, in fact, are function
+descriptors, which require additional dereferencing before we can lookup
+the symbol. As of now, ``S`` and ``s`` perform dereferencing on those
+platforms (when needed), so ``F`` and ``f`` exist for compatibility
+reasons only.
+
 The ``B`` specifier results in the symbol name with offsets and should be
 used when printing stack backtraces. The specifier takes into
 consideration the effect of compiler optimisations which may occur
@@ -77,8 +79,6 @@ when tail-call``s are used and marked with the noreturn GCC attribute.
 
 Examples::
 
-	printk("Going to call: %pF\n", gettimeofday);
-	printk("Going to call: %pF\n", p->func);
 	printk("%s: called from %pS\n", __func__, (void *)_RET_IP_);
 	printk("%s: called from %pS\n", __func__,
 				(void *)__builtin_return_address(0));
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 127e7cfafa55..e2fc09ea9509 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -322,6 +322,7 @@ const char *kallsyms_lookup(unsigned long addr,
 	if (is_ksym_addr(addr)) {
 		unsigned long pos;
 
+		addr = dereference_kernel_function_descriptor(addr);
 		pos = get_symbol_pos(addr, symbolsize, offset);
 		/* Grab name */
 		kallsyms_expand_symbol(get_symbol_offset(pos),
diff --git a/kernel/module.c b/kernel/module.c
index b792e814150a..63361de377ad 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3948,6 +3948,7 @@ const char *module_address_lookup(unsigned long addr,
 	preempt_disable();
 	mod = __module_address(addr);
 	if (mod) {
+		addr = dereference_module_function_descriptor(mod, addr);
 		if (modname)
 			*modname = mod->name;
 		ret = get_ksymbol(mod, addr, size, offset);
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index bcd906a39010..bf04b4f5d8e7 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -40,7 +40,6 @@
 #include "../mm/internal.h"	/* For the trace_print_flags arrays */
 
 #include <asm/page.h>		/* for PAGE_SIZE */
-#include <asm/sections.h>	/* for dereference_function_descriptor() */
 #include <asm/byteorder.h>	/* cpu_to_le16 */
 
 #include <linux/string_helpers.h>
@@ -1721,10 +1720,8 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 	}
 
 	switch (*fmt) {
-	case 'F':
+	case 'F': /* %pF and %pf are kept for compatibility reasons only */
 	case 'f':
-		ptr = (void *)dereference_function_descriptor((unsigned long)ptr);
-		/* Fallthrough */
 	case 'S':
 	case 's':
 	case 'B':
-- 
2.14.2

[PATCHv3 7/7] checkpatch: add pF/pf deprecation warning

[Sergey Senozhatsky]

We deprecated '%pF/%pf' printk specifiers, since '%pS/%ps' is now smart
enough to handle function pointer dereference on platforms where such
dereference is required.

checkpatch warning example:

WARNING: Deprecated vsprintf pointer extension '%pF' - use %pS instead

Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Signed-off-by: Joe Perches <joe@perches.com>
Cc: Andy Whitcroft <apw@canonical.com>
Tested-by: Helge Deller <deller@gmx.de> # parisc64
Tested-by: Santosh Sivaraj <santosh@fossix.org> # powerpc64
Acked-by: Michael Ellerman <mpe@ellerman.id.au> # powerpc64
Tested-by: Tony Luck <tony.luck@intel.com> # ia64
---
 scripts/checkpatch.pl | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 03eb2551477d..387c453413e0 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -5762,18 +5762,25 @@ sub process {
 		        for (my $count = $linenr; $count <= $lc; $count++) {
 				my $fmt = get_quoted_string($lines[$count - 1], raw_line($count, 0));
 				$fmt =~ s/%%//g;
-				if ($fmt =~ /(\%[\*\d\.]*p(?![\WFfSsBKRraEhMmIiUDdgVCbGNO]).)/) {
+				if ($fmt =~ /(\%[\*\d\.]*p(?![\WSsBKRraEhMmIiUDdgVCbGNO]).)/) {
 					$bad_extension = $1;
 					last;
 				}
 			}
 			if ($bad_extension ne "") {
 				my $stat_real = raw_line($linenr, 0);
+				my $ext_type = "Invalid";
+				my $use = "";
 				for (my $count = $linenr + 1; $count <= $lc; $count++) {
 					$stat_real = $stat_real . "\n" . raw_line($count, 0);
 				}
+				if ($bad_extension =~ /p[Ff]/) {
+					$ext_type = "Deprecated";
+					$use = " - use %pS instead";
+					$use =~ s/pS/ps/ if ($bad_extension =~ /pf/);
+				}
 				WARN("VSPRINTF_POINTER_EXTENSION",
-				     "Invalid vsprintf pointer extension '$bad_extension'\n" . "$here\n$stat_real\n");
+				     "$ext_type vsprintf pointer extension '$bad_extension'$use\n" . "$here\n$stat_real\n");
 			}
 		}
 
-- 
2.14.2

Re: [PATCHv3 1/7] switch dereference_function_descriptor() to `unsigned long'

[Petr Mladek]

On Sat 2017-09-30 11:53:13, Sergey Senozhatsky wrote:
> Convert dereference_function_descriptor() to accept and return
> `unsigned long'. There will be two new ARCH function for kernel
> and module function pointer dereference, which will work with
> `unsigned long', so the patch unifies interfaces.
> 
> Besides, dereference_function_descriptor() mostly work with
> `unsigned long':
> 
> Convert dereference_function_descriptor() users tree-wide.

I am not sure if this is a real win. If I count correctly,
the net result is 6 additional casts in this patch. Many
casts are still needed also in the other patches.


> diff --git a/arch/ia64/include/asm/sections.h b/arch/ia64/include/asm/sections.h
> index 2ab2003698ef..de6bfa1ef8fb 100644
> --- a/arch/ia64/include/asm/sections.h
> +++ b/arch/ia64/include/asm/sections.h
> @@ -27,13 +27,13 @@ extern char __start_unwind[], __end_unwind[];
>  extern char __start_ivt_text[], __end_ivt_text[];
>  
>  #undef dereference_function_descriptor
> -static inline void *dereference_function_descriptor(void *ptr)
> +static inline unsigned long dereference_function_descriptor(unsigned long ptr)

I would also expect that a function called "dereference*" would work with
a pointer. The parameter is called ptr but it is unsigned long.

>  {
> -	struct fdesc *desc = ptr;
> +	struct fdesc *desc = (struct fdesc *)ptr;
>  	void *p;
>  
>  	if (!probe_kernel_address(&desc->ip, p))
> -		ptr = p;
> +		ptr = (unsigned long)p;
>  	return ptr;
>  }
>  
> diff --git a/arch/parisc/mm/init.c b/arch/parisc/mm/init.c
> index 1ca9a2b4239f..06e1b79e2946 100644
> --- a/arch/parisc/mm/init.c
> +++ b/arch/parisc/mm/init.c
> @@ -389,10 +389,10 @@ static void __init setup_bootmem(void)
>  static int __init parisc_text_address(unsigned long vaddr)
>  {
>  	static unsigned long head_ptr __initdata;
> +	unsigned long addr = (unsigned long)&parisc_kernel_start;
>  
>  	if (!head_ptr)
> -		head_ptr = PAGE_MASK & (unsigned long)
> -			dereference_function_descriptor(&parisc_kernel_start);
> +		head_ptr = PAGE_MASK & dereference_function_descriptor(addr);

IMHO, this is harder to read than the original. You need to
search the definition of "addr" and check its manipulation
to understand the meaning.

>  
>  	return core_kernel_text(vaddr) || vaddr == head_ptr;
>  }


To make it clear. All these comments are not a big deal and I do
not want to invalidate all the acked-by and tested-by just because
of them.

But please, consider removing this change if we need to do
another iteration of this patchset. IMHO, there is no real win
and it would just pollute the git history.

Best Regards,
Petr

Re: [PATCHv3 2/7] sections: split dereference_function_descriptor()

[Petr Mladek]

On Sat 2017-09-30 11:53:14, Sergey Senozhatsky wrote:
> There are two format specifiers to print out a pointer in symbolic
> format: '%pS/%ps' and '%pF/%pf'. On most architectures, the two
> mean exactly the same thing, but some architectures (ia64, ppc64,
> parisc64) use an indirect pointer for C function pointers, where
> the function pointer points to a function descriptor (which in
> turn contains the actual pointer to the code). The '%pF/%pf, when
> used appropriately, automatically does the appropriate function
> descriptor dereference on such architectures.
> 
> The "when used appropriately" part is tricky. Basically this is
> a subtle ABI detail, specific to some platforms, that made it to
> the API level and people can be unaware of it and miss the whole
> "we need to dereference the function" business out. [1] proves
> that point (note that it fixes only '%pF' and '%pS', there might
> be '%pf' and '%ps' cases as well).
> 
> It appears that we can handle everything within the affected
> arches and make '%pS/%ps' smart enough to retire '%pF/%pf'.
> Function descriptors live in .opd elf section and all affected
> arches (ia64, ppc64, parisc64) handle it properly for kernel
> and modules. So we, technically, can decide if the dereference
> is needed by simply looking at the pointer: if it belongs to
> .opd section then we need to dereference it.

Great catch. I really like this approach!

> diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
> index e5da44eddd2f..387f22c41e0d 100644
> --- a/include/asm-generic/sections.h
> +++ b/include/asm-generic/sections.h
> @@ -29,6 +29,7 @@
>   *	__ctors_start, __ctors_end
>   *	__irqentry_text_start, __irqentry_text_end
>   *	__softirqentry_text_start, __softirqentry_text_end
> + *	__start_opd, __end_opd
>   */
>  extern char _text[], _stext[], _etext[];
>  extern char _data[], _sdata[], _edata[];
> @@ -47,12 +48,15 @@ extern char __softirqentry_text_start[], __softirqentry_text_end[];
>  /* Start and end of .ctors section - used for constructor calls. */
>  extern char __ctors_start[], __ctors_end[];
>  
> +/* Start and end of .opd section - used for function descriptors. */
> +extern char __start_opd[], __end_opd[];
> +
>  extern __visible const void __nosave_begin, __nosave_end;
>  
> -/* function descriptor handling (if any).  Override
> - * in asm/sections.h */
> +/* Function descriptor handling (if any).  Override in asm/sections.h */
>  #ifndef dereference_function_descriptor
>  #define dereference_function_descriptor(p) (p)
> +#define dereference_kernel_function_descriptor(p) (p)
>  #endif
>  
>  /* random extra sections (if any).  Override
> diff --git a/include/linux/moduleloader.h b/include/linux/moduleloader.h
> index 4d0cb9bba93e..172904e9cded 100644
> --- a/include/linux/moduleloader.h
> +++ b/include/linux/moduleloader.h
> @@ -85,6 +85,10 @@ void module_arch_cleanup(struct module *mod);
>  /* Any cleanup before freeing mod->module_init */
>  void module_arch_freeing_init(struct module *mod);
>  
> +/* Dereference module function descriptor */
> +unsigned long dereference_module_function_descriptor(struct module *mod,
> +						     unsigned long addr);
> +

The function is used when the module is already loaded. IMHO,
include/linux/module.h would be a better place.

One advantage would be that we could use the same trick
as in include/asm-generic/sections.h. I mean:

#define dereference_module_function_descriptor(mod, addr) (addr)

and redefine it in the three affected
arch/<arch>/include/asm/module.h headers. Then it might be completely
optimized out on all architectures.

Anyway, we need to get ack from Jessica for this change.

Best Regards,
Petr

Re: [PATCHv3 3/7] ia64: Add .opd based function descriptor dereference

[Petr Mladek]

On Sat 2017-09-30 11:53:15, Sergey Senozhatsky wrote:
> We are moving towards separate kernel and module function descriptor
> dereference callbacks. This patch enables it for IA64.
> 
> For pointers that belong to the kernel
> -  Added __start_opd and __end_opd pointers, to track the kernel
>    .opd section address range;
> 
> -  Added dereference_kernel_function_descriptor(). Now we
>    will dereference only function pointers that are within
>    [__start_opd, __end_opd];
> 
> For pointers that belong to a module
> -  Added dereference_module_function_descriptor() to handle module
>    function descriptor dereference. Now we will dereference only
>    pointers that are within [module->opd.start, module->opd.end].
> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> Tested-by: Helge Deller <deller@gmx.de> # parisc64
> Tested-by: Santosh Sivaraj <santosh@fossix.org> # powerpc64
> Acked-by: Michael Ellerman <mpe@ellerman.id.au> # powerpc64
> Tested-by: Tony Luck <tony.luck@intel.com> # ia64

Looks good to me.

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

Re: [PATCHv3 4/7] powerpc64: Add .opd based function descriptor dereference

[Petr Mladek]

On Sat 2017-09-30 11:53:16, Sergey Senozhatsky wrote:
> We are moving towards separate kernel and module function descriptor
> dereference callbacks. This patch enables it for powerpc64.
> 
> For pointers that belong to the kernel
> -  Added __start_opd and __end_opd pointers, to track the kernel
>    .opd section address range;
> 
> -  Added dereference_kernel_function_descriptor(). Now we
>    will dereference only function pointers that are within
>    [__start_opd, __end_opd];
> 
> For pointers that belong to a module
> -  Added dereference_module_function_descriptor() to handle module
>    function descriptor dereference. Now we will dereference only
>    pointers that are within [module->opd.start, module->opd.end].
> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> Tested-by: Helge Deller <deller@gmx.de> # parisc64
> Tested-by: Santosh Sivaraj <santosh@fossix.org> # powerpc64
> Acked-by: Michael Ellerman <mpe@ellerman.id.au> # powerpc64
> Tested-by: Tony Luck <tony.luck@intel.com> # ia64
> ---
>  arch/powerpc/include/asm/module.h   |  3 +++
>  arch/powerpc/include/asm/sections.h | 11 +++++++++++
>  arch/powerpc/kernel/module_64.c     | 16 ++++++++++++++++
>  arch/powerpc/kernel/vmlinux.lds.S   |  2 ++
>  4 files changed, 32 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/module.h b/arch/powerpc/include/asm/module.h
> index 6c0132c7212f..7e28442827f1 100644
> --- a/arch/powerpc/include/asm/module.h
> +++ b/arch/powerpc/include/asm/module.h
> @@ -45,6 +45,9 @@ struct mod_arch_specific {
>  	unsigned long tramp;
>  #endif
>  
> +	/* For module function descriptor dereference */
> +	unsigned long start_opd;
> +	unsigned long end_opd;
>  #else /* powerpc64 */
>  	/* Indices of PLT sections within module. */
>  	unsigned int core_plt_section;
> diff --git a/arch/powerpc/include/asm/sections.h b/arch/powerpc/include/asm/sections.h
> index 67379b8945e8..6b4ee0d1645f 100644
> --- a/arch/powerpc/include/asm/sections.h
> +++ b/arch/powerpc/include/asm/sections.h
> @@ -75,6 +75,17 @@ static inline unsigned long dereference_function_descriptor(unsigned long ptr)
>  		ptr = (unsigned long)p;
>  	return ptr;
>  }
> +
> +#undef dereference_kernel_function_descriptor
> +static inline unsigned long
> +dereference_kernel_function_descriptor(unsigned long addr)
> +{
> +	if (addr < (unsigned long)__start_opd ||
> +			addr >= (unsigned long)__end_opd)
> +		return addr;
> +
> +	return dereference_function_descriptor(addr);
> +}
>  #endif /* PPC64_ELF_ABI_v1 */
>  
>  #endif
> diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
> index 0b0f89685b67..94caec045a90 100644
> --- a/arch/powerpc/kernel/module_64.c
> +++ b/arch/powerpc/kernel/module_64.c
> @@ -344,6 +344,11 @@ int module_frob_arch_sections(Elf64_Ehdr *hdr,
>  		else if (strcmp(secstrings+sechdrs[i].sh_name,"__versions")==0)
>  			dedotify_versions((void *)hdr + sechdrs[i].sh_offset,
>  					  sechdrs[i].sh_size);
> +		else if (!strcmp(secstrings + sechdrs[i].sh_name, ".opd")) {
> +			me->arch.start_opd = sechdrs[i].sh_addr;
> +			me->arch.end_opd = sechdrs[i].sh_addr +
> +					   sechdrs[i].sh_size;
> +		}
>  
>  		/* We don't handle .init for the moment: rename to _init */
>  		while ((p = strstr(secstrings + sechdrs[i].sh_name, ".init")))
> @@ -712,6 +717,17 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
>  	return 0;
>  }
>  
> +#ifdef PPC64_ELF_ABI_v1
> +unsigned long dereference_module_function_descriptor(struct module *mod,
> +						     unsigned long addr)
> +{
> +	if (addr < mod->arch.start_opd || addr >= mod->arch.end_opd)
> +		return addr;
> +
> +	return dereference_function_descriptor(addr);
> +}
> +#endif /* PPC64_ELF_ABI_v1 */

I would personally move this up in the source file. It is related to
the definition of func_desc() and other functions that are
also PPC_ELF_ABI-specific.

Otherwise, it looks good to me.

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

Re: [PATCHv3 5/7] parisc64: Add .opd based function descriptor dereference

[Petr Mladek]

On Sat 2017-09-30 11:53:17, Sergey Senozhatsky wrote:
> We are moving towards separate kernel and module function descriptor
> dereference callbacks. This patch enables it for parisc64.
> 
> For pointers that belong to the kernel
> -  Added __start_opd and __end_opd pointers, to track the kernel
>    .opd section address range;
> 
> -  Added dereference_kernel_function_descriptor(). Now we
>    will dereference only function pointers that are within
>    [__start_opd, __end_opd];
> 
> For pointers that belong to a module
> -  Added dereference_module_function_descriptor() to handle module
>    function descriptor dereference. Now we will dereference only
>    pointers that are within [module->opd.start, module->opd.end].

> diff --git a/arch/parisc/kernel/module.c b/arch/parisc/kernel/module.c
> index f1a76935a314..28f89b3dcc11 100644
> --- a/arch/parisc/kernel/module.c
> +++ b/arch/parisc/kernel/module.c
> @@ -954,3 +955,19 @@ void module_arch_cleanup(struct module *mod)
>  {
>  	deregister_unwind_table(mod);
>  }
> +
> +#ifdef CONFIG_64BIT
> +unsigned long dereference_module_function_descriptor(struct module *mod,
> +						     unsigned long addr)
> +{
> +	unsigned long start_opd = (Elf64_Addr)mod->core_layout.base +
> +				   mod->arch.fdesc_offset;
> +	unsigned long end_opd = start_opd +
> +				mod->arch.fdesc_count * sizeof(Elf64_Fdesc);

I know that this is used in rather slow paths. But it still might
make sense to have these section borders pre-computed and
stored in struct mod_arch_specific. I mean to do similar
thing that we do on powerpc.

Well, we could do this in a followup patch if parisc people
wanted it.


> +	if (addr < start_opd || addr >= end_opd)
> +		return addr;
> +
> +	return dereference_function_descriptor(addr);
> +}
> +#endif

Otherwise the patch looks fine to me.

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

Re: [PATCHv3 4/7] powerpc64: Add .opd based function descriptor dereference

[Michael Ellerman]

Petr Mladek <pmladek@suse.com> writes:
> On Sat 2017-09-30 11:53:16, Sergey Senozhatsky wrote:
>> diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
>> index 0b0f89685b67..94caec045a90 100644
>> --- a/arch/powerpc/kernel/module_64.c
>> +++ b/arch/powerpc/kernel/module_64.c
>> @@ -712,6 +717,17 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
>>  	return 0;
>>  }
>>  
>> +#ifdef PPC64_ELF_ABI_v1
>> +unsigned long dereference_module_function_descriptor(struct module *mod,
>> +						     unsigned long addr)
>> +{
>> +	if (addr < mod->arch.start_opd || addr >= mod->arch.end_opd)
>> +		return addr;
>> +
>> +	return dereference_function_descriptor(addr);
>> +}
>> +#endif /* PPC64_ELF_ABI_v1 */
>
> I would personally move this up in the source file. It is related to
> the definition of func_desc() and other functions that are
> also PPC_ELF_ABI-specific.

Yeah that would be neater. There's already a PPC64_ELF_ABI_v2 block, you
could put this in the else case of that.

But we can do that later if you're not respinning otherwise.

cheers

Re: [PATCHv3 6/7] symbol lookup: use new kernel and module dereference functions

[Petr Mladek]

On Sat 2017-09-30 11:53:18, Sergey Senozhatsky wrote:
> Call appropriate function descriptor dereference ARCH callbacks:
> - dereference_kernel_function_descriptor() if the pointer is a
>   kernel symbol;
> 
> - dereference_module_function_descriptor() if the pointer is a
>   module symbol.
> 
> This patch also removes dereference_function_descriptor() from
> '%pF/%pf' vsprintf handler, because it has the same behavior with
> '%pS/%ps' now.

The description is pretty criptic. It should explain why
the dereference was moved from vsprintf to the symbol lookup
and if it is safe.

Note that kallsyms_lookup() and module_address_lookup() is used
in many other situations.

Also I would not be afraid to repeat description of the big picture
from the 2nd patch.

> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> Tested-by: Helge Deller <deller@gmx.de> # parisc64
> Tested-by: Santosh Sivaraj <santosh@fossix.org> # powerpc64
> Acked-by: Michael Ellerman <mpe@ellerman.id.au> # powerpc64
> Tested-by: Tony Luck <tony.luck@intel.com> # ia64
> ---
>  Documentation/printk-formats.txt | 20 ++++++++++----------
>  kernel/kallsyms.c                |  1 +
>  kernel/module.c                  |  1 +
>  lib/vsprintf.c                   |  5 +----
>  4 files changed, 13 insertions(+), 14 deletions(-)
> 
> diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt
> index 361789df51ec..3adbc4fdd482 100644
> --- a/Documentation/printk-formats.txt
> +++ b/Documentation/printk-formats.txt
> @@ -50,26 +50,28 @@ Symbols/Function Pointers
>  
>  ::
>  
> +	%pS	versatile_init+0x0/0x110
> +	%ps	versatile_init
>  	%pF	versatile_init+0x0/0x110
>  	%pf	versatile_init
> -	%pS	versatile_init+0x0/0x110
>  	%pSR	versatile_init+0x9/0x110
>  		(with __builtin_extract_return_addr() translation)
> -	%ps	versatile_init
>  	%pB	prev_fn_of_versatile_init+0x88/0x88
>  
> -The ``F`` and ``f`` specifiers are for printing function pointers,
> -for example, f->func, &gettimeofday. They have the same result as
> -``S`` and ``s`` specifiers. But they do an extra conversion on
> -ia64, ppc64 and parisc64 architectures where the function pointers
> -are actually function descriptors.
> -
>  The ``S`` and ``s`` specifiers can be used for printing symbols
>  from direct addresses, for example, __builtin_return_address(0),
>  (void *)regs->ip. They result in the symbol name with (``S``) or
>  without (``s``) offsets. If KALLSYMS are disabled then the symbol
>  address is printed instead.

This paragraph makes the feeling that ``S`` is still only for direct
adresses. We should update it as well.


> +Note, that the ``F`` and ``f`` specifiers are identical to ``S`` (``s``)
> +and thus deprecated. We have ``F`` and ``f`` because on ia64, ppc64 and
> +parisc64 function pointers are indirect and, in fact, are function
> +descriptors, which require additional dereferencing before we can lookup
> +the symbol. As of now, ``S`` and ``s`` perform dereferencing on those
> +platforms (when needed), so ``F`` and ``f`` exist for compatibility
> +reasons only.
> +
>  The ``B`` specifier results in the symbol name with offsets and should be
>  used when printing stack backtraces. The specifier takes into
>  consideration the effect of compiler optimisations which may occur
> @@ -77,8 +79,6 @@ when tail-call``s are used and marked with the noreturn GCC attribute.
>  
>  Examples::
>  
> -	printk("Going to call: %pF\n", gettimeofday);
> -	printk("Going to call: %pF\n", p->func);
>  	printk("%s: called from %pS\n", __func__, (void *)_RET_IP_);
>  	printk("%s: called from %pS\n", __func__,
>  				(void *)__builtin_return_address(0));

We should either replace %pF with %pS or remove all examples.
It is strange to keep only half of them.


> diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
> index 127e7cfafa55..e2fc09ea9509 100644
> --- a/kernel/kallsyms.c
> +++ b/kernel/kallsyms.c
> @@ -322,6 +322,7 @@ const char *kallsyms_lookup(unsigned long addr,
>  	if (is_ksym_addr(addr)) {

is_ksym_addr() ignores the special .opd elf sections if
CONFIG_KALLSYMS_ALL is disabled. We should dereference before
this call.

>  		unsigned long pos;
>  
> +		addr = dereference_kernel_function_descriptor(addr);
>  		pos = get_symbol_pos(addr, symbolsize, offset);

I still wonder if doing the dereference in the widely used kallsyms
might cause any regression.

One possible problem is that this function returns "offset".
One might expect that it is offset against "addr" but
it is not if the dereference happens here.

Also get_symbol_pos() is called in several other helpers
but the dereference is done only here. It would be
confusing if for example kallsyms_lookup_size_offset()
and kallsyms_lookup() give different result.

I would feel much more comfortable if we keep the derefenrece
only in vsprintf.


In each case, we need approval from Jessica for the
change in module.c.

Best Regards,
Petr

Re: [PATCHv3 7/7] checkpatch: add pF/pf deprecation warning

[Petr Mladek]

On Sat 2017-09-30 11:53:19, Sergey Senozhatsky wrote:
> We deprecated '%pF/%pf' printk specifiers, since '%pS/%ps' is now smart
> enough to handle function pointer dereference on platforms where such
> dereference is required.
> 
> checkpatch warning example:
> 
> WARNING: Deprecated vsprintf pointer extension '%pF' - use %pS instead
> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
> Signed-off-by: Joe Perches <joe@perches.com>
> Cc: Andy Whitcroft <apw@canonical.com>
> Tested-by: Helge Deller <deller@gmx.de> # parisc64
> Tested-by: Santosh Sivaraj <santosh@fossix.org> # powerpc64
> Acked-by: Michael Ellerman <mpe@ellerman.id.au> # powerpc64
> Tested-by: Tony Luck <tony.luck@intel.com> # ia64

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

Re: [PATCHv3 6/7] symbol lookup: use new kernel and module dereference functions

[Sergey Senozhatsky]

Sorry for the delay and thanks for taking a look.

I'll try to re-spin the patch set by the end of this week/early next
week.


On (10/04/17 13:53), Petr Mladek wrote:
[..]
> Note that kallsyms_lookup() and module_address_lookup() is used
> in many other situations.

we dereference only things that can be dereferenced.
so calling it on already dereferenced address, or address
that does need to be dereferenced is OK.

besides, not all of those "other" places are available on
ppc64, ia64, parisc.

[..]
> > diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
> > index 127e7cfafa55..e2fc09ea9509 100644
> > --- a/kernel/kallsyms.c
> > +++ b/kernel/kallsyms.c
> > @@ -322,6 +322,7 @@ const char *kallsyms_lookup(unsigned long addr,
> >  	if (is_ksym_addr(addr)) {
> 
> is_ksym_addr() ignores the special .opd elf sections if
> CONFIG_KALLSYMS_ALL is disabled. We should dereference before
> this call.

I'll move it.

> >  		unsigned long pos;
> >  
> > +		addr = dereference_kernel_function_descriptor(addr);
> >  		pos = get_symbol_pos(addr, symbolsize, offset);
> 
> I still wonder if doing the dereference in the widely used kallsyms
> might cause any regression.

more testing wouldn't hurt, yes.

> Also get_symbol_pos() is called in several other helpers
> but the dereference is done only here. It would be
> confusing if for example kallsyms_lookup_size_offset()
> and kallsyms_lookup() give different result.

hm, so there is no change in this regard, right? there was no
deference before, there is no dereference now. what am I missing?


I'm touching the pf/pF part in this patch set. if there are cases
of missing dereferences anywhere else then we need to address it
in a separate patch set, I think.

> I would feel much more comfortable if we keep the derefenrece
> only in vsprintf.

at a price of extra module lookup, because we need `struct module *'
for module address dereference.

	-ss

Re: [PATCHv3 5/7] parisc64: Add .opd based function descriptor dereference

[Sergey Senozhatsky]

On (10/04/17 12:40), Petr Mladek wrote:
> > +unsigned long dereference_module_function_descriptor(struct module *mod,
> > +						     unsigned long addr)
> > +{
> > +	unsigned long start_opd = (Elf64_Addr)mod->core_layout.base +
> > +				   mod->arch.fdesc_offset;
> > +	unsigned long end_opd = start_opd +
> > +				mod->arch.fdesc_count * sizeof(Elf64_Fdesc);
> 
> I know that this is used in rather slow paths. But it still might
> make sense to have these section borders pre-computed and
> stored in struct mod_arch_specific. I mean to do similar
> thing that we do on powerpc.
> 
> Well, we could do this in a followup patch if parisc people
> wanted it.
> 
> 
> > +	if (addr < start_opd || addr >= end_opd)
> > +		return addr;
> > +
> > +	return dereference_function_descriptor(addr);
> > +}
> > +#endif
> 
> Otherwise the patch looks fine to me.
> 
> Reviewed-by: Petr Mladek <pmladek@suse.com>

let's do it later, if need be.

	-ss

Re: [PATCHv3 4/7] powerpc64: Add .opd based function descriptor dereference

[Sergey Senozhatsky]

On (10/04/17 11:21), Petr Mladek wrote:
[..]
> > +#ifdef PPC64_ELF_ABI_v1
> > +unsigned long dereference_module_function_descriptor(struct module *mod,
> > +						     unsigned long addr)
> > +{
> > +	if (addr < mod->arch.start_opd || addr >= mod->arch.end_opd)
> > +		return addr;
> > +
> > +	return dereference_function_descriptor(addr);
> > +}
> > +#endif /* PPC64_ELF_ABI_v1 */
> 
> I would personally move this up in the source file. It is related to
> the definition of func_desc() and other functions that are
> also PPC_ELF_ABI-specific.
> 
> Otherwise, it looks good to me.
> 
> Reviewed-by: Petr Mladek <pmladek@suse.com>

OK, will move.

	-ss

Re: [PATCHv3 2/7] sections: split dereference_function_descriptor()

[Sergey Senozhatsky]

On (10/04/17 11:00), Petr Mladek wrote:
[..]
> >  /* random extra sections (if any).  Override
> > diff --git a/include/linux/moduleloader.h b/include/linux/moduleloader.h
> > index 4d0cb9bba93e..172904e9cded 100644
> > --- a/include/linux/moduleloader.h
> > +++ b/include/linux/moduleloader.h
> > @@ -85,6 +85,10 @@ void module_arch_cleanup(struct module *mod);
> >  /* Any cleanup before freeing mod->module_init */
> >  void module_arch_freeing_init(struct module *mod);
> >  
> > +/* Dereference module function descriptor */
> > +unsigned long dereference_module_function_descriptor(struct module *mod,
> > +						     unsigned long addr);
> > +
> 
> The function is used when the module is already loaded. IMHO,
> include/linux/module.h would be a better place.
> 
> One advantage would be that we could use the same trick
> as in include/asm-generic/sections.h. I mean:
> 
> #define dereference_module_function_descriptor(mod, addr) (addr)
> 
> and redefine it in the three affected
> arch/<arch>/include/asm/module.h headers. Then it might be completely
> optimized out on all architectures.

will take a look.

	-ss

Re: [PATCHv3 1/7] switch dereference_function_descriptor() to `unsigned long'

[Sergey Senozhatsky]

On (10/04/17 10:24), Petr Mladek wrote:
[..]
> To make it clear. All these comments are not a big deal and I do
> not want to invalidate all the acked-by and tested-by just because
> of them.
>
> But please, consider removing this change if we need to do
> another iteration of this patchset. IMHO, there is no real win
> and it would just pollute the git history.

I tend to rather keep it. would that cause any problems on your side?
it saves a ton of ugly casts later in the patch set and lets us to drop
some casts from the modules code/etc. the patch is here because otherwise
I had to add a bunch of new casts.

	-ss

Re: [PATCHv3 4/7] powerpc64: Add .opd based function descriptor dereference

[Sergey Senozhatsky]

Hello,

Michael, sorry for the delay. I'm catching up with the emails
after... absence.

On (10/04/17 22:06), Michael Ellerman wrote:
> Petr Mladek <pmladek@suse.com> writes:
> > On Sat 2017-09-30 11:53:16, Sergey Senozhatsky wrote:
> >> diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
> >> index 0b0f89685b67..94caec045a90 100644
> >> --- a/arch/powerpc/kernel/module_64.c
> >> +++ b/arch/powerpc/kernel/module_64.c
> >> @@ -712,6 +717,17 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
> >>  	return 0;
> >>  }
> >>  
> >> +#ifdef PPC64_ELF_ABI_v1
> >> +unsigned long dereference_module_function_descriptor(struct module *mod,
> >> +						     unsigned long addr)
> >> +{
> >> +	if (addr < mod->arch.start_opd || addr >= mod->arch.end_opd)
> >> +		return addr;
> >> +
> >> +	return dereference_function_descriptor(addr);
> >> +}
> >> +#endif /* PPC64_ELF_ABI_v1 */
> >
> > I would personally move this up in the source file. It is related to
> > the definition of func_desc() and other functions that are
> > also PPC_ELF_ABI-specific.
> 
> Yeah that would be neater. There's already a PPC64_ELF_ABI_v2 block, you
> could put this in the else case of that.
> 
> But we can do that later if you're not respinning otherwise.

will do.

	-ss

Re: [PATCHv3 6/7] symbol lookup: use new kernel and module dereference functions

[Petr Mladek]

On Thu 2017-10-19 15:42:35, Sergey Senozhatsky wrote:
> Sorry for the delay and thanks for taking a look.
> 
> I'll try to re-spin the patch set by the end of this week/early next
> week.
> 
> 
> On (10/04/17 13:53), Petr Mladek wrote:
> [..]
> > Note that kallsyms_lookup() and module_address_lookup() is used
> > in many other situations.
> 
> we dereference only things that can be dereferenced.
> so calling it on already dereferenced address, or address
> that does need to be dereferenced is OK.

My concern is that it changes the behavior. It will suddenly return
another information for addresses that were not dereference before.

> [..]
> > > diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
> > > index 127e7cfafa55..e2fc09ea9509 100644
> > > --- a/kernel/kallsyms.c
> > > +++ b/kernel/kallsyms.c
> > > @@ -322,6 +322,7 @@ const char *kallsyms_lookup(unsigned long addr,
> > >  	if (is_ksym_addr(addr)) {
> > 
> > is_ksym_addr() ignores the special .opd elf sections if
> > CONFIG_KALLSYMS_ALL is disabled. We should dereference before
> > this call.
> 
> I'll move it.
> 
> > >  		unsigned long pos;
> > >  
> > > +		addr = dereference_kernel_function_descriptor(addr);
> > >  		pos = get_symbol_pos(addr, symbolsize, offset);
> > 
> > I still wonder if doing the dereference in the widely used kallsyms
> > might cause any regression.
> 
> more testing wouldn't hurt, yes.
> 
> > Also get_symbol_pos() is called in several other helpers
> > but the dereference is done only here. It would be
> > confusing if for example kallsyms_lookup_size_offset()
> > and kallsyms_lookup() give different result.
> 
> hm, so there is no change in this regard, right? there was no
> deference before, there is no dereference now. what am I missing?

But there was no dereference in kallsyms_lookup() before
and there is dereference now.

I mean that both kallsyms_lookup_size_offset() and kallsyms_lookup()
always returned the same @symbolsize and @offset before this patch.
But they might give different results now because kallsyms_lookup()
might be newly working with dereferenced value.

It is non-consistent, unexpected behavior and might cause problems.

> I'm touching the pf/pF part in this patch set. if there are cases
> of missing dereferences anywhere else then we need to address it
> in a separate patch set, I think.

You are changing the behavior of kallsyms_lookup() and introduce
a possible inconsistency in this patchset.

It might be innocent if kallsyms are used only to display
debug messages. But there are even functional dependencies,
for example kallsyms_lookup() is called in ftrace_match_record().

> > I would feel much more comfortable if we keep the derefenrece
> > only in vsprintf.
> 
> at a price of extra module lookup, because we need `struct module *'
> for module address dereference.

It would be more code but it should not be slower. The module lookup
is just hidden in the kallsyms call now.

Another solution would be to add another helper function into kallsyms
that does the dereference and keep the current one as is.

I think that the dereference might make sense even in the kallsyms
code. But we need to make sure that it is safe and consistent.
This complicates review of this patchset.

Best Regards,
Petr

Re: [PATCHv3 1/7] switch dereference_function_descriptor() to `unsigned long'

[Petr Mladek]

On Thu 2017-10-19 15:50:04, Sergey Senozhatsky wrote:
> On (10/04/17 10:24), Petr Mladek wrote:
> [..]
> > To make it clear. All these comments are not a big deal and I do
> > not want to invalidate all the acked-by and tested-by just because
> > of them.
> >
> > But please, consider removing this change if we need to do
> > another iteration of this patchset. IMHO, there is no real win
> > and it would just pollute the git history.
> 
> I tend to rather keep it. would that cause any problems on your side?
> it saves a ton of ugly casts later in the patch set and lets us to drop
> some casts from the modules code/etc. the patch is here because otherwise
> I had to add a bunch of new casts.

OK, let's keep it.

Best Regards,
Petr

Re: [PATCHv3 6/7] symbol lookup: use new kernel and module dereference functions

[Sergey Senozhatsky]

On (10/20/17 15:08), Petr Mladek wrote:
> On Thu 2017-10-19 15:42:35, Sergey Senozhatsky wrote:
> > Sorry for the delay and thanks for taking a look.
> > 
> > I'll try to re-spin the patch set by the end of this week/early next
> > week.
> > 
> > 
> > On (10/04/17 13:53), Petr Mladek wrote:
> > [..]
> > > Note that kallsyms_lookup() and module_address_lookup() is used
> > > in many other situations.
> > 
> > we dereference only things that can be dereferenced.
> > so calling it on already dereferenced address, or address
> > that does need to be dereferenced is OK.
> 
> My concern is that it changes the behavior. It will suddenly return
> another information for addresses that were not dereference before.

OK. I'd be really-really surprised to find out that anyone did
kallsyms_lookup()/module_address_lookup() on func descriptors,
but I understand your concerns. I'll try to keep everything
within vsprintf().

	-ss