| 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
| ||
|
Message-ID: <5ad9aac3-6170-47cb-87be-b77d4425e31a@gmail.com>
Date: Fri, 12 Apr 2024 18:01:21 -0700
From: Kui-Feng Lee <sinquersw@...il.com>
To: Jordan Rife <jrife@...gle.com>, bpf@...r.kernel.org
Cc: linux-kselftest@...r.kernel.org, netdev@...r.kernel.org,
Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>,
Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau
<martin.lau@...ux.dev>, Eduard Zingerman <eddyz87@...il.com>,
Song Liu <song@...nel.org>, Yonghong Song <yonghong.song@...ux.dev>,
John Fastabend <john.fastabend@...il.com>, KP Singh <kpsingh@...nel.org>,
Stanislav Fomichev <sdf@...gle.com>, Hao Luo <haoluo@...gle.com>,
Jiri Olsa <jolsa@...nel.org>, Mykola Lysenko <mykolal@...com>,
Shuah Khan <shuah@...nel.org>, Kui-Feng Lee <thinker.li@...il.com>,
Artem Savkov <asavkov@...hat.com>, Dave Marchevsky <davemarchevsky@...com>,
Menglong Dong <imagedong@...cent.com>, Daniel Xu <dxu@...uu.xyz>,
David Vernet <void@...ifault.com>, Daan De Meyer <daan.j.demeyer@...il.com>,
Willem de Bruijn <willemdebruijn.kernel@...il.com>
Subject: Re: [PATCH v2 bpf-next 1/6] selftests/bpf: Fix bind program for big
endian systems
On 4/12/24 09:52, Jordan Rife wrote:
> Without this fix, the bind4 and bind6 programs will reject bind attempts
> on big endian systems. This patch ensures that CI tests pass for the
> s390x architecture.
>
> Signed-off-by: Jordan Rife <jrife@...gle.com>
> ---
> .../testing/selftests/bpf/progs/bind4_prog.c | 18 ++++++++++--------
> .../testing/selftests/bpf/progs/bind6_prog.c | 18 ++++++++++--------
> tools/testing/selftests/bpf/progs/bind_prog.h | 19 +++++++++++++++++++
> 3 files changed, 39 insertions(+), 16 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/progs/bind_prog.h
>
> diff --git a/tools/testing/selftests/bpf/progs/bind4_prog.c b/tools/testing/selftests/bpf/progs/bind4_prog.c
> index a487f60b73ac4..2bc052ecb6eef 100644
> --- a/tools/testing/selftests/bpf/progs/bind4_prog.c
> +++ b/tools/testing/selftests/bpf/progs/bind4_prog.c
> @@ -12,6 +12,8 @@
> #include <bpf/bpf_helpers.h>
> #include <bpf/bpf_endian.h>
>
> +#include "bind_prog.h"
> +
> #define SERV4_IP 0xc0a801feU /* 192.168.1.254 */
> #define SERV4_PORT 4040
> #define SERV4_REWRITE_IP 0x7f000001U /* 127.0.0.1 */
> @@ -118,23 +120,23 @@ int bind_v4_prog(struct bpf_sock_addr *ctx)
>
> // u8 narrow loads:
> user_ip4 = 0;
> - user_ip4 |= ((volatile __u8 *)&ctx->user_ip4)[0] << 0;
> - user_ip4 |= ((volatile __u8 *)&ctx->user_ip4)[1] << 8;
> - user_ip4 |= ((volatile __u8 *)&ctx->user_ip4)[2] << 16;
> - user_ip4 |= ((volatile __u8 *)&ctx->user_ip4)[3] << 24;
> + user_ip4 |= load_byte_ntoh(ctx->user_ip4, 0, sizeof(user_ip4));
> + user_ip4 |= load_byte_ntoh(ctx->user_ip4, 1, sizeof(user_ip4));
> + user_ip4 |= load_byte_ntoh(ctx->user_ip4, 2, sizeof(user_ip4));
> + user_ip4 |= load_byte_ntoh(ctx->user_ip4, 3, sizeof(user_ip4));
> if (ctx->user_ip4 != user_ip4)
> return 0;
>
> user_port = 0;
> - user_port |= ((volatile __u8 *)&ctx->user_port)[0] << 0;
> - user_port |= ((volatile __u8 *)&ctx->user_port)[1] << 8;
> + user_port |= load_byte_ntoh(ctx->user_port, 0, sizeof(user_port));
> + user_port |= load_byte_ntoh(ctx->user_port, 1, sizeof(user_port));
> if (ctx->user_port != user_port)
> return 0;
>
> // u16 narrow loads:
> user_ip4 = 0;
> - user_ip4 |= ((volatile __u16 *)&ctx->user_ip4)[0] << 0;
> - user_ip4 |= ((volatile __u16 *)&ctx->user_ip4)[1] << 16;
> + user_ip4 |= load_word_ntoh(ctx->user_ip4, 0, sizeof(user_ip4));
> + user_ip4 |= load_word_ntoh(ctx->user_ip4, 1, sizeof(user_ip4));
> if (ctx->user_ip4 != user_ip4)
> return 0;
>
> diff --git a/tools/testing/selftests/bpf/progs/bind6_prog.c b/tools/testing/selftests/bpf/progs/bind6_prog.c
> index d62cd9e9cf0ea..194583e3375bf 100644
> --- a/tools/testing/selftests/bpf/progs/bind6_prog.c
> +++ b/tools/testing/selftests/bpf/progs/bind6_prog.c
> @@ -12,6 +12,8 @@
> #include <bpf/bpf_helpers.h>
> #include <bpf/bpf_endian.h>
>
> +#include "bind_prog.h"
> +
> #define SERV6_IP_0 0xfaceb00c /* face:b00c:1234:5678::abcd */
> #define SERV6_IP_1 0x12345678
> #define SERV6_IP_2 0x00000000
> @@ -129,25 +131,25 @@ int bind_v6_prog(struct bpf_sock_addr *ctx)
> // u8 narrow loads:
> for (i = 0; i < 4; i++) {
> user_ip6 = 0;
> - user_ip6 |= ((volatile __u8 *)&ctx->user_ip6[i])[0] << 0;
> - user_ip6 |= ((volatile __u8 *)&ctx->user_ip6[i])[1] << 8;
> - user_ip6 |= ((volatile __u8 *)&ctx->user_ip6[i])[2] << 16;
> - user_ip6 |= ((volatile __u8 *)&ctx->user_ip6[i])[3] << 24;
> + user_ip6 |= load_byte_ntoh(ctx->user_ip6[i], 0, sizeof(user_ip6));
> + user_ip6 |= load_byte_ntoh(ctx->user_ip6[i], 1, sizeof(user_ip6));
> + user_ip6 |= load_byte_ntoh(ctx->user_ip6[i], 2, sizeof(user_ip6));
> + user_ip6 |= load_byte_ntoh(ctx->user_ip6[i], 3, sizeof(user_ip6));
> if (ctx->user_ip6[i] != user_ip6)
> return 0;
> }
>
> user_port = 0;
> - user_port |= ((volatile __u8 *)&ctx->user_port)[0] << 0;
> - user_port |= ((volatile __u8 *)&ctx->user_port)[1] << 8;
> + user_port |= load_byte_ntoh(ctx->user_port, 0, sizeof(user_port));
> + user_port |= load_byte_ntoh(ctx->user_port, 1, sizeof(user_port));
> if (ctx->user_port != user_port)
> return 0;
>
> // u16 narrow loads:
> for (i = 0; i < 4; i++) {
> user_ip6 = 0;
> - user_ip6 |= ((volatile __u16 *)&ctx->user_ip6[i])[0] << 0;
> - user_ip6 |= ((volatile __u16 *)&ctx->user_ip6[i])[1] << 16;
> + user_ip6 |= load_word_ntoh(ctx->user_ip6[i], 0, sizeof(user_ip6));
> + user_ip6 |= load_word_ntoh(ctx->user_ip6[i], 1, sizeof(user_ip6));
> if (ctx->user_ip6[i] != user_ip6)
> return 0;
> }
> diff --git a/tools/testing/selftests/bpf/progs/bind_prog.h b/tools/testing/selftests/bpf/progs/bind_prog.h
> new file mode 100644
> index 0000000000000..0fdc466aec346
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/bind_prog.h
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __BIND_PROG_H__
> +#define __BIND_PROG_H__
> +
> +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
> +#define load_byte_ntoh(src, b, s) \
> + (((volatile __u8 *)&(src))[b] << 8 * b)
> +#define load_word_ntoh(src, w, s) \
> + (((volatile __u16 *)&(src))[w] << 16 * w)
> +#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
> +#define load_byte_ntoh(src, b, s) \
> + (((volatile __u8 *)&(src))[(b) + (sizeof(src) - (s))] << 8 * ((s) - (b) - 1))
> +#define load_word_ntoh(src, w, s) \
> + (((volatile __u16 *)&(src))[w] << 16 * (((s) / 2) - (w) - 1))
These names, load_byte_ntoh() and load_word_ntoh(), are miss-leading.
They don't actually do byte-order conversion from network order to host
order. Network order is big endian. 0xdeadbeef in u32 should be stored
as the sequence of
0xde, 0xad, 0xbe, 0xef
The little endian implementation of load_word_ntoh() provided here will
return 0xadde and 0xefbe0000. However, a network order to host order
conversion should return 0xbeef and 0xdead0000 for little endian.
The little endian implementation of load_byte_ntoh() here returns 0xde,
0xad00, 0xbe0000, and 0xef000000. However, a network to host order
conversion should return 0xef, 0xbe00, 0xad0000, and 0xde00000.
So, they just access raw data following the host byte order, not
providing any byte order conversion.
> +#else
> +# error "Fix your compiler's __BYTE_ORDER__?!"
> +#endif
> +
> +#endif
Powered by blists - more mailing lists