netdev - Re: [PATCH bpf v1] bpf: fix libelf endian handling in resolv

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: <caf1dcbd-7a07-993c-e940-1b2689985c5a@fb.com>
Date:   Wed, 16 Jun 2021 08:56:42 -0700
From:   Yonghong Song <yhs@...com>
To:     Tony Ambardar <tony.ambardar@...il.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <ast@...nel.org>,
        Andrii Nakryiko <andrii@...nel.org>
CC:     <bpf@...r.kernel.org>, <netdev@...r.kernel.org>,
        <stable@...r.kernel.org>, Jiri Olsa <jolsa@...nel.org>
Subject: Re: [PATCH bpf v1] bpf: fix libelf endian handling in resolv_btfids



On 6/16/21 2:25 AM, Tony Ambardar wrote:
> While patching the .BTF_ids section in vmlinux, resolve_btfids writes type
> ids using host-native endianness, and relies on libelf for any required
> translation when finally updating vmlinux. However, the default type of the
> .BTF_ids section content is ELF_T_BYTE (i.e. unsigned char), and undergoes
> no translation. This results in incorrect patched values if cross-compiling
> to non-native endianness, and can manifest as kernel Oops and test failures
> which are difficult to debug.
> 
> Explicitly set the type of patched data to ELF_T_WORD, allowing libelf to
> transparently handle the endian conversions.
> 
> Fixes: fbbb68de80a4 ("bpf: Add resolve_btfids tool to resolve BTF IDs in ELF object")
> Cc: stable@...r.kernel.org # v5.10+
> Cc: Jiri Olsa <jolsa@...nel.org>
> Cc: Yonghong Song <yhs@...com>
> Link: https://lore.kernel.org/bpf/CAPGftE_eY-Zdi3wBcgDfkz_iOr1KF10n=9mJHm1_a_PykcsoeA@mail.gmail.com/
> Signed-off-by: Tony Ambardar <Tony.Ambardar@...il.com>
> ---
>   tools/bpf/resolve_btfids/main.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
> index d636643ddd35..f32c059fbfb4 100644
> --- a/tools/bpf/resolve_btfids/main.c
> +++ b/tools/bpf/resolve_btfids/main.c
> @@ -649,6 +649,9 @@ static int symbols_patch(struct object *obj)
>   	if (sets_patch(obj))
>   		return -1;
>   
> +	/* Set type to ensure endian translation occurs. */
> +	obj->efile.idlist->d_type = ELF_T_WORD;

The change makes sense to me as .BTF_ids contains just a list of
u32's.

Jiri, could you double check on this?

> +
>   	elf_flagdata(obj->efile.idlist, ELF_C_SET, ELF_F_DIRTY);
>   
>   	err = elf_update(obj->efile.elf, ELF_C_WRITE);
>