diff mbox series

ARM: change link order of $(mmy-y) to avoid veneers

Message ID 20230619143725.57967-1-masahiroy@kernel.org (mailing list archive)
State New, archived
Headers show
Series ARM: change link order of $(mmy-y) to avoid veneers | expand

Commit Message

Masahiro Yamada June 19, 2023, 2:37 p.m. UTC
The kernel compiled with multi_v7_defconfig + CONFIG_KASAN=y +
CONFIG_KASAN_INLINE=y does not boot.

I do not think KASAN is the direct reason of the boot failure.
CONFIG_KASAN_INLINE is just one example configuration that grows the
image size significantly and makes a big distance between function
callers and callees.

I see some veneers for __get_user_* in the bad kernel image. I am
not perfectly clear, but __get_user_* may not work with veneers for
some reasons.

If I move the link order of arch/arm/lib/getuser.S, the veneers are
gone, and the kernel gets working again.

I do not see a good reason that $(mmu-y) must be added to lib-y because
all the code in $(mmu-y) is mandatory. Add it to obj-y to move the code
to lower address.

[1] multi_v7_defconfig (works)

 $ arm-linux-gnueabihf-nm vmlinux | grep __get_user_1
 c072a450 T __get_user_1
 c17ea033 r __kstrtab___get_user_1
 c18119fe r __kstrtabns___get_user_1
 c17c4878 r __ksymtab___get_user_1

[2] multi_v7_defconfig + CONFIG_KASAN_INLINE (does not work)

 $ arm-linux-gnueabihf-nm vmlinux | grep __get_user_1
 c341ec2c T __get_user_1
 c06e3580 t ____get_user_1_veneer
 c0adc6c0 t ____get_user_1_veneer
 c12cf054 t ____get_user_1_veneer
 c43f42cc r __kstrtab___get_user_1
 c441c128 r __kstrtabns___get_user_1
 c43cead8 r __ksymtab___get_user_1

[3] multi_v7_defconfig + CONFIG_KASAN_INLINE + this patch (works)

 $ arm-linux-gnueabihf-nm vmlinux | grep __get_user_1
 c10975b0 T __get_user_1
 c43f42cc r __kstrtab___get_user_1
 c441c128 r __kstrtabns___get_user_1
 c43cead8 r __ksymtab___get_user_1

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 arch/arm/lib/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Masahiro Yamada June 20, 2023, 2:55 a.m. UTC | #1
On Mon, Jun 19, 2023 at 11:37 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> The kernel compiled with multi_v7_defconfig + CONFIG_KASAN=y +
> CONFIG_KASAN_INLINE=y does not boot.
>
> I do not think KASAN is the direct reason of the boot failure.
> CONFIG_KASAN_INLINE is just one example configuration that grows the
> image size significantly and makes a big distance between function
> callers and callees.
>
> I see some veneers for __get_user_* in the bad kernel image. I am
> not perfectly clear, but __get_user_* may not work with veneers for
> some reasons.
>
> If I move the link order of arch/arm/lib/getuser.S, the veneers are
> gone, and the kernel gets working again.
>
> I do not see a good reason that $(mmu-y) must be added to lib-y because
> all the code in $(mmu-y) is mandatory. Add it to obj-y to move the code
> to lower address.
>
> [1] multi_v7_defconfig (works)
>
>  $ arm-linux-gnueabihf-nm vmlinux | grep __get_user_1
>  c072a450 T __get_user_1
>  c17ea033 r __kstrtab___get_user_1
>  c18119fe r __kstrtabns___get_user_1
>  c17c4878 r __ksymtab___get_user_1
>
> [2] multi_v7_defconfig + CONFIG_KASAN_INLINE (does not work)
>
>  $ arm-linux-gnueabihf-nm vmlinux | grep __get_user_1
>  c341ec2c T __get_user_1
>  c06e3580 t ____get_user_1_veneer
>  c0adc6c0 t ____get_user_1_veneer
>  c12cf054 t ____get_user_1_veneer
>  c43f42cc r __kstrtab___get_user_1
>  c441c128 r __kstrtabns___get_user_1
>  c43cead8 r __ksymtab___get_user_1
>
> [3] multi_v7_defconfig + CONFIG_KASAN_INLINE + this patch (works)
>
>  $ arm-linux-gnueabihf-nm vmlinux | grep __get_user_1
>  c10975b0 T __get_user_1
>  c43f42cc r __kstrtab___get_user_1
>  c441c128 r __kstrtabns___get_user_1
>  c43cead8 r __ksymtab___get_user_1
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
>  arch/arm/lib/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
> index 650404be6768..4d092ef87a1d 100644
> --- a/arch/arm/lib/Makefile
> +++ b/arch/arm/lib/Makefile
> @@ -28,7 +28,7 @@ endif
>  # using lib_ here won't override already available weak symbols
>  obj-$(CONFIG_UACCESS_WITH_MEMCPY) += uaccess_with_memcpy.o
>
> -lib-$(CONFIG_MMU) += $(mmu-y)
> +obj-$(CONFIG_MMU) += $(mmu-y)
>
>  ifeq ($(CONFIG_CPU_32v3),y)
>    lib-y        += io-readsw-armv3.o io-writesw-armv3.o
> --
> 2.39.2
>



I also wonder why __get_user* is defined in
arch/arm/lib/ instead of arch/arm/kernel/,
but I might be a convention.

x86 puts it in the lib directory,
  arch/x86/lib/getuser.S
then other architectures copy the code structure.
Nicolas Schier June 20, 2023, 4:14 a.m. UTC | #2
Subject: [PATCH] ARM: change link order of $(mmy-y) to avoid veneers

There is a typo in the subject (mmy-y -> mmu-y).

Kind regards,
Nicolas


On Mon, Jun 19, 2023 at 11:37:25PM +0900, Masahiro Yamada wrote:
> The kernel compiled with multi_v7_defconfig + CONFIG_KASAN=y +
> CONFIG_KASAN_INLINE=y does not boot.
> 
> I do not think KASAN is the direct reason of the boot failure.
> CONFIG_KASAN_INLINE is just one example configuration that grows the
> image size significantly and makes a big distance between function
> callers and callees.
> 
> I see some veneers for __get_user_* in the bad kernel image. I am
> not perfectly clear, but __get_user_* may not work with veneers for
> some reasons.
> 
> If I move the link order of arch/arm/lib/getuser.S, the veneers are
> gone, and the kernel gets working again.
> 
> I do not see a good reason that $(mmu-y) must be added to lib-y because
> all the code in $(mmu-y) is mandatory. Add it to obj-y to move the code
> to lower address.
> 
> [1] multi_v7_defconfig (works)
> 
>  $ arm-linux-gnueabihf-nm vmlinux | grep __get_user_1
>  c072a450 T __get_user_1
>  c17ea033 r __kstrtab___get_user_1
>  c18119fe r __kstrtabns___get_user_1
>  c17c4878 r __ksymtab___get_user_1
> 
> [2] multi_v7_defconfig + CONFIG_KASAN_INLINE (does not work)
> 
>  $ arm-linux-gnueabihf-nm vmlinux | grep __get_user_1
>  c341ec2c T __get_user_1
>  c06e3580 t ____get_user_1_veneer
>  c0adc6c0 t ____get_user_1_veneer
>  c12cf054 t ____get_user_1_veneer
>  c43f42cc r __kstrtab___get_user_1
>  c441c128 r __kstrtabns___get_user_1
>  c43cead8 r __ksymtab___get_user_1
> 
> [3] multi_v7_defconfig + CONFIG_KASAN_INLINE + this patch (works)
> 
>  $ arm-linux-gnueabihf-nm vmlinux | grep __get_user_1
>  c10975b0 T __get_user_1
>  c43f42cc r __kstrtab___get_user_1
>  c441c128 r __kstrtabns___get_user_1
>  c43cead8 r __ksymtab___get_user_1
> 
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
> 
>  arch/arm/lib/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
> index 650404be6768..4d092ef87a1d 100644
> --- a/arch/arm/lib/Makefile
> +++ b/arch/arm/lib/Makefile
> @@ -28,7 +28,7 @@ endif
>  # using lib_ here won't override already available weak symbols
>  obj-$(CONFIG_UACCESS_WITH_MEMCPY) += uaccess_with_memcpy.o
>  
> -lib-$(CONFIG_MMU) += $(mmu-y)
> +obj-$(CONFIG_MMU) += $(mmu-y)
>  
>  ifeq ($(CONFIG_CPU_32v3),y)
>    lib-y	+= io-readsw-armv3.o io-writesw-armv3.o
> -- 
> 2.39.2
>
Nick Desaulniers June 20, 2023, 7:13 p.m. UTC | #3
On Mon, Jun 19, 2023 at 10:37 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> The kernel compiled with multi_v7_defconfig + CONFIG_KASAN=y +
> CONFIG_KASAN_INLINE=y does not boot.
>
> I do not think KASAN is the direct reason of the boot failure.
> CONFIG_KASAN_INLINE is just one example configuration that grows the
> image size significantly and makes a big distance between function
> callers and callees.
>
> I see some veneers for __get_user_* in the bad kernel image. I am
> not perfectly clear, but __get_user_* may not work with veneers for
> some reasons.

I'm kind of curious to know more about this.  Has there been other
instances in the ARCH=arm port where "there must not be a veneer from
X to Y for reason Z?"

I thought the linker inserted veneers were meant to be transparent
here.  If you disassemble ____get_user_1_veneer, do they themselves
branch to different symbols, or the same symbol? (Perhaps they
trampoline to each other then the final one is the "call" to the
original symbol).  But perhaps the symbol at the end of the chain
gives us more clues. I'd bet it's the KASAN callback, though does
KASAN_INLINE inline those?  Perhaps the veneer is corrupting a
register?  Maybe inline asm in the caller is missing a clobber for
that register...

>
> If I move the link order of arch/arm/lib/getuser.S, the veneers are
> gone, and the kernel gets working again.
>
> I do not see a good reason that $(mmu-y) must be added to lib-y because
> all the code in $(mmu-y) is mandatory. Add it to obj-y to move the code
> to lower address.
>
> [1] multi_v7_defconfig (works)
>
>  $ arm-linux-gnueabihf-nm vmlinux | grep __get_user_1
>  c072a450 T __get_user_1
>  c17ea033 r __kstrtab___get_user_1
>  c18119fe r __kstrtabns___get_user_1
>  c17c4878 r __ksymtab___get_user_1
>
> [2] multi_v7_defconfig + CONFIG_KASAN_INLINE (does not work)
>
>  $ arm-linux-gnueabihf-nm vmlinux | grep __get_user_1
>  c341ec2c T __get_user_1
>  c06e3580 t ____get_user_1_veneer
>  c0adc6c0 t ____get_user_1_veneer
>  c12cf054 t ____get_user_1_veneer
>  c43f42cc r __kstrtab___get_user_1
>  c441c128 r __kstrtabns___get_user_1
>  c43cead8 r __ksymtab___get_user_1
>
> [3] multi_v7_defconfig + CONFIG_KASAN_INLINE + this patch (works)
>
>  $ arm-linux-gnueabihf-nm vmlinux | grep __get_user_1
>  c10975b0 T __get_user_1
>  c43f42cc r __kstrtab___get_user_1
>  c441c128 r __kstrtabns___get_user_1
>  c43cead8 r __ksymtab___get_user_1
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
>  arch/arm/lib/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
> index 650404be6768..4d092ef87a1d 100644
> --- a/arch/arm/lib/Makefile
> +++ b/arch/arm/lib/Makefile
> @@ -28,7 +28,7 @@ endif
>  # using lib_ here won't override already available weak symbols
>  obj-$(CONFIG_UACCESS_WITH_MEMCPY) += uaccess_with_memcpy.o
>
> -lib-$(CONFIG_MMU) += $(mmu-y)
> +obj-$(CONFIG_MMU) += $(mmu-y)
>
>  ifeq ($(CONFIG_CPU_32v3),y)
>    lib-y        += io-readsw-armv3.o io-writesw-armv3.o
> --
> 2.39.2
>
diff mbox series

Patch

diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
index 650404be6768..4d092ef87a1d 100644
--- a/arch/arm/lib/Makefile
+++ b/arch/arm/lib/Makefile
@@ -28,7 +28,7 @@  endif
 # using lib_ here won't override already available weak symbols
 obj-$(CONFIG_UACCESS_WITH_MEMCPY) += uaccess_with_memcpy.o
 
-lib-$(CONFIG_MMU) += $(mmu-y)
+obj-$(CONFIG_MMU) += $(mmu-y)
 
 ifeq ($(CONFIG_CPU_32v3),y)
   lib-y	+= io-readsw-armv3.o io-writesw-armv3.o