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

[Sergey Senozhatsky]

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