linux-kernel - Re: [PATCH v6 5/6] LoongArch: Support PC-relative relocations in modules

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAAhV-H4PXsBop6c3or92hv01YCiY=JFLnTwjSykaeN21+YvwFQ@mail.gmail.com>
Date:   Tue, 30 Aug 2022 15:10:29 +0800
From:   Huacai Chen <chenhuacai@...nel.org>
To:     WANG Xuerui <kernel@...0n.name>
Cc:     Xi Ruoyao <xry111@...111.site>, loongarch@...ts.linux.dev,
        LKML <linux-kernel@...r.kernel.org>,
        Youling Tang <tangyouling@...ngson.cn>,
        Jinyang He <hejinyang@...ngson.cn>
Subject: Re: [PATCH v6 5/6] LoongArch: Support PC-relative relocations in modules

On Tue, Aug 30, 2022 at 9:42 AM WANG Xuerui <kernel@...0n.name> wrote:
>
> On 2022/8/29 23:08, Huacai Chen wrote:
> > [snip]
> >> +static int apply_r_larch_pcala(struct module *mod, u32 *location, Elf_Addr v,
> >> +                       s64 *rela_stack, size_t *rela_stack_top, unsigned int type)
> >> +{
> >> +       union loongarch_instruction *insn = (union loongarch_instruction *)location;
> >> +       /* Use s32 for a sign-extension deliberately. */
> >> +       s32 offset_hi20 = (void *)((v + 0x800) & ~0xfff) -
> >> +               (void *)((Elf_Addr)location & ~0xfff);
> >> +       Elf_Addr anchor = (((Elf_Addr)location) & ~0xfff) + offset_hi20;
> >> +       ptrdiff_t offset_rem = (void *)v - (void *)anchor;
> >> +
> >> +       switch (type) {
> >> +       case R_LARCH_PCALA_HI20:
> >> +               v = offset_hi20 >> 12;
> >> +               break;
> >> +       case R_LARCH_PCALA64_LO20:
> >> +               v = offset_rem >> 32;
> >> +               break;
> >> +       case R_LARCH_PCALA64_HI12:
> >> +               v = offset_rem >> 52;
> >> +               break;
> >> +       default:
> >> +               /* Do nothing. */
> >> +       }
> >> +
> >> +       switch (type) {
> >> +       case R_LARCH_PCALA_HI20:
> >> +       case R_LARCH_PCALA64_LO20:
> >> +               insn->reg1i20_format.immediate = v & 0xfffff;
> >> +               break;
> >> +       case R_LARCH_PCALA_LO12:
> >> +       case R_LARCH_PCALA64_HI12:
> >> +               insn->reg2i12_format.immediate = v & 0xfff;
> >> +               break;
> >> +       default:
> >> +               pr_err("%s: Unsupport relocation type %u\n", mod->name, type);
> >> +               return -EINVAL;
> >> +       }
> > Can we merge the two switch here?
>
> IMO leaving as-is or even splitting into two functions would be
> acceptable, as the two switches are performing two different things --
> namely "adjustFixupValue" (in LLVM-speak) and actually inserting the
> value into the insn word. But an argument for merging the two can be
> made too, because the v2.00 reloc types are purposely designed with
> unique use case for each, meaning there is actually no flexibility in
> between the fixup value's calculation and application. So I think this
> eventually comes down to coder's preference?
Merging them can just make me understand the logic better, and save
some lines. :)

Huacai
>
> --
> WANG "xen0n" Xuerui
>
> Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/
>