On Tue, Apr 24, 2018 at 3:34 PM, Will Deacon will.deacon@arm.com wrote:
I've not run into any build issues here -- is this specifically with some out-of-tree module?
I received a bug report email about this. I'm not sure which specific module, and I assumed from the email that it was actually a result of in-tree configuration options rather than an out-of-tree module, but I'm not sure exactly. Either way, I was able to reproduce the problem by coding up a little PoC out-of-tree module, so it is certainly a real problem.
It would be better not to introduce a new header file just for this, I think. How about compiler.h instead?
I could, but actually after I wrote this email I noticed that this is a widespread convention:
zx2c4@thinkpad ~/Projects/linux $ subfind asm-prototypes ./arch/s390/include/asm/asm-prototypes.h ./arch/alpha/include/asm/asm-prototypes.h ./arch/powerpc/include/asm/asm-prototypes.h ./arch/m68k/include/asm/asm-prototypes.h ./arch/mips/include/asm/asm-prototypes.h ./arch/x86/include/asm/asm-prototypes.h ./arch/sparc/include/asm/asm-prototypes.h ./arch/ia64/include/asm/asm-prototypes.h ./arch/um/include/asm/asm-prototypes.h ./include/asm-generic/asm-prototypes.h
We normally export asm symbols via arm64ksyms.c. In fact, would doing that remove the need for the explicit declarations completely?
I'm pretty sure it still needs the declaration; otherwise the module hashing will get confused. Also, the EXPORT_SYMBOL macro is a different one when called from assembly versus from C, though not sure that makes a substantive difference. It seems like this is what other architectures are doing:
zx2c4@thinkpad ~/Projects/linux $ rg 'EXPORT_SYMBOL.*__.*[std]i[0-9]' -g '*.S' arch/m68k/lib/modsi3.S 111: EXPORT_SYMBOL(__modsi3)
arch/m68k/lib/umodsi3.S 108: EXPORT_SYMBOL(__umodsi3)
arch/m68k/lib/udivsi3.S 157: EXPORT_SYMBOL(__udivsi3)
arch/m68k/lib/divsi3.S 123: EXPORT_SYMBOL(__divsi3)
arch/m68k/lib/mulsi3.S 105: EXPORT_SYMBOL(__mulsi3)
arch/powerpc/kernel/misc_32.S 529:EXPORT_SYMBOL(__ashrdi3) 541:EXPORT_SYMBOL(__ashldi3) 553:EXPORT_SYMBOL(__lshrdi3) 569:EXPORT_SYMBOL(__cmpdi2) 584:EXPORT_SYMBOL(__ucmpdi2) 596:EXPORT_SYMBOL(__bswapdi2)
arch/powerpc/kernel/misc_64.S 211:EXPORT_SYMBOL(__bswapdi2)
arch/sparc/lib/lshrdi3.S 30:EXPORT_SYMBOL(__lshrdi3)
arch/sparc/lib/muldi3.S 78:EXPORT_SYMBOL(__muldi3)
arch/sparc/lib/divdi3.S 283:EXPORT_SYMBOL(__divdi3)
arch/sparc/lib/ashrdi3.S 40:EXPORT_SYMBOL(__ashrdi3)
arch/sparc/lib/multi3.S 36:EXPORT_SYMBOL(__multi3)
arch/sparc/lib/ashldi3.S 38:EXPORT_SYMBOL(__ashldi3)
On Tue, Apr 24, 2018 at 03:43:04PM +0200, Jason A. Donenfeld wrote:
On Tue, Apr 24, 2018 at 3:34 PM, Will Deacon will.deacon@arm.com wrote:
I've not run into any build issues here -- is this specifically with some out-of-tree module?
I received a bug report email about this. I'm not sure which specific module, and I assumed from the email that it was actually a result of in-tree configuration options rather than an out-of-tree module, but I'm not sure exactly. Either way, I was able to reproduce the problem by coding up a little PoC out-of-tree module, so it is certainly a real problem.
Any chance you could share the module, please? I tried to write one but the compiler just inlines the __in128 arithmetic.
Will
On Tue, Apr 24, 2018 at 5:40 PM, Will Deacon will.deacon@arm.com wrote:
On Tue, Apr 24, 2018 at 03:43:04PM +0200, Jason A. Donenfeld wrote:
On Tue, Apr 24, 2018 at 3:34 PM, Will Deacon will.deacon@arm.com wrote:
I've not run into any build issues here -- is this specifically with some out-of-tree module?
I received a bug report email about this. I'm not sure which specific module, and I assumed from the email that it was actually a result of in-tree configuration options rather than an out-of-tree module, but I'm not sure exactly. Either way, I was able to reproduce the problem by coding up a little PoC out-of-tree module, so it is certainly a real problem.
Any chance you could share the module, please? I tried to write one but the compiler just inlines the __in128 arithmetic.
Sure. I didn't save my original but just cooked a new one up and verified it errors out. The below will get these errors on mainline when compiled as a module, even as an in-tree module:
ERROR: "__lshrti3" [uhohspehgettio.ko] undefined! ERROR: "__ashlti3" [uhohspehgettio.ko.ko] undefined!
#include <linux/module.h>
__uint128_t global1, global2; __uint128_t addrs[8];
void do_something(__uint128_t a, unsigned int idx) { addrs[idx] = a; }
static int __init mod_init(void) { do_something(global1 >> global2, (global2 << global1) & 7); return 0; }
module_init(mod_init); MODULE_LICENSE("GPL v2");
Hi Jason,
On Tue, Apr 24, 2018 at 05:56:39PM +0200, Jason A. Donenfeld wrote:
On Tue, Apr 24, 2018 at 5:40 PM, Will Deacon will.deacon@arm.com wrote:
On Tue, Apr 24, 2018 at 03:43:04PM +0200, Jason A. Donenfeld wrote:
On Tue, Apr 24, 2018 at 3:34 PM, Will Deacon will.deacon@arm.com wrote:
I've not run into any build issues here -- is this specifically with some out-of-tree module?
I received a bug report email about this. I'm not sure which specific module, and I assumed from the email that it was actually a result of in-tree configuration options rather than an out-of-tree module, but I'm not sure exactly. Either way, I was able to reproduce the problem by coding up a little PoC out-of-tree module, so it is certainly a real problem.
Any chance you could share the module, please? I tried to write one but the compiler just inlines the __in128 arithmetic.
Sure. I didn't save my original but just cooked a new one up and verified it errors out. The below will get these errors on mainline when compiled as a module, even as an in-tree module:
Thanks for sharing the module source, I can reproduce the issue on my machine. Putting both the prototype and the EXPORT_SYMBOL in arm64ksyms.c appears to resolve the issue and is my preference since these functions shouldn't be called directly anyway. Would you be able to spin a v2 doing that, please? I can take it as fix.
I've also queued your clang int128 patch.
Cheers,
Will
Otherwise modules that use these arithmetic operations will fail to link. We accomplish this with the usual EXPORT_SYMBOL, which on most architectures goes in the .S file but the ARM64 maintainers prefer that insead it goes into arm64ksyms.
While we're at it, we also fix this up to use SPDX, and I personally choose to relicense this as GPL2||BSD so that these symbols don't need to be export_symbol_gpl, so all modules can use the routines, since these are important general purpose compiler-generated function calls.
Signed-off-by: Jason A. Donenfeld Jason@zx2c4.com Reported-by: PaX Team pageexec@freemail.hu Cc: stable@vger.kernel.org --- arch/arm64/kernel/arm64ksyms.c | 8 ++++++++ arch/arm64/lib/tishift.S | 15 ++------------- 2 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/arch/arm64/kernel/arm64ksyms.c b/arch/arm64/kernel/arm64ksyms.c index 66be504edb6c..d894a20b70b2 100644 --- a/arch/arm64/kernel/arm64ksyms.c +++ b/arch/arm64/kernel/arm64ksyms.c @@ -75,3 +75,11 @@ NOKPROBE_SYMBOL(_mcount); /* arm-smccc */ EXPORT_SYMBOL(__arm_smccc_smc); EXPORT_SYMBOL(__arm_smccc_hvc); + + /* tishift.S */ +extern long long __ashlti3(long long a, int b); +EXPORT_SYMBOL(__ashlti3); +extern long long __ashrti3(long long a, int b); +EXPORT_SYMBOL(__ashrti3); +extern long long __lshrti3(long long a, int b); +EXPORT_SYMBOL(__lshrti3); diff --git a/arch/arm64/lib/tishift.S b/arch/arm64/lib/tishift.S index d3db9b2cd479..0fdff97794de 100644 --- a/arch/arm64/lib/tishift.S +++ b/arch/arm64/lib/tishift.S @@ -1,17 +1,6 @@ -/* - * Copyright (C) 2017 Jason A. Donenfeld Jason@zx2c4.com. All Rights Reserved. +/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program. If not, see http://www.gnu.org/licenses/. + * Copyright (C) 2017-2018 Jason A. Donenfeld Jason@zx2c4.com. All Rights Reserved. */
#include <linux/linkage.h>
linux-stable-mirror@lists.linaro.org