Message-ID: <555A2D39.4030809@plumgrid.com>
Date: Mon, 18 May 2015 11:19:37 -0700
From: Alexei Starovoitov <ast@plumgrid.com>
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:31.0) Gecko/20100101 Thunderbird/31.6.0
MIME-Version: 1.0
To: Wang Nan <wangnan0@huawei.com>, paulus@samba.org, a.p.zijlstra@chello.nl,
        mingo@redhat.com, acme@kernel.org, namhyung@kernel.org,
        jolsa@kernel.org, dsahern@gmail.com, daniel@iogearbox.net,
        brendan.d.gregg@gmail.com, masami.hiramatsu.pt@hitachi.com
CC: lizefan@huawei.com, linux-kernel@vger.kernel.org, pi3orama@163.com
Subject: Re: [RFC PATCH v3 10/37] bpf tools: Check endianess and set swap
 flag according to EHDR
References: <1431860222-61636-1-git-send-email-wangnan0@huawei.com> <1431860222-61636-11-git-send-email-wangnan0@huawei.com>
In-Reply-To: <1431860222-61636-11-git-send-email-wangnan0@huawei.com>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1716
Lines: 45

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@huawei.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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/