+ linux-arm-kernel and some folks who might know another idea.
On Sun, May 21, 2023 at 9:05 AM Masahiro Yamada <masahiroy@xxxxxxxxxx> wrote:
>
> addend_arm_rel() processes R_ARM_ABS32 in a wrong way.
>
> Here, simple test code.
>
> [test code 1]
>
> #include <linux/init.h>
>
> int __initdata foo;
> int get_foo(int x) { return foo; }
>
> If you compile it with ARM versatile_defconfig, modpost will show the
> symbol name, (unknown).
>
> WARNING: modpost: vmlinux.o: section mismatch in reference: get_foo (section: .text) -> (unknown) (section: .init.data)
>
> If you compile it for other architectures, modpost will show the correct
> symbol name.
>
> WARNING: modpost: vmlinux.o: section mismatch in reference: get_foo (section: .text) -> foo (section: .init.data)
>
> For R_ARM_ABS32, addend_arm_rel() sets r->r_addend to a wrong value.
>
> I just mimicked the code in arch/arm/kernel/module.c.
>
> However, there is more difficulty for ARM.
>
> Here, test code.
>
> [test code 2]
>
> #include <linux/init.h>
>
> int __initdata foo;
> int get_foo(int x) { return foo; }
>
> int __initdata bar;
> int get_bar(int x) { return bar; }
>
> With this commit applied, modpost will show the following messages
> for ARM versatile_defconfig:
>
> WARNING: modpost: vmlinux.o: section mismatch in reference: get_foo (section: .text) -> foo (section: .init.data)
> WARNING: modpost: vmlinux.o: section mismatch in reference: get_bar (section: .text) -> foo (section: .init.data)
>
> The reference from 'get_bar' to 'foo' seems wrong.
>
> I have no solution for this because it is true in assembly level.
>
> In the following output, relocation at 0x1c is no longer associated
> with 'bar'. The two relocation entries point to the same symbol, and
> the offset to 'bar' is encoded in the instruction 'r0, [r3, #4]'.
>
> Disassembly of section .text:
>
> 00000000 <get_foo>:
> 0: e59f3004 ldr r3, [pc, #4] @ c <get_foo+0xc>
> 4: e5930000 ldr r0, [r3]
> 8: e12fff1e bx lr
> c: 00000000 .word 0x00000000
>
> 00000010 <get_bar>:
> 10: e59f3004 ldr r3, [pc, #4] @ 1c <get_bar+0xc>
> 14: e5930004 ldr r0, [r3, #4]
> 18: e12fff1e bx lr
> 1c: 00000000 .word 0x00000000
>
> Relocation section '.rel.text' at offset 0x244 contains 2 entries:
> Offset Info Type Sym.Value Sym. Name
> 0000000c 00000c02 R_ARM_ABS32 00000000 .init.data
> 0000001c 00000c02 R_ARM_ABS32 00000000 .init.data
>
> When find_elf_symbol() gets into a situation where relsym->st_name is
> zero, there is no guarantee to get the symbol name as written in C.
>
> I am keeping the current logic because it is useful in many architectures,
> but the symbol name is not always correct depending on the optimization
> of the relocation. I left some comments in find_tosym().
>
> Fixes: 56a974fa2d59 ("kbuild: make better section mismatch reports on arm")
> Signed-off-by: Masahiro Yamada <masahiroy@xxxxxxxxxx>
> ---
>
> Changes in v6:
> - More detailed commit log
>
> scripts/mod/modpost.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 71de14544432..34fbbd85bfde 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -1124,6 +1124,10 @@ static Elf_Sym *find_tosym(struct elf_info *elf, Elf64_Sword addr,
> if (relsym->st_name != 0)
> return relsym;
>
> + /*
> + * Strive to find a better symbol name, but the resulting name does not
> + * always match the symbol referenced in the original code.
> + */
> relsym_secindex = get_secindex(elf, relsym);
> for (sym = elf->symtab_start; sym < elf->symtab_stop; sym++) {
> if (get_secindex(elf, sym) != relsym_secindex)
> @@ -1306,12 +1310,12 @@ static int addend_386_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r)
> static int addend_arm_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r)
> {
> unsigned int r_typ = ELF_R_TYPE(r->r_info);
> + Elf_Sym *sym = elf->symtab_start + ELF_R_SYM(r->r_info);
> + unsigned int inst = TO_NATIVE(*reloc_location(elf, sechdr, r));
>
> switch (r_typ) {
> case R_ARM_ABS32:
> - /* From ARM ABI: (S + A) | T */
> - r->r_addend = (int)(long)
> - (elf->symtab_start + ELF_R_SYM(r->r_info));
> + r->r_addend = inst + sym->st_value;
> break;
> case R_ARM_PC24:
> case R_ARM_CALL:
> --
> 2.39.2
>
--
Thanks,
~Nick Desaulniers
