[PATCH] kbuild: Fix changing ELF file type for output of gen

patches.lists.linux.dev archive mirror
 help / color / mirror / Atom feed

* [PATCH] kbuild: Fix changing ELF file type for output of gen_btf for big endian
@ 2024-02-08 20:21 Nathan Chancellor
  2024-02-08 21:10 ` Nicolas Schier
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Nathan Chancellor @ 2024-02-08 20:21 UTC (permalink / raw)
  To: masahiroy
  Cc: nicolas, ndesaulniers, morbo, justinstitt, keescook, maskray,
	linux-kbuild, bpf, llvm, patches, stable, Nathan Chancellor

Commit 90ceddcb4950 ("bpf: Support llvm-objcopy for vmlinux BTF")
changed the ELF type of .btf.vmlinux.bin.o from ET_EXEC to ET_REL via
dd, which works fine for little endian platforms:

   00000000  7f 45 4c 46 02 01 01 00  00 00 00 00 00 00 00 00  |.ELF............|
  -00000010  03 00 b7 00 01 00 00 00  00 00 00 80 00 80 ff ff  |................|
  +00000010  01 00 b7 00 01 00 00 00  00 00 00 80 00 80 ff ff  |................|

However, for big endian platforms, it changes the wrong byte, resulting
in an invalid ELF file type, which ld.lld rejects:

   00000000  7f 45 4c 46 02 02 01 00  00 00 00 00 00 00 00 00  |.ELF............|
  -00000010  00 03 00 16 00 00 00 01  00 00 00 00 00 10 00 00  |................|
  +00000010  01 03 00 16 00 00 00 01  00 00 00 00 00 10 00 00  |................|

  Type:                              <unknown>: 103

  ld.lld: error: .btf.vmlinux.bin.o: unknown file type

Fix this by using a different seek value for dd when targeting big
endian, so that the correct byte gets changed and everything works
correctly for all linkers.

   00000000  7f 45 4c 46 02 02 01 00  00 00 00 00 00 00 00 00  |.ELF............|
  -00000010  00 03 00 16 00 00 00 01  00 00 00 00 00 10 00 00  |................|
  +00000010  00 01 00 16 00 00 00 01  00 00 00 00 00 10 00 00  |................|

  Type:                              REL (Relocatable file)

Cc: stable@vger.kernel.org
Fixes: 90ceddcb4950 ("bpf: Support llvm-objcopy for vmlinux BTF")
Link: https://github.com/llvm/llvm-project/pull/75643
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 scripts/link-vmlinux.sh | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index a432b171be82..8a9f48b3cb32 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -135,8 +135,15 @@ gen_btf()
 	${OBJCOPY} --only-section=.BTF --set-section-flags .BTF=alloc,readonly \
 		--strip-all ${1} ${2} 2>/dev/null
 	# Change e_type to ET_REL so that it can be used to link final vmlinux.
-	# Unlike GNU ld, lld does not allow an ET_EXEC input.
-	printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 status=none
+	# Unlike GNU ld, lld does not allow an ET_EXEC input. Make sure the correct
+	# byte gets changed with big endian platforms, otherwise e_type may be an
+	# invalid value.
+	if is_enabled CONFIG_CPU_BIG_ENDIAN; then
+		seek=17
+	else
+		seek=16
+	fi
+	printf '\1' | dd of=${2} conv=notrunc bs=1 seek=${seek} status=none
 }
 
 # Create ${2} .S file with all symbols from the ${1} object file

---
base-commit: 54be6c6c5ae8e0d93a6c4641cb7528eb0b6ba478
change-id: 20240208-fix-elf-type-btf-vmlinux-bin-o-big-endian-dbc55a1e1296

Best regards,
-- 
Nathan Chancellor <nathan@kernel.org>


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] kbuild: Fix changing ELF file type for output of gen_btf for big endian
  2024-02-08 20:21 [PATCH] kbuild: Fix changing ELF file type for output of gen_btf for big endian Nathan Chancellor
@ 2024-02-08 21:10 ` Nicolas Schier
  2024-02-08 21:22   ` Fangrui Song
  2024-02-08 21:50 ` Justin Stitt
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Nicolas Schier @ 2024-02-08 21:10 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: masahiroy, ndesaulniers, morbo, justinstitt, keescook, maskray,
	linux-kbuild, bpf, llvm, patches, stable

[-- Attachment #1: Type: text/plain, Size: 3142 bytes --]

On Thu, Feb 08, 2024 at 01:21:06PM -0700 Nathan Chancellor wrote:
> Commit 90ceddcb4950 ("bpf: Support llvm-objcopy for vmlinux BTF")
> changed the ELF type of .btf.vmlinux.bin.o from ET_EXEC to ET_REL via
> dd, which works fine for little endian platforms:
> 
>    00000000  7f 45 4c 46 02 01 01 00  00 00 00 00 00 00 00 00  |.ELF............|
>   -00000010  03 00 b7 00 01 00 00 00  00 00 00 80 00 80 ff ff  |................|
>   +00000010  01 00 b7 00 01 00 00 00  00 00 00 80 00 80 ff ff  |................|
> 
> However, for big endian platforms, it changes the wrong byte, resulting
> in an invalid ELF file type, which ld.lld rejects:
> 
>    00000000  7f 45 4c 46 02 02 01 00  00 00 00 00 00 00 00 00  |.ELF............|
>   -00000010  00 03 00 16 00 00 00 01  00 00 00 00 00 10 00 00  |................|
>   +00000010  01 03 00 16 00 00 00 01  00 00 00 00 00 10 00 00  |................|
> 
>   Type:                              <unknown>: 103
> 
>   ld.lld: error: .btf.vmlinux.bin.o: unknown file type
> 
> Fix this by using a different seek value for dd when targeting big
> endian, so that the correct byte gets changed and everything works
> correctly for all linkers.
> 
>    00000000  7f 45 4c 46 02 02 01 00  00 00 00 00 00 00 00 00  |.ELF............|
>   -00000010  00 03 00 16 00 00 00 01  00 00 00 00 00 10 00 00  |................|
>   +00000010  00 01 00 16 00 00 00 01  00 00 00 00 00 10 00 00  |................|
> 
>   Type:                              REL (Relocatable file)
> 
> Cc: stable@vger.kernel.org
> Fixes: 90ceddcb4950 ("bpf: Support llvm-objcopy for vmlinux BTF")
> Link: https://github.com/llvm/llvm-project/pull/75643
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
>  scripts/link-vmlinux.sh | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index a432b171be82..8a9f48b3cb32 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -135,8 +135,15 @@ gen_btf()
>  	${OBJCOPY} --only-section=.BTF --set-section-flags .BTF=alloc,readonly \
>  		--strip-all ${1} ${2} 2>/dev/null
>  	# Change e_type to ET_REL so that it can be used to link final vmlinux.
> -	# Unlike GNU ld, lld does not allow an ET_EXEC input.
> -	printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 status=none
> +	# Unlike GNU ld, lld does not allow an ET_EXEC input. Make sure the correct
> +	# byte gets changed with big endian platforms, otherwise e_type may be an
> +	# invalid value.
> +	if is_enabled CONFIG_CPU_BIG_ENDIAN; then
> +		seek=17
> +	else
> +		seek=16
> +	fi
> +	printf '\1' | dd of=${2} conv=notrunc bs=1 seek=${seek} status=none
>  }
>  
>  # Create ${2} .S file with all symbols from the ${1} object file
> 
> ---
> base-commit: 54be6c6c5ae8e0d93a6c4641cb7528eb0b6ba478
> change-id: 20240208-fix-elf-type-btf-vmlinux-bin-o-big-endian-dbc55a1e1296
> 
> Best regards,
> -- 
> Nathan Chancellor <nathan@kernel.org>
> 

Thanks for the verbose examples!
Looks good to me.

Reviewed-by: Nicolas Schier <nicolas@fjasle.eu>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] kbuild: Fix changing ELF file type for output of gen_btf for big endian
  2024-02-08 21:10 ` Nicolas Schier
@ 2024-02-08 21:22   ` Fangrui Song
  0 siblings, 0 replies; 7+ messages in thread
From: Fangrui Song @ 2024-02-08 21:22 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Nicolas Schier, masahiroy, ndesaulniers, morbo, justinstitt,
	keescook, linux-kbuild, bpf, llvm, patches, stable

On Thu, Feb 8, 2024 at 1:11 PM Nicolas Schier <nicolas@fjasle.eu> wrote:
>
> On Thu, Feb 08, 2024 at 01:21:06PM -0700 Nathan Chancellor wrote:
> > Commit 90ceddcb4950 ("bpf: Support llvm-objcopy for vmlinux BTF")
> > changed the ELF type of .btf.vmlinux.bin.o from ET_EXEC to ET_REL via
> > dd, which works fine for little endian platforms:
> >
> >    00000000  7f 45 4c 46 02 01 01 00  00 00 00 00 00 00 00 00  |.ELF............|
> >   -00000010  03 00 b7 00 01 00 00 00  00 00 00 80 00 80 ff ff  |................|
> >   +00000010  01 00 b7 00 01 00 00 00  00 00 00 80 00 80 ff ff  |................|
> >
> > However, for big endian platforms, it changes the wrong byte, resulting
> > in an invalid ELF file type, which ld.lld rejects:
> >
> >    00000000  7f 45 4c 46 02 02 01 00  00 00 00 00 00 00 00 00  |.ELF............|
> >   -00000010  00 03 00 16 00 00 00 01  00 00 00 00 00 10 00 00  |................|
> >   +00000010  01 03 00 16 00 00 00 01  00 00 00 00 00 10 00 00  |................|
> >
> >   Type:                              <unknown>: 103
> >
> >   ld.lld: error: .btf.vmlinux.bin.o: unknown file type
> >
> > Fix this by using a different seek value for dd when targeting big
> > endian, so that the correct byte gets changed and everything works
> > correctly for all linkers.
> >
> >    00000000  7f 45 4c 46 02 02 01 00  00 00 00 00 00 00 00 00  |.ELF............|
> >   -00000010  00 03 00 16 00 00 00 01  00 00 00 00 00 10 00 00  |................|
> >   +00000010  00 01 00 16 00 00 00 01  00 00 00 00 00 10 00 00  |................|
> >
> >   Type:                              REL (Relocatable file)
> >
> > Cc: stable@vger.kernel.org
> > Fixes: 90ceddcb4950 ("bpf: Support llvm-objcopy for vmlinux BTF")
> > Link: https://github.com/llvm/llvm-project/pull/75643
> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > ---
> >  scripts/link-vmlinux.sh | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> > index a432b171be82..8a9f48b3cb32 100755
> > --- a/scripts/link-vmlinux.sh
> > +++ b/scripts/link-vmlinux.sh
> > @@ -135,8 +135,15 @@ gen_btf()
> >       ${OBJCOPY} --only-section=.BTF --set-section-flags .BTF=alloc,readonly \
> >               --strip-all ${1} ${2} 2>/dev/null
> >       # Change e_type to ET_REL so that it can be used to link final vmlinux.
> > -     # Unlike GNU ld, lld does not allow an ET_EXEC input.

From binutils 2.35 onwards, GNU ld also rejects ET_EXEC input. So the
comment can be adjusted.

% file a2
a2: ELF 64-bit LSB executable, x86-64, version 1 (SYSV), statically
linked, not stripped
% ld.bfd a2 -o a3
ld.bfd: cannot use executable file 'a2' as input to a link

> > -     printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 status=none
> > +     # Unlike GNU ld, lld does not allow an ET_EXEC input. Make sure the correct
> > +     # byte gets changed with big endian platforms, otherwise e_type may be an
> > +     # invalid value.
> > +     if is_enabled CONFIG_CPU_BIG_ENDIAN; then
> > +             seek=17
> > +     else
> > +             seek=16
> > +     fi
> > +     printf '\1' | dd of=${2} conv=notrunc bs=1 seek=${seek} status=none
> >  }
> >
> >  # Create ${2} .S file with all symbols from the ${1} object file
> >
> > ---
> > base-commit: 54be6c6c5ae8e0d93a6c4641cb7528eb0b6ba478
> > change-id: 20240208-fix-elf-type-btf-vmlinux-bin-o-big-endian-dbc55a1e1296
> >
> > Best regards,
> > --
> > Nathan Chancellor <nathan@kernel.org>
> >
>
> Thanks for the verbose examples!
> Looks good to me.
>
> Reviewed-by: Nicolas Schier <nicolas@fjasle.eu>

Reviewed-by: Fangrui Song <maskray@google.com>


-- 
宋方睿

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] kbuild: Fix changing ELF file type for output of gen_btf for big endian
  2024-02-08 20:21 [PATCH] kbuild: Fix changing ELF file type for output of gen_btf for big endian Nathan Chancellor
  2024-02-08 21:10 ` Nicolas Schier
@ 2024-02-08 21:50 ` Justin Stitt
  2024-02-10  0:48 ` Kees Cook
  2024-02-13  0:55 ` Masahiro Yamada
  3 siblings, 0 replies; 7+ messages in thread
From: Justin Stitt @ 2024-02-08 21:50 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: masahiroy, nicolas, ndesaulniers, morbo, keescook, maskray,
	linux-kbuild, bpf, llvm, patches, stable

On Thu, Feb 8, 2024 at 12:21 PM Nathan Chancellor <nathan@kernel.org> wrote:
>
> Commit 90ceddcb4950 ("bpf: Support llvm-objcopy for vmlinux BTF")
> changed the ELF type of .btf.vmlinux.bin.o from ET_EXEC to ET_REL via
> dd, which works fine for little endian platforms:
>
>    00000000  7f 45 4c 46 02 01 01 00  00 00 00 00 00 00 00 00  |.ELF............|
>   -00000010  03 00 b7 00 01 00 00 00  00 00 00 80 00 80 ff ff  |................|
>   +00000010  01 00 b7 00 01 00 00 00  00 00 00 80 00 80 ff ff  |................|
>
> However, for big endian platforms, it changes the wrong byte, resulting
> in an invalid ELF file type, which ld.lld rejects:
>
>    00000000  7f 45 4c 46 02 02 01 00  00 00 00 00 00 00 00 00  |.ELF............|
>   -00000010  00 03 00 16 00 00 00 01  00 00 00 00 00 10 00 00  |................|
>   +00000010  01 03 00 16 00 00 00 01  00 00 00 00 00 10 00 00  |................|
>
>   Type:                              <unknown>: 103
>
>   ld.lld: error: .btf.vmlinux.bin.o: unknown file type
>
> Fix this by using a different seek value for dd when targeting big
> endian, so that the correct byte gets changed and everything works
> correctly for all linkers.
>
>    00000000  7f 45 4c 46 02 02 01 00  00 00 00 00 00 00 00 00  |.ELF............|
>   -00000010  00 03 00 16 00 00 00 01  00 00 00 00 00 10 00 00  |................|
>   +00000010  00 01 00 16 00 00 00 01  00 00 00 00 00 10 00 00  |................|
>
>   Type:                              REL (Relocatable file)
>
> Cc: stable@vger.kernel.org
> Fixes: 90ceddcb4950 ("bpf: Support llvm-objcopy for vmlinux BTF")
> Link: https://github.com/llvm/llvm-project/pull/75643
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

Tested-by: Justin Stitt <justinstitt@google.com>

> ---
>  scripts/link-vmlinux.sh | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index a432b171be82..8a9f48b3cb32 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -135,8 +135,15 @@ gen_btf()
>         ${OBJCOPY} --only-section=.BTF --set-section-flags .BTF=alloc,readonly \
>                 --strip-all ${1} ${2} 2>/dev/null
>         # Change e_type to ET_REL so that it can be used to link final vmlinux.
> -       # Unlike GNU ld, lld does not allow an ET_EXEC input.
> -       printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 status=none
> +       # Unlike GNU ld, lld does not allow an ET_EXEC input. Make sure the correct
> +       # byte gets changed with big endian platforms, otherwise e_type may be an
> +       # invalid value.
> +       if is_enabled CONFIG_CPU_BIG_ENDIAN; then
> +               seek=17
> +       else
> +               seek=16
> +       fi
> +       printf '\1' | dd of=${2} conv=notrunc bs=1 seek=${seek} status=none
>  }
>
>  # Create ${2} .S file with all symbols from the ${1} object file
>
> ---
> base-commit: 54be6c6c5ae8e0d93a6c4641cb7528eb0b6ba478
> change-id: 20240208-fix-elf-type-btf-vmlinux-bin-o-big-endian-dbc55a1e1296
>
> Best regards,
> --
> Nathan Chancellor <nathan@kernel.org>
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] kbuild: Fix changing ELF file type for output of gen_btf for big endian
  2024-02-08 20:21 [PATCH] kbuild: Fix changing ELF file type for output of gen_btf for big endian Nathan Chancellor
  2024-02-08 21:10 ` Nicolas Schier
  2024-02-08 21:50 ` Justin Stitt
@ 2024-02-10  0:48 ` Kees Cook
  2024-02-13  0:55 ` Masahiro Yamada
  3 siblings, 0 replies; 7+ messages in thread
From: Kees Cook @ 2024-02-10  0:48 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: masahiroy, nicolas, ndesaulniers, morbo, justinstitt, maskray,
	linux-kbuild, bpf, llvm, patches, stable

On Thu, Feb 08, 2024 at 01:21:06PM -0700, Nathan Chancellor wrote:
> Commit 90ceddcb4950 ("bpf: Support llvm-objcopy for vmlinux BTF")
> changed the ELF type of .btf.vmlinux.bin.o from ET_EXEC to ET_REL via
> dd, which works fine for little endian platforms:
> 
>    00000000  7f 45 4c 46 02 01 01 00  00 00 00 00 00 00 00 00  |.ELF............|
>   -00000010  03 00 b7 00 01 00 00 00  00 00 00 80 00 80 ff ff  |................|
>   +00000010  01 00 b7 00 01 00 00 00  00 00 00 80 00 80 ff ff  |................|
> 
> However, for big endian platforms, it changes the wrong byte, resulting
> in an invalid ELF file type, which ld.lld rejects:
> 
>    00000000  7f 45 4c 46 02 02 01 00  00 00 00 00 00 00 00 00  |.ELF............|
>   -00000010  00 03 00 16 00 00 00 01  00 00 00 00 00 10 00 00  |................|
>   +00000010  01 03 00 16 00 00 00 01  00 00 00 00 00 10 00 00  |................|
> 
>   Type:                              <unknown>: 103
> 
>   ld.lld: error: .btf.vmlinux.bin.o: unknown file type
> 
> Fix this by using a different seek value for dd when targeting big
> endian, so that the correct byte gets changed and everything works
> correctly for all linkers.
> 
>    00000000  7f 45 4c 46 02 02 01 00  00 00 00 00 00 00 00 00  |.ELF............|
>   -00000010  00 03 00 16 00 00 00 01  00 00 00 00 00 10 00 00  |................|
>   +00000010  00 01 00 16 00 00 00 01  00 00 00 00 00 10 00 00  |................|
> 
>   Type:                              REL (Relocatable file)
> 
> Cc: stable@vger.kernel.org
> Fixes: 90ceddcb4950 ("bpf: Support llvm-objcopy for vmlinux BTF")
> Link: https://github.com/llvm/llvm-project/pull/75643
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

Yeah, looks good to me.

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] kbuild: Fix changing ELF file type for output of gen_btf for big endian
  2024-02-08 20:21 [PATCH] kbuild: Fix changing ELF file type for output of gen_btf for big endian Nathan Chancellor
                   ` (2 preceding siblings ...)
  2024-02-10  0:48 ` Kees Cook
@ 2024-02-13  0:55 ` Masahiro Yamada
  2024-02-13  1:43   ` Nathan Chancellor
  3 siblings, 1 reply; 7+ messages in thread
From: Masahiro Yamada @ 2024-02-13  0:55 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: nicolas, ndesaulniers, morbo, justinstitt, keescook, maskray,
	linux-kbuild, bpf, llvm, patches, stable

On Fri, Feb 9, 2024 at 5:21 AM Nathan Chancellor <nathan@kernel.org> wrote:
>
> Commit 90ceddcb4950 ("bpf: Support llvm-objcopy for vmlinux BTF")
> changed the ELF type of .btf.vmlinux.bin.o from ET_EXEC to ET_REL via
> dd, which works fine for little endian platforms:
>
>    00000000  7f 45 4c 46 02 01 01 00  00 00 00 00 00 00 00 00  |.ELF............|
>   -00000010  03 00 b7 00 01 00 00 00  00 00 00 80 00 80 ff ff  |................|



I am afraid this dump is confusing.

The byte stream "03 00" is ET_DYN, as specified in ELF:



  Name        Value
  ------------------
  ET_REL        1
  ET_EXEC       2
  ET_DYN        3



It disagrees with your commit message "from ET_EXEC to ET_REL"

The dump for the old ELF was "02 00", wasn't it?





>   +00000010  01 00 b7 00 01 00 00 00  00 00 00 80 00 80 ff ff  |................|
>
> However, for big endian platforms, it changes the wrong byte, resulting
> in an invalid ELF file type, which ld.lld rejects:


Fangrui pointed out this is true for inutils >= 2.35



>
>    00000000  7f 45 4c 46 02 02 01 00  00 00 00 00 00 00 00 00  |.ELF............|
>   -00000010  00 03 00 16 00 00 00 01  00 00 00 00 00 10 00 00  |................|
>   +00000010  01 03 00 16 00 00 00 01  00 00 00 00 00 10 00 00  |................|

 -  00 02
 +  01 02



>
>   Type:                              <unknown>: 103
>
>   ld.lld: error: .btf.vmlinux.bin.o: unknown file type
>
> Fix this by using a different seek value for dd when targeting big
> endian, so that the correct byte gets changed and everything works
> correctly for all linkers.
>
>    00000000  7f 45 4c 46 02 02 01 00  00 00 00 00 00 00 00 00  |.ELF............|
>   -00000010  00 03 00 16 00 00 00 01  00 00 00 00 00 10 00 00  |................|


Ditto.




>   +00000010  00 01 00 16 00 00 00 01  00 00 00 00 00 10 00 00  |................|
>
>   Type:                              REL (Relocatable file)
>
> Cc: stable@vger.kernel.org
> Fixes: 90ceddcb4950 ("bpf: Support llvm-objcopy for vmlinux BTF")
> Link: https://github.com/llvm/llvm-project/pull/75643
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> ---
>  scripts/link-vmlinux.sh | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index a432b171be82..8a9f48b3cb32 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -135,8 +135,15 @@ gen_btf()
>         ${OBJCOPY} --only-section=.BTF --set-section-flags .BTF=alloc,readonly \
>                 --strip-all ${1} ${2} 2>/dev/null
>         # Change e_type to ET_REL so that it can be used to link final vmlinux.
> -       # Unlike GNU ld, lld does not allow an ET_EXEC input.
> -       printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 status=none
> +       # Unlike GNU ld, lld does not allow an ET_EXEC input. Make sure the correct
> +       # byte gets changed with big endian platforms, otherwise e_type may be an
> +       # invalid value.
> +       if is_enabled CONFIG_CPU_BIG_ENDIAN; then
> +               seek=17
> +       else
> +               seek=16
> +       fi
> +       printf '\1' | dd of=${2} conv=notrunc bs=1 seek=${seek} status=none
>  }
>
>  # Create ${2} .S file with all symbols from the ${1} object file



Do you want to send v2 to update the commit description?


The current code will work, but another approach might be to
update both byte 16 and byte 17 because e_type is a 16-bit field.


It works without relying on the MSB of the previous e_type being zero.
The comment does not need updating because the intention is obvious
from the code.


if is_enabled CONFIG_CPU_BIG_ENDIAN; then
        et_rel='\0\1'
else
        et_rel='\1\0'
fi

printf "${et_rel}" | dd of=${2} conv=notrunc bs=1 seek=16 status=none










> ---
> base-commit: 54be6c6c5ae8e0d93a6c4641cb7528eb0b6ba478
> change-id: 20240208-fix-elf-type-btf-vmlinux-bin-o-big-endian-dbc55a1e1296
>
> Best regards,
> --
> Nathan Chancellor <nathan@kernel.org>
>


-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] kbuild: Fix changing ELF file type for output of gen_btf for big endian
  2024-02-13  0:55 ` Masahiro Yamada
@ 2024-02-13  1:43   ` Nathan Chancellor
  0 siblings, 0 replies; 7+ messages in thread
From: Nathan Chancellor @ 2024-02-13  1:43 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: nicolas, ndesaulniers, morbo, justinstitt, keescook, maskray,
	linux-kbuild, bpf, llvm, patches, stable

On Tue, Feb 13, 2024 at 09:55:07AM +0900, Masahiro Yamada wrote:
> On Fri, Feb 9, 2024 at 5:21 AM Nathan Chancellor <nathan@kernel.org> wrote:
> >
> > Commit 90ceddcb4950 ("bpf: Support llvm-objcopy for vmlinux BTF")
> > changed the ELF type of .btf.vmlinux.bin.o from ET_EXEC to ET_REL via
> > dd, which works fine for little endian platforms:
> >
> >    00000000  7f 45 4c 46 02 01 01 00  00 00 00 00 00 00 00 00  |.ELF............|
> >   -00000010  03 00 b7 00 01 00 00 00  00 00 00 80 00 80 ff ff  |................|
> 
> I am afraid this dump is confusing.
> 
> The byte stream "03 00" is ET_DYN, as specified in ELF:
> 
>   Name        Value
>   ------------------
>   ET_REL        1
>   ET_EXEC       2
>   ET_DYN        3
> 
> It disagrees with your commit message "from ET_EXEC to ET_REL"
> 
> The dump for the old ELF was "02 00", wasn't it?

No, I have not manually edited or changed these diffs from hexdumping
the .o files. The little endian one was from arm64 and the big endian
one was from s390. Perhaps this is some difference between the
toolchains? I don't recall which one I used in this case, pretty sure it
was GNU though. I can just remove "from ET_EXEC" from the commit message
if that would help make it less confusing.

> >   +00000010  01 00 b7 00 01 00 00 00  00 00 00 80 00 80 ff ff  |................|
> >
> > However, for big endian platforms, it changes the wrong byte, resulting
> > in an invalid ELF file type, which ld.lld rejects:
> 
> Fangrui pointed out this is true for inutils >= 2.35

Not for this particular error, which occurs because e_type is not a
valid value. If it was true for binutils, we would have seen this issue
sooner.

> >
> >    00000000  7f 45 4c 46 02 02 01 00  00 00 00 00 00 00 00 00  |.ELF............|
> >   -00000010  00 03 00 16 00 00 00 01  00 00 00 00 00 10 00 00  |................|
> >   +00000010  01 03 00 16 00 00 00 01  00 00 00 00 00 10 00 00  |................|
> 
>  -  00 02
>  +  01 02

See above.

> >
> >   Type:                              <unknown>: 103
> >
> >   ld.lld: error: .btf.vmlinux.bin.o: unknown file type
> >
> > Fix this by using a different seek value for dd when targeting big
> > endian, so that the correct byte gets changed and everything works
> > correctly for all linkers.
> >
> >    00000000  7f 45 4c 46 02 02 01 00  00 00 00 00 00 00 00 00  |.ELF............|
> >   -00000010  00 03 00 16 00 00 00 01  00 00 00 00 00 10 00 00  |................|
> 
> Ditto.

See above.

> >   +00000010  00 01 00 16 00 00 00 01  00 00 00 00 00 10 00 00  |................|
> >
> >   Type:                              REL (Relocatable file)
> >
> > Cc: stable@vger.kernel.org
> > Fixes: 90ceddcb4950 ("bpf: Support llvm-objcopy for vmlinux BTF")
> > Link: https://github.com/llvm/llvm-project/pull/75643
> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> > ---
> >  scripts/link-vmlinux.sh | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> > index a432b171be82..8a9f48b3cb32 100755
> > --- a/scripts/link-vmlinux.sh
> > +++ b/scripts/link-vmlinux.sh
> > @@ -135,8 +135,15 @@ gen_btf()
> >         ${OBJCOPY} --only-section=.BTF --set-section-flags .BTF=alloc,readonly \
> >                 --strip-all ${1} ${2} 2>/dev/null
> >         # Change e_type to ET_REL so that it can be used to link final vmlinux.
> > -       # Unlike GNU ld, lld does not allow an ET_EXEC input.
> > -       printf '\1' | dd of=${2} conv=notrunc bs=1 seek=16 status=none
> > +       # Unlike GNU ld, lld does not allow an ET_EXEC input. Make sure the correct
> > +       # byte gets changed with big endian platforms, otherwise e_type may be an
> > +       # invalid value.
> > +       if is_enabled CONFIG_CPU_BIG_ENDIAN; then
> > +               seek=17
> > +       else
> > +               seek=16
> > +       fi
> > +       printf '\1' | dd of=${2} conv=notrunc bs=1 seek=${seek} status=none
> >  }
> >
> >  # Create ${2} .S file with all symbols from the ${1} object file
> 
> Do you want to send v2 to update the commit description?
> 

I don't think a v2 is necessary for the commit description...

> The current code will work, but another approach might be to
> update both byte 16 and byte 17 because e_type is a 16-bit field.
> 
> It works without relying on the MSB of the previous e_type being zero.
> The comment does not need updating because the intention is obvious
> from the code.
> 
> if is_enabled CONFIG_CPU_BIG_ENDIAN; then
>         et_rel='\0\1'
> else
>         et_rel='\1\0'
> fi
> 
> printf "${et_rel}" | dd of=${2} conv=notrunc bs=1 seek=16 status=none

but I do like this suggested change because I was thinking that updating
the single bit could be fragile at some point. I'll send a v2 with that
and a slightly updated commit message shortly. Because it is
substantially different from v1, I won't carry forward all the tags I
received but I hope people will take a look at v2 and provide them
again.

Thanks a lot for taking a look!
Nathan

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-02-13  1:43 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-08 20:21 [PATCH] kbuild: Fix changing ELF file type for output of gen_btf for big endian Nathan Chancellor
2024-02-08 21:10 ` Nicolas Schier
2024-02-08 21:22   ` Fangrui Song
2024-02-08 21:50 ` Justin Stitt
2024-02-10  0:48 ` Kees Cook
2024-02-13  0:55 ` Masahiro Yamada
2024-02-13  1:43   ` Nathan Chancellor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).