linux-kernel - Re: [PATCH v1 4/6] objtool/LoongArch: Enable orc to be built

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230725115609.GD3765278@hirez.programming.kicks-ass.net>
Date:   Tue, 25 Jul 2023 13:56:09 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Tiezhu Yang <yangtiezhu@...ngson.cn>
Cc:     Josh Poimboeuf <jpoimboe@...nel.org>,
        Huacai Chen <chenhuacai@...nel.org>, loongarch@...ts.linux.dev,
        linux-kernel@...r.kernel.org, loongson-kernel@...ts.loongnix.cn
Subject: Re: [PATCH v1 4/6] objtool/LoongArch: Enable orc to be built

On Tue, Jul 25, 2023 at 04:15:08PM +0800, Tiezhu Yang wrote:

> +void arch_write_orc(struct elf *elf, struct orc_entry *orc)
> +{
> +	orc->ra_offset = bswap_if_needed(elf, orc->ra_offset);
> +}

This hardly 'writes' anything, all it does is a bswap.

> diff --git a/tools/objtool/orc_gen.c b/tools/objtool/orc_gen.c
> index 1eff7e0a..6975056 100644
> --- a/tools/objtool/orc_gen.c
> +++ b/tools/objtool/orc_gen.c
> @@ -14,6 +14,8 @@
>  #include <objtool/warn.h>
>  #include <objtool/endianness.h>
>  
> +void __weak arch_write_orc(struct elf *elf, struct orc_entry *orc) {}

And for that you need a weak function? What's wrong with an inline if
anything?

>  static int write_orc_entry(struct elf *elf, struct section *orc_sec,
>  			   struct section *ip_sec, unsigned int idx,
>  			   struct section *insn_sec, unsigned long insn_off,
> @@ -26,6 +28,7 @@ static int write_orc_entry(struct elf *elf, struct section *orc_sec,
>  	memcpy(orc, o, sizeof(*orc));
>  	orc->sp_offset = bswap_if_needed(elf, orc->sp_offset);
>  	orc->bp_offset = bswap_if_needed(elf, orc->bp_offset);
> +	arch_write_orc(elf, orc);

Why can't this simply be:

	orc->ra_offset = bswap_if_needed(elf, orc->ra_offset);

AFAICT this won't actually do anything for x86 and I don't think Power
got around to doing ORC yet.

>  
>  	/* populate reloc for ip */
>  	if (!elf_init_reloc_text_sym(elf, ip_sec, idx * sizeof(int), idx,

Again, not much explaination for why you did things