Commit d895a0f1 authored by Ilya Leoshkevich's avatar Ilya Leoshkevich Committed by Daniel Borkmann

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: e1550bfe ("bpf: Add file_pos field to bpf_sysctl ctx")
Signed-off-by: default avatarIlya Leoshkevich <iii@linux.ibm.com>
Acked-by: default avatarYonghong Song <yhs@fb.com>
Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
Link: https://lore.kernel.org/bpf/20190816105300.49035-1-iii@linux.ibm.com/
parent af58e7ee
...@@ -749,14 +749,14 @@ bpf_ctx_narrow_access_ok(u32 off, u32 size, u32 size_default) ...@@ -749,14 +749,14 @@ bpf_ctx_narrow_access_ok(u32 off, u32 size, u32 size_default)
} }
static inline u8 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 #ifdef __LITTLE_ENDIAN
return load_off * 8; return access_off;
#else #else
return (size_default - (load_off + size)) * 8; return size_default - (access_off + size);
#endif #endif
} }
......
...@@ -1334,6 +1334,7 @@ static u32 sysctl_convert_ctx_access(enum bpf_access_type type, ...@@ -1334,6 +1334,7 @@ static u32 sysctl_convert_ctx_access(enum bpf_access_type type,
struct bpf_prog *prog, u32 *target_size) struct bpf_prog *prog, u32 *target_size)
{ {
struct bpf_insn *insn = insn_buf; struct bpf_insn *insn = insn_buf;
u32 read_size;
switch (si->off) { switch (si->off) {
case offsetof(struct bpf_sysctl, write): case offsetof(struct bpf_sysctl, write):
...@@ -1365,7 +1366,9 @@ static u32 sysctl_convert_ctx_access(enum bpf_access_type type, ...@@ -1365,7 +1366,9 @@ static u32 sysctl_convert_ctx_access(enum bpf_access_type type,
treg, si->dst_reg, treg, si->dst_reg,
offsetof(struct bpf_sysctl_kern, ppos)); offsetof(struct bpf_sysctl_kern, ppos));
*insn++ = BPF_STX_MEM( *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( *insn++ = BPF_LDX_MEM(
BPF_DW, treg, si->dst_reg, BPF_DW, treg, si->dst_reg,
offsetof(struct bpf_sysctl_kern, tmp_reg)); offsetof(struct bpf_sysctl_kern, tmp_reg));
...@@ -1374,8 +1377,11 @@ static u32 sysctl_convert_ctx_access(enum bpf_access_type type, ...@@ -1374,8 +1377,11 @@ static u32 sysctl_convert_ctx_access(enum bpf_access_type type,
BPF_FIELD_SIZEOF(struct bpf_sysctl_kern, ppos), BPF_FIELD_SIZEOF(struct bpf_sysctl_kern, ppos),
si->dst_reg, si->src_reg, si->dst_reg, si->src_reg,
offsetof(struct bpf_sysctl_kern, ppos)); offsetof(struct bpf_sysctl_kern, ppos));
read_size = bpf_size_to_bytes(BPF_SIZE(si->code));
*insn++ = BPF_LDX_MEM( *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); *target_size = sizeof(u32);
break; break;
......
...@@ -8619,8 +8619,8 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env) ...@@ -8619,8 +8619,8 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
} }
if (is_narrower_load && size < target_size) { if (is_narrower_load && size < target_size) {
u8 shift = bpf_ctx_narrow_load_shift(off, size, u8 shift = bpf_ctx_narrow_access_offset(
size_default); off, size, size_default) * 8;
if (ctx_field_size <= 4) { if (ctx_field_size <= 4) {
if (shift) if (shift)
insn_buf[cnt++] = BPF_ALU32_IMM(BPF_RSH, insn_buf[cnt++] = BPF_ALU32_IMM(BPF_RSH,
......
...@@ -32,6 +32,7 @@ struct sysctl_test { ...@@ -32,6 +32,7 @@ struct sysctl_test {
enum bpf_attach_type attach_type; enum bpf_attach_type attach_type;
const char *sysctl; const char *sysctl;
int open_flags; int open_flags;
int seek;
const char *newval; const char *newval;
const char *oldval; const char *oldval;
enum { enum {
...@@ -140,7 +141,7 @@ static struct sysctl_test tests[] = { ...@@ -140,7 +141,7 @@ static struct sysctl_test tests[] = {
/* If (file_pos == X) */ /* If (file_pos == X) */
BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_1, BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_1,
offsetof(struct bpf_sysctl, file_pos)), 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; */ /* return ALLOW; */
BPF_MOV64_IMM(BPF_REG_0, 1), BPF_MOV64_IMM(BPF_REG_0, 1),
...@@ -153,6 +154,7 @@ static struct sysctl_test tests[] = { ...@@ -153,6 +154,7 @@ static struct sysctl_test tests[] = {
.attach_type = BPF_CGROUP_SYSCTL, .attach_type = BPF_CGROUP_SYSCTL,
.sysctl = "kernel/ostype", .sysctl = "kernel/ostype",
.open_flags = O_RDONLY, .open_flags = O_RDONLY,
.seek = 3,
.result = SUCCESS, .result = SUCCESS,
}, },
{ {
...@@ -1481,6 +1483,11 @@ static int access_sysctl(const char *sysctl_path, ...@@ -1481,6 +1483,11 @@ static int access_sysctl(const char *sysctl_path,
if (fd < 0) if (fd < 0)
return fd; 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) { if (test->open_flags == O_RDONLY) {
char buf[128]; char buf[128];
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment