Re: [PATCH v3] x86: Treat R_386_PLT32 as R_386_PC32

From: Fangrui Song
Date: Mon Jan 25 2021 - 12:32:02 EST



On 2021-01-25, Borislav Petkov wrote:
It's a good thing I have a toolchain guy who can explain to me what you
guys are doing because you need to start writing those commit messages
for !toolchain developers.

How about this following message? I'll answer your questions in line as
well. Explaining everything in the message will be quite long... If
someone is interested, I have put every possibly related matter in
https://maskray.me/blog/2021-01-09-copy-relocations-canonical-plt-entries-and-protected


This is similar to commit b21ebf2fb4cd ("x86: Treat R_X86_64_PLT32 as
R_X86_64_PC32"), but for i386. As far as Linux kernel is concerned,
R_386_PLT32 can be treated the same as R_386_PC32.

R_386_PLT32/R_X86_64_PLT32 are PC-relative relocation types which can
only be used by branches. If the referenced symbol is defined
externally, a PLT will be used.
R_386_PC32/R_X86_64_PC32 are PC-relative relocation types which can be
used by address taking operations and branches. If the referenced symbol
is defined externally, a copy relocation/canonical PLT entry will be
created in the executable.

On x86-64, there is no PIC vs non-PIC PLT distinction and an
R_X86_64_PLT32 relocation is produced for both `call/jmp foo` and
`call/jmp foo@PLT` with newer (2018) GNU as/LLVM integrated assembler.
This avoids copy relocations/canonical PLT entries.

On i386, there are 2 types of PLTs, PIC and non-PIC. Currently the
GCC/GNU as convention is to use R_386_PC32 for non-PIC PLT and
R_386_PLT32 for PIC PLT. Copy relocations/canonical PLT entries are
possible ABI issues but GCC/GNU as will likely keep the status quo
because (1) the ABI is legacy (2) the change will drop a GNU ld
diagnostic for non-default visibility ifunc in shared objects.
https://sourceware.org/bugzilla/show_bug.cgi?id=27169

clang-12 -fno-pic (since
https://github.com/llvm/llvm-project/commit/a084c0388e2a59b9556f2de0083333232da3f1d6)
can emit R_386_PLT32 for compiler generated function declarations,
because preventing canonical PLT entries is weighed over the rare ifunc
diagnostic.

Link: https://github.com/ClangBuiltLinux/linux/issues/1210
Reported-by: Arnd Bergmann <arnd@xxxxxxxx>
Signed-off-by: Fangrui Song <maskray@xxxxxxxxxx>
Reviewed-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
Reviewed-by: Nathan Chancellor <natechancellor@xxxxxxxxx>
Tested-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
Tested-by: Nathan Chancellor <natechancellor@xxxxxxxxx>


On Thu, Jan 14, 2021 at 02:48:19PM -0800, Fangrui Song wrote:
This is similar to commit b21ebf2fb4cd ("x86: Treat R_X86_64_PLT32 as
R_X86_64_PC32"), but for i386. As far as Linux kernel is concerned,
R_386_PLT32 can be treated the same as R_386_PC32.

R_386_PC32/R_X86_64_PC32 are PC-relative relocation types with the
requirement that the symbol address is significant.
R_386_PLT32/R_X86_64_PLT32 are PC-relative relocation types without the
address significance requirement.

I was told what "significant" means in that context and while it is
clear to you, I'm pretty sure it is not clear to kernel developers who
haven't looked at toolchains in depth. So please elaborate.

Expanded "significant" to more words. See above.

On x86-64, there is no PIC vs non-PIC PLT distinction and an
R_X86_64_PLT32 relocation is produced for both `call/jmp foo` and
`call/jmp foo@PLT` with newer (2018) GNU as/LLVM integrated assembler.

Also, please explain in short why LLVM is generating R_X86_64_PLT32
relocs now? I.e., is it the same reason as why binutils does that?

I.e., mentioning the big picture of things would help as to why you're
doing this.

It has been explained. The LLVM change was in 2018, roughly the same
time when GNU as emitted R_X86_64_PLT32. I think it does not need
extended explanation because of the separate canonical PLT entries
paragraph.

On i386, there are 2 types of PLTs, PIC and non-PIC. Currently the
convention is to use R_386_PC32 for non-PIC PLT and R_386_PLT32 for PIC
PLT.

Convention in general or convention for LLVM?

Changed to "GCC/GNU as convention".

clang-12 -fno-pic (since
https://github.com/llvm/llvm-project/commit/a084c0388e2a59b9556f2de0083333232da3f1d6)
can emit R_386_PLT32 for compiler generated function declarations as
well to avoid a canonical PLT entry (st_shndx=0, st_value!=0) if the
symbol turns out to be defined externally. GCC/GNU as will likely keep
using R_386_PC32 because (1) the ABI is legacy (2) the change will drop
a GNU ld non-default visibility ifunc for shared objects.
https://sourceware.org/bugzilla/show_bug.cgi?id=27169

Not sure how useful this paragraph is for kernel developers...

Reorganize it a bit...

Link: https://github.com/ClangBuiltLinux/linux/issues/1210
Reported-by: Arnd Bergmann <arnd@xxxxxxxx>
Signed-off-by: Fangrui Song <maskray@xxxxxxxxxx>
Reviewed-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
Reviewed-by: Nathan Chancellor <natechancellor@xxxxxxxxx>
Tested-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
Tested-by: Nathan Chancellor <natechancellor@xxxxxxxxx>

---
Change in v2:
* Improve commit message
---
Change in v3:
* Change the GCC link to the more relevant GNU as link.
* Fix the relevant llvm-project commit id.
---
arch/x86/kernel/module.c | 1 +
arch/x86/tools/relocs.c | 2 ++
2 files changed, 3 insertions(+)

diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
index 34b153cbd4ac..5e9a34b5bd74 100644
--- a/arch/x86/kernel/module.c
+++ b/arch/x86/kernel/module.c
@@ -114,6 +114,7 @@ int apply_relocate(Elf32_Shdr *sechdrs,
*location += sym->st_value;
break;
case R_386_PC32:
+ case R_386_PLT32:
/* Add the value, subtract its position */
*location += sym->st_value - (uint32_t)location;
break;
diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index ce7188cbdae5..717e48ca28b6 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -867,6 +867,7 @@ static int do_reloc32(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
case R_386_PC32:
case R_386_PC16:
case R_386_PC8:
+ case R_386_PLT32:
/*
* NONE can be ignored and PC relative relocations don't
* need to be adjusted.

That comment might need adjustment.

@@ -910,6 +911,7 @@ static int do_reloc_real(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
case R_386_PC32:
case R_386_PC16:
case R_386_PC8:
+ case R_386_PLT32:
/*
* NONE can be ignored and PC relative relocations don't
* need to be adjusted.

Ditto.

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette