linux-kernel - Re: [PATCH v1 00/10] Remove weak function declarations

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: <20141015112727.49a453c3f6cef674fe1aecd3@linux-foundation.org>
Date:	Wed, 15 Oct 2014 11:27:27 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Bjorn Helgaas <bhelgaas@...gle.com>
Cc:	Jason Wessel <jason.wessel@...driver.com>,
	Ralf Baechle <ralf@...ux-mips.org>,
	Ingo Molnar <mingo@...hat.com>,
	John Stultz <john.stultz@...aro.org>,
	Eric Paris <eparis@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	linux-kernel@...r.kernel.org, Joe Perches <joe@...ches.com>
Subject: Re: [PATCH v1 00/10] Remove weak function declarations

On Wed, 15 Oct 2014 11:05:41 -0600 Bjorn Helgaas <bhelgaas@...gle.com> wrote:

> A common usage of "weak" is for a default implementation of a function.
> An architecture that needs something different can supply a non-weak
> ("strong") implementation, with the expectation that the linker will select
> the strong version and discard the weak default version.
> 
> We have a few function declarations in header files annotated as "weak".
> That causes every *every* definition to be marked "weak", which means there
> is no strong version at all.  In this case, the linker selects one of the
> weak versions based on link order.  I don't think this is what we want.
> 
> These patches remove almost all the weak annotations from header files
> (MIPS still uses it for get_c0_compare_int(), apparently relying on the
> fact that a weak symbol need not be defined at all).  In most cases, the
> default implementation was already marked weak at the definition.  When it
> wasn't, I added that.
> 
> It might be simplest if I ask Linus to pull these all as a group from my
> branch [1].  I'll look for acks from the following people.  If I don't see
> an ack, I'll drop the patch and you can take it yourself or ignore it as
> you wish.
> 
>   Eric:                    audit
>   Thomas, Ingo, or Peter:  x86
>   Ralf:                    MIPS
>   John or Thomas:          clocksource
>   Jason:                   kgdb
>   Ingo:                    uprobes
>   Andrew:                  vmcore, memory-hotplug

Acks, of course..

> I don't know whether these fix any actual bugs.  We *did* have a bug like
> this on MIPS a while ago (10629d711ed7 ("PCI: Remove __weak annotation from
> pcibios_get_phb_of_node decl")), so it's possible that they do fix
> something.

I'm rather astonished that we haven't hit problems with this before
now.

This is pretty rude behaviour from the linker, really - grabbing the
first __weak function and using that is very likely to be the wrong
thing to do.

Still, this is a bit of a hand grenade and we should think up some way
of detecting/preventing recurrences.

I guess a checkpatch rule which warns about __weak and
__attribute__((weak)) in a header file would help.  Is there anything
more robust we can do?  Coccinelle, sparse, etc?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/