linux-kernel - Re: [RFC PATCH v3 10/37] bpf tools: Check endianess and set swap flag according to EHDR

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: <555A2D39.4030809@plumgrid.com>
Date:	Mon, 18 May 2015 11:19:37 -0700
From:	Alexei Starovoitov <ast@...mgrid.com>
To:	Wang Nan <wangnan0@...wei.com>, paulus@...ba.org,
	a.p.zijlstra@...llo.nl, mingo@...hat.com, acme@...nel.org,
	namhyung@...nel.org, jolsa@...nel.org, dsahern@...il.com,
	daniel@...earbox.net, brendan.d.gregg@...il.com,
	masami.hiramatsu.pt@...achi.com
CC:	lizefan@...wei.com, linux-kernel@...r.kernel.org, pi3orama@....com
Subject: Re: [RFC PATCH v3 10/37] bpf tools: Check endianess and set swap
 flag according to EHDR

On 5/17/15 3:56 AM, Wang Nan wrote:
> Check endianess according to EHDR to support loading eBPF objects into
> big endian machines. Code is taken from tools/perf/util/symbol-elf.c.
>
> Signed-off-by: Wang Nan <wangnan0@...wei.com>
> ---
...
> +static int
> +bpf_obj_swap_init(struct bpf_object *obj)
> +{
> +	static unsigned int const endian = 1;
> +
> +	obj->needs_swap = false;
> +
> +	switch (obj->elf.ehdr.e_ident[EI_DATA]) {
> +	case ELFDATA2LSB:
> +		/* We are big endian, BPF obj is little endian. */
> +		if (*(unsigned char const *)&endian != 1)
> +			obj->needs_swap = true;
> +		return 0;
> +
> +	case ELFDATA2MSB:
> +		/* We are little endian, BPF obj is big endian. */
> +		if (*(unsigned char const *)&endian != 0)
> +			obj->needs_swap = true;
> +		return 0;

I don't like 'needs_swap', since it implies that bpf loader
can magically convert little endian code into big endian.
It's obviously not the case.
For example original C program may have: (u8) ptr->val;
where 'val' is 32-bit variable. llvm may optimize it into
byte load, but offset will be different depending on endianness
of the target. By byteswapping instruction encoding you'll be able
to load such code, but it will be logically incorrect. Verifier will
catch the broken code if accesses are out of bounds, but for
this u32->u8 example, the code is safe, but logically incorrect.
Instead, please help fixing llvm. It needs -march=bpfbe switch,
which will generate proper big-endian code.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/