From d895a0f16fadb26d22ab531c49768f7642ae5c3e Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Fri, 16 Aug 2019 12:53:00 +0200 Subject: [PATCH] bpf: fix accessing bpf_sysctl.file_pos on s390 "ctx:file_pos sysctl:read write ok" fails on s390 with "Read value != nux". This is because verifier rewrites a complete 32-bit bpf_sysctl.file_pos update to a partial update of the first 32 bits of 64-bit *bpf_sysctl_kern.ppos, which is not correct on big-endian systems. Fix by using an offset on big-endian systems. Ditto for bpf_sysctl.file_pos reads. Currently the test does not detect a problem there, since it expects to see 0, which it gets with high probability in error cases, so change it to seek to offset 3 and expect 3 in bpf_sysctl.file_pos. Fixes: e1550bfe0de4 ("bpf: Add file_pos field to bpf_sysctl ctx") Signed-off-by: Ilya Leoshkevich Acked-by: Yonghong Song Signed-off-by: Daniel Borkmann Link: https://lore.kernel.org/bpf/20190816105300.49035-1-iii@linux.ibm.com/ --- include/linux/filter.h | 8 ++++---- kernel/bpf/cgroup.c | 10 ++++++++-- kernel/bpf/verifier.c | 4 ++-- tools/testing/selftests/bpf/test_sysctl.c | 9 ++++++++- 4 files changed, 22 insertions(+), 9 deletions(-) diff --git a/include/linux/filter.h b/include/linux/filter.h index 92c6e31..2ce5764 100644 --- a/include/linux/filter.h +++ b/include/linux/filter.h @@ -749,14 +749,14 @@ bpf_ctx_narrow_access_ok(u32 off, u32 size, u32 size_default) } static inline u8 -bpf_ctx_narrow_load_shift(u32 off, u32 size, u32 size_default) +bpf_ctx_narrow_access_offset(u32 off, u32 size, u32 size_default) { - u8 load_off = off & (size_default - 1); + u8 access_off = off & (size_default - 1); #ifdef __LITTLE_ENDIAN - return load_off * 8; + return access_off; #else - return (size_default - (load_off + size)) * 8; + return size_default - (access_off + size); #endif } diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c index 6a6a154..ddd8add 100644 --- a/kernel/bpf/cgroup.c +++ b/kernel/bpf/cgroup.c @@ -1334,6 +1334,7 @@ static u32 sysctl_convert_ctx_access(enum bpf_access_type type, struct bpf_prog *prog, u32 *target_size) { struct bpf_insn *insn = insn_buf; + u32 read_size; switch (si->off) { case offsetof(struct bpf_sysctl, write): @@ -1365,7 +1366,9 @@ static u32 sysctl_convert_ctx_access(enum bpf_access_type type, treg, si->dst_reg, offsetof(struct bpf_sysctl_kern, ppos)); *insn++ = BPF_STX_MEM( - BPF_SIZEOF(u32), treg, si->src_reg, 0); + BPF_SIZEOF(u32), treg, si->src_reg, + bpf_ctx_narrow_access_offset( + 0, sizeof(u32), sizeof(loff_t))); *insn++ = BPF_LDX_MEM( BPF_DW, treg, si->dst_reg, offsetof(struct bpf_sysctl_kern, tmp_reg)); @@ -1374,8 +1377,11 @@ static u32 sysctl_convert_ctx_access(enum bpf_access_type type, BPF_FIELD_SIZEOF(struct bpf_sysctl_kern, ppos), si->dst_reg, si->src_reg, offsetof(struct bpf_sysctl_kern, ppos)); + read_size = bpf_size_to_bytes(BPF_SIZE(si->code)); *insn++ = BPF_LDX_MEM( - BPF_SIZE(si->code), si->dst_reg, si->dst_reg, 0); + BPF_SIZE(si->code), si->dst_reg, si->dst_reg, + bpf_ctx_narrow_access_offset( + 0, read_size, sizeof(loff_t))); } *target_size = sizeof(u32); break; diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 3fb5075..92a4332 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -8619,8 +8619,8 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env) } if (is_narrower_load && size < target_size) { - u8 shift = bpf_ctx_narrow_load_shift(off, size, - size_default); + u8 shift = bpf_ctx_narrow_access_offset( + off, size, size_default) * 8; if (ctx_field_size <= 4) { if (shift) insn_buf[cnt++] = BPF_ALU32_IMM(BPF_RSH, diff --git a/tools/testing/selftests/bpf/test_sysctl.c b/tools/testing/selftests/bpf/test_sysctl.c index fc33ae3..4f8ec1f 100644 --- a/tools/testing/selftests/bpf/test_sysctl.c +++ b/tools/testing/selftests/bpf/test_sysctl.c @@ -32,6 +32,7 @@ struct sysctl_test { enum bpf_attach_type attach_type; const char *sysctl; int open_flags; + int seek; const char *newval; const char *oldval; enum { @@ -140,7 +141,7 @@ static struct sysctl_test tests[] = { /* If (file_pos == X) */ BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_1, offsetof(struct bpf_sysctl, file_pos)), - BPF_JMP_IMM(BPF_JNE, BPF_REG_7, 0, 2), + BPF_JMP_IMM(BPF_JNE, BPF_REG_7, 3, 2), /* return ALLOW; */ BPF_MOV64_IMM(BPF_REG_0, 1), @@ -153,6 +154,7 @@ static struct sysctl_test tests[] = { .attach_type = BPF_CGROUP_SYSCTL, .sysctl = "kernel/ostype", .open_flags = O_RDONLY, + .seek = 3, .result = SUCCESS, }, { @@ -1481,6 +1483,11 @@ static int access_sysctl(const char *sysctl_path, if (fd < 0) return fd; + if (test->seek && lseek(fd, test->seek, SEEK_SET) == -1) { + log_err("lseek(%d) failed", test->seek); + goto err; + } + if (test->open_flags == O_RDONLY) { char buf[128]; -- 2.7.4