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

[PATCH] arm64: export tishift functions to modules

[Jason A. Donenfeld]

Otherwise modules that use these arithmetic operations will fail to
link. We accomplish this with EXPORT_SYMBOL in the .S file, but because
of symbol versioning, we actually need to have a declaration of these
too in C. So, we introduce asm-prototypes.h, which is the same file name
and technique used for similar reasons in the m68k arch tree.

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/include/asm/asm-prototypes.h | 11 +++++++++++
 arch/arm64/lib/tishift.S                | 19 ++++++-------------
 2 files changed, 17 insertions(+), 13 deletions(-)
 create mode 100644 arch/arm64/include/asm/asm-prototypes.h

diff --git a/arch/arm64/include/asm/asm-prototypes.h b/arch/arm64/include/asm/asm-prototypes.h
new file mode 100644
index 000000000000..8f1919e44f51
--- /dev/null
+++ b/arch/arm64/include/asm/asm-prototypes.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+ *
+ * Copyright (C) 2017-2018 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
+ */
+
+/* These functions are defined in lib/tishift.S, but need to be declared
+ * here so that symbol versioning picks them up.
+ */
+extern long long __ashlti3(long long a, int b);
+extern long long __ashrti3(long long a, int b);
+extern long long __lshrti3(long long a, int b);
diff --git a/arch/arm64/lib/tishift.S b/arch/arm64/lib/tishift.S
index d3db9b2cd479..3bca433973cb 100644
--- a/arch/arm64/lib/tishift.S
+++ b/arch/arm64/lib/tishift.S
@@ -1,20 +1,10 @@
-/*
- * 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>
+#include <asm-generic/export.h>
 
 ENTRY(__ashlti3)
 	cbz	x2, 1f
@@ -36,6 +26,7 @@ ENTRY(__ashlti3)
 	mov	x0, x2
 	ret
 ENDPROC(__ashlti3)
+EXPORT_SYMBOL(__ashlti3)
 
 ENTRY(__ashrti3)
 	cbz	x2, 1f
@@ -57,6 +48,7 @@ ENTRY(__ashrti3)
 	mov	x1, x2
 	ret
 ENDPROC(__ashrti3)
+EXPORT_SYMBOL(__ashrti3)
 
 ENTRY(__lshrti3)
 	cbz	x2, 1f
@@ -78,3 +70,4 @@ ENTRY(__lshrti3)
 	mov	x1, x2
 	ret
 ENDPROC(__lshrti3)
+EXPORT_SYMBOL(__lshrti3)
-- 
2.17.0

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

[Will Deacon]

Hi Jason,

On Sun, Apr 15, 2018 at 06:04:16PM +0200, Jason A. Donenfeld wrote:
> Otherwise modules that use these arithmetic operations will fail to
> link. We accomplish this with EXPORT_SYMBOL in the .S file, but because
> of symbol versioning, we actually need to have a declaration of these
> too in C. So, we introduce asm-prototypes.h, which is the same file name
> and technique used for similar reasons in the m68k arch tree.
> 
> 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/include/asm/asm-prototypes.h | 11 +++++++++++
>  arch/arm64/lib/tishift.S                | 19 ++++++-------------
>  2 files changed, 17 insertions(+), 13 deletions(-)
>  create mode 100644 arch/arm64/include/asm/asm-prototypes.h

I've not run into any build issues here -- is this specifically with some
out-of-tree module?

> diff --git a/arch/arm64/include/asm/asm-prototypes.h b/arch/arm64/include/asm/asm-prototypes.h
> new file mode 100644
> index 000000000000..8f1919e44f51
> --- /dev/null
> +++ b/arch/arm64/include/asm/asm-prototypes.h

It would be better not to introduce a new header file just for this, I
think. How about compiler.h instead?

> @@ -0,0 +1,11 @@
> +/* SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
> + *
> + * Copyright (C) 2017-2018 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
> + */
> +
> +/* These functions are defined in lib/tishift.S, but need to be declared
> + * here so that symbol versioning picks them up.
> + */
> +extern long long __ashlti3(long long a, int b);
> +extern long long __ashrti3(long long a, int b);
> +extern long long __lshrti3(long long a, int b);
> diff --git a/arch/arm64/lib/tishift.S b/arch/arm64/lib/tishift.S
> index d3db9b2cd479..3bca433973cb 100644
> --- a/arch/arm64/lib/tishift.S
> +++ b/arch/arm64/lib/tishift.S
> @@ -1,20 +1,10 @@
> -/*
> - * 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>
> +#include <asm-generic/export.h>
>  
>  ENTRY(__ashlti3)
>  	cbz	x2, 1f
> @@ -36,6 +26,7 @@ ENTRY(__ashlti3)
>  	mov	x0, x2
>  	ret
>  ENDPROC(__ashlti3)
> +EXPORT_SYMBOL(__ashlti3)

We normally export asm symbols via arm64ksyms.c. In fact, would doing that
remove the need for the explicit declarations completely?

Will