linux-kernel - Re: [PATCHv3 2/7] sections: split dereference_function

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20171004090043.GD20084@pathway.suse.cz>
Date:   Wed, 4 Oct 2017 11:00:43 +0200
From:   Petr Mladek <pmladek@...e.com>
To:     Sergey Senozhatsky <sergey.senozhatsky@...il.com>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        Tony Luck <tony.luck@...el.com>,
        Fenghua Yu <fenghua.yu@...el.com>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Paul Mackerras <paulus@...ba.org>,
        Michael Ellerman <mpe@...erman.id.au>,
        James Bottomley <jejb@...isc-linux.org>,
        Helge Deller <deller@....de>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Jessica Yu <jeyu@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        linux-ia64@...r.kernel.org, linux-parisc@...r.kernel.org,
        linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCHv3 2/7] sections: split dereference_function_descriptor()

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