linux-kernel - Re: [PATCH] dump_stack: convert generic dump

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]

Date:   Mon, 5 Mar 2018 15:48:46 +0100
From:   Petr Mladek <pmladek@...e.com>
To:     Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
Cc:     Tejun Heo <tj@...nel.org>, Steven Rostedt <rostedt@...dmis.org>,
        Dave Young <dyoung@...hat.com>,
        Andi Kleen <ak@...ux.intel.com>,
        Greentime Hu <green.hu@...il.com>,
        Vincent Chen <deanbo422@...il.com>,
        Arnd Bergmann <arnd@...db.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Stephen Rothwell <sfr@...b.auug.org.au>,
        adi-buildroot-devel@...ts.sourceforge.net,
        linux-kernel@...r.kernel.org,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>
Subject: Re: [PATCH] dump_stack: convert generic dump_stack into a weak symbol

On Mon 2018-03-05 14:37:42, Sergey Senozhatsky wrote:
> We want to move dump_stack related functions out of printk C
> code and consolidate them in lib/dump_stack file. The reason why
> dump_stack_print_info()/etc ended up in printk.c was a "generic"
> (dummy) lib dump_stack() function, which archs can override.
> For example, blackfin and nds32, define their own EXPORT_SYMBOL
> dump_stack() functions.
> 
> In case of blackfin that arch-specific dump_stack() symbol invokes
> a global dump_stack_print_info() function. So we can't easily move
> dump_stack_print_info() to lib/dump_stack, because this way we will
> link with lib/dump_stack.o file, which will bring in a generic
> dump_stack() symbol with it, causing a multiple definitions error:
>   - one dump_stack() from arch/blackfin/dumpstack
>   - the other one from lib/dump_stack
> 
> Convert generic dump_stack() into a weak symbol. So we will be
> able link with lib/dump_stack and at the same time archs will
> be able to override dump_stack(). It also opens up a way for us
> to move dump_stack_set_arch_desc(), dump_stack_print_info() and
> show_regs_print_info() to lib/dump_stack.
> 
> Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@...il.com>
> ---
>  arch/blackfin/kernel/dumpstack.c | 1 -
>  arch/nds32/kernel/traps.c        | 2 --
>  lib/dump_stack.c                 | 4 ++--
>  3 files changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/blackfin/kernel/dumpstack.c b/arch/blackfin/kernel/dumpstack.c
> index 3c992c1f8ef2..61af017130cd 100644
> --- a/arch/blackfin/kernel/dumpstack.c
> +++ b/arch/blackfin/kernel/dumpstack.c
> @@ -174,4 +174,3 @@ void dump_stack(void)
>  	show_stack(current, &stack);
>  	trace_buffer_restore(tflags);
>  }
> -EXPORT_SYMBOL(dump_stack);

I was afraid that blackfin modules would not longer be able
to use arch-specific dump_stack symbol. But it seems that only
the symbol name is important. Alos the list of symbols look
promissing:

before:

$> objdump -x vmlinux | less | grep dump_stack
00248530 l     O __ksymtab      00000008 ___ksymtab_dump_stack
002500d4 l     O __ksymtab_strings      0000000c ___kstrtab_dump_stack
00272bb6 l     O .bss   00000080 _dump_stack_arch_desc_str
000051a8 g     F .text  00000042 _dump_stack
002ab05c g     F .init.text     0000002a _dump_stack_set_arch_desc
0003051c g     F .text  000000a4 _dump_stack_print_info


after:

$> objdump -x vmlinux.patched | less | grep dump_stack
00000000 l    df *ABS*  00000000 lib/dump_stack.c
0027c3e8 l     O .bss   00000080 _dump_stack_arch_desc_str
00248580 l     O __ksymtab      00000008 ___ksymtab_dump_stack
002653d4 l     O __ksymtab_strings      0000000c ___kstrtab_dump_stack
000051a8 g     F .text  00000042 _dump_stack
002b69dc g     F .init.text     0000002a _dump_stack_set_arch_desc
001c2a90 g     F .text  000000a4 _dump_stack_print_info

I hope that I did not miss anything. I could not try this at
runtime. I could just cross-compile.

Anyway, from my side:

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

I'll wait a bit and push it into printk.git for-4.17.


Greentime Hu, you tested this on nds32. Could I use your Tested-by,
please?

Best Regards,
Petr