Re: [PATCH] arm64: export tishift functions to modules

Re: [PATCH] arm64: export tishift functions to modules

[Jason A. Donenfeld]

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)

Re: [PATCH] arm64: export tishift functions to modules

[Will Deacon]

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

Re: [PATCH] arm64: export tishift functions to modules

[Jason A. Donenfeld]

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");

Re: [PATCH] arm64: export tishift functions to modules

[Will Deacon]

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

[PATCH v2] arm64: export tishift functions to modules

[Jason A. Donenfeld]

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