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:   Wed, 7 Mar 2018 17:44:27 +0800
From:   Greentime Hu <green.hu@...il.com>
To:     Petr Mladek <pmladek@...e.com>
Cc:     Arnd Bergmann <arnd@...db.de>,
        Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
        Tejun Heo <tj@...nel.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Dave Young <dyoung@...hat.com>,
        Andi Kleen <ak@...ux.intel.com>,
        Vincent Chen <deanbo422@...il.com>,
        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 Mailing List <linux-kernel@...r.kernel.org>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>
Subject: Re: [PATCH] dump_stack: convert generic dump_stack into a weak symbol

2018-03-07 17:09 GMT+08:00 Petr Mladek <pmladek@...e.com>:
> Ah, there was a mid-air collision. Arnd already answered most of my
> questions and even more.
>
> On Wed 2018-03-07 09:46:27, Arnd Bergmann wrote:
>> On Wed, Mar 7, 2018 at 3:21 AM, Sergey Senozhatsky
>> <sergey.senozhatsky.work@...il.com> wrote:
>> > On (03/06/18 14:27), Arnd Bergmann wrote:
>> >> Weak symbols are generally discouraged in the kernel. We have
>> >> them in a couple of places, but I find them rather confusing as they
>> >> make it harder to figure out what is actually going on.
>> >
>> > Honestly, I kind of find __weak less confusing than EXPORT_SYMBOL(dump_stack)
>> > in 3 different places. __weak hints that the symbol likely will be overridden
>> > somewhere, while EXPORT_SYMBOL() does not (at least not to me). Dunno.
>>
>> It's not either/or, they are both wrong ;-)
>>
>> The EXPORT_SYMBOL() is not the thing that makes it work. The duplicate
>> declaration today only works because lib/dump_stack.o is listed as lib-y
>> in the Makefile instead of obj-y. On blackfin and nds32, this causes the entire
>> file to just not be included in the final vmlinux, because there are no
>> references to it.
>>
>> With your patch, I would actually expect the lib/dump_stack.o file
>> to still not be picked up, so now you have a missing EXPORT_SYMBOL()
>> on the two unusual architectures until the point when you add another
>> (referenced) symbol to it.
>
> Great catch! We should change it from lib-y to obj-y. Of course,
> the best solution would be if nds32 uses the generic implementation
> and we could avoid adding __weak.
>

I agree too.
I think I can add a patch to remove the dump_stack() implementation in
nds32 and use the generic one.

diff --git a/arch/nds32/kernel/traps.c b/arch/nds32/kernel/traps.c
index 8828b4aeb72b..6e34eb9824a4 100644
--- a/arch/nds32/kernel/traps.c
+++ b/arch/nds32/kernel/traps.c
@@ -156,18 +156,6 @@ static void __dump(struct task_struct *tsk,
unsigned long *base_reg)
        pr_emerg("\n");
 }

-void dump_stack(void)
-{
-       unsigned long *base_reg;
-       if (!IS_ENABLED(CONFIG_FRAME_POINTER))
-               __asm__ __volatile__("\tori\t%0, $sp, #0\n":"=r"(base_reg));
-       else
-               __asm__ __volatile__("\tori\t%0, $fp, #0\n":"=r"(base_reg));
-       __dump(NULL, base_reg);
-}
-
-EXPORT_SYMBOL(dump_stack);
-