linux-kernel - Re: [PATCH] bpf: fix selftest/bpf/test_pkt_md

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: <5984C5A4.8040301@iogearbox.net>
Date:   Fri, 04 Aug 2017 21:06:12 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Thomas Richter <tmricht@...ux.vnet.ibm.com>, ast@...nel.org,
        linux-kernel@...r.kernel.org
CC:     schwidefsky@...ibm.com, brueckner@...ux.vnet.ibm.com
Subject: Re: [PATCH] bpf: fix selftest/bpf/test_pkt_md_access on s390x

Hi Thomas,

On 08/04/2017 08:40 AM, Thomas Richter wrote:
> Commit 18f3d6be6be1 ("selftests/bpf: Add test cases to test narrower ctx field loads")
> introduced new eBPF test cases. One of them (test_pkt_md_access.c)
> fails on s390x. The BPF verifier error message is:

Could you submit the fix to: netdev@...r.kernel.org

(BPF patches are handled/accepted there.)

Thanks a lot,
Daniel

> [root@...60046 bpf]# ./test_progs
> test_pkt_access:PASS:ipv4 349 nsec
> test_pkt_access:PASS:ipv6 212 nsec
> [....]
> libbpf: load bpf program failed: Permission denied
> libbpf: -- BEGIN DUMP LOG ---
> libbpf:
> 0: (71) r2 = *(u8 *)(r1 +0)
> invalid bpf_context access off=0 size=1
>
> libbpf: -- END LOG --
> libbpf: failed to load program 'test1'
> libbpf: failed to load object './test_pkt_md_access.o'
> Summary: 29 PASSED, 1 FAILED
> [root@...60046 bpf]#
>
> This is caused by a byte endianess issue. S390x is a big endian
> architecture.  Pointer access to the lowest byte or halfword of a
> four byte value need to add an offset.
> On little endian architectures this offset is not needed.
>
> Fix this and use the same approach as the originator used for other files
> (for example test_verifier.c) in his original commit.
>
> With this fix the test program test_progs succeeds on s390x:
> [root@...60046 bpf]# ./test_progs
> test_pkt_access:PASS:ipv4 236 nsec
> test_pkt_access:PASS:ipv6 217 nsec
> test_xdp:PASS:ipv4 3624 nsec
> test_xdp:PASS:ipv6 1722 nsec
> test_l4lb:PASS:ipv4 926 nsec
> test_l4lb:PASS:ipv6 1322 nsec
> test_tcp_estats:PASS: 0 nsec
> test_bpf_obj_id:PASS:get-fd-by-notexist-prog-id 0 nsec
> test_bpf_obj_id:PASS:get-fd-by-notexist-map-id 0 nsec
> test_bpf_obj_id:PASS:get-prog-info(fd) 0 nsec
> test_bpf_obj_id:PASS:get-map-info(fd) 0 nsec
> test_bpf_obj_id:PASS:get-prog-info(fd) 0 nsec
> test_bpf_obj_id:PASS:get-map-info(fd) 0 nsec
> test_bpf_obj_id:PASS:get-prog-fd(next_id) 0 nsec
> test_bpf_obj_id:PASS:get-prog-info(next_id->fd) 0 nsec
> test_bpf_obj_id:PASS:get-prog-fd(next_id) 0 nsec
> test_bpf_obj_id:PASS:get-prog-info(next_id->fd) 0 nsec
> test_bpf_obj_id:PASS:check total prog id found by get_next_id 0 nsec
> test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
> test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
> test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
> test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
> test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
> test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
> test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
> test_bpf_obj_id:PASS:check get-map-info(next_id->fd) 0 nsec
> test_bpf_obj_id:PASS:get-map-fd(next_id) 0 nsec
> test_bpf_obj_id:PASS:check get-map-info(next_id->fd) 0 nsec
> test_bpf_obj_id:PASS:check total map id found by get_next_id 0 nsec
> test_pkt_md_access:PASS: 277 nsec
> Summary: 30 PASSED, 0 FAILED
> [root@...60046 bpf]#
>
> Signed-off-by: Thomas Richter <tmricht@...ux.vnet.ibm.com>
> ---
>   tools/testing/selftests/bpf/test_pkt_md_access.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/test_pkt_md_access.c b/tools/testing/selftests/bpf/test_pkt_md_access.c
> index 71729d4..8393ced 100644
> --- a/tools/testing/selftests/bpf/test_pkt_md_access.c
> +++ b/tools/testing/selftests/bpf/test_pkt_md_access.c
> @@ -12,12 +12,23 @@
>
>   int _version SEC("version") = 1;
>
> +#ifdef __LITTLE_ENDIAN
>   #define TEST_FIELD(TYPE, FIELD, MASK)					\
>   	{								\
>   		TYPE tmp = *(volatile TYPE *)&skb->FIELD;		\
>   		if (tmp != ((*(volatile __u32 *)&skb->FIELD) & MASK))	\
>   			return TC_ACT_SHOT;				\
>   	}
> +#else
> +#define TEST_FIELD_OFFSET(a, b)	((sizeof(a) - sizeof(b)) / sizeof(b))
> +#define TEST_FIELD(TYPE, FIELD, MASK)					\
> +	{								\
> +		TYPE tmp = *((volatile TYPE *)&skb->FIELD +		\
> +			      TEST_FIELD_OFFSET(skb->FIELD, TYPE));	\
> +		if (tmp != ((*(volatile __u32 *)&skb->FIELD) & MASK))	\
> +			return TC_ACT_SHOT;				\
> +	}
> +#endif
>
>   SEC("test1")
>   int process(struct __sk_buff *skb)
>