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 09:49:31 +0100
From:   Petr Mladek <pmladek@...e.com>
To:     Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>,
        Arnd Bergmann <arnd@...db.de>
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>,
        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

On Wed 2018-03-07 11:21:27, Sergey Senozhatsky wrote:
> Hello Arnd,
> 
> On (03/06/18 14:27), Arnd Bergmann wrote:
> [..]
> > As we are now removing blackfin, based on the latest discussion, this
> > part should no longer be necessary.
> 
> When is this going to happen? 4.17?
> 
> [..]
> > nds32 currently only exists in linux-next, not in the mainline kernel.
> > If it's the only architecture that does something different from everyone
> > else, I think we should change nds32.
> > 
> > I looked at the nds32 show_stack() implementation now and it
> > seems to me that it is completely unnecessary, as the implementation
> > from lib/dump_stack.c does basically the same thing (by calling
> > show_stack(NULL, NULL)).
> 
> Interesting point. I'll leave it to nds32 maintainers.
> OTOH blackfin is still in linux-next, so I assume we need
> that __weak dump_stack() for the time being.

My understanding is that blacfin will not be removed in the first
wave. Therefore we would need to do something about it for 4.17.
Or is there any new info, Arnd?


> [..]
> > > +asmlinkage __weak __visible void dump_stack(void)
> > >  {
> > >         __dump_stack();
> > >  }
> > 
> > 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.

I agree that using weak symbols might be confusing. But I wonder what
is the preferred alternative when only few architectures do something
slightly different.


> 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.

The trick used for dump_stack() is really confusing. I mean the
linking of lib/dump_stack.o only when there is no arch-specific
variant.

Best Regards,
Petr