bpf: Allow narrow loads of bpf_sysctl fields with offset > 0
"ctx:file_pos sysctl:read read ok narrow" works on s390 by accident: it reads the wrong byte, which happens to have the expected value of 0. Improve the test by seeking to the 4th byte and expecting 4 instead of 0. This makes the latent problem apparent: the test attempts to read the first byte of bpf_sysctl.file_pos, assuming this is the least-significant byte, which is not the case on big-endian machines: a non-zero offset is needed. The point of the test is to verify narrow loads, so we cannot cheat our way out by simply using BPF_W. The existence of the test means that such loads have to be supported, most likely because llvm can generate them. Fix the test by adding a big-endian variant, which uses an offset to access the least-significant byte of bpf_sysctl.file_pos. This reveals the final problem: verifier rejects accesses to bpf_sysctl fields with offset > 0. Such accesses are already allowed for a wide range of structs: __sk_buff, bpf_sock_addr and sk_msg_md to name a few. Extend this support to bpf_sysctl by using bpf_ctx_range instead of offsetof when matching field offsets. Fixes:7b146cebe3
("bpf: Sysctl hook") Fixes:e1550bfe0d
("bpf: Add file_pos field to bpf_sysctl ctx") Fixes:9a1027e525
("selftests/bpf: Test file_pos field in bpf_sysctl ctx") Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> Signed-off-by: Alexei Starovoitov <ast@kernel.org> Acked-by: Andrey Ignatov <rdna@fb.com> Acked-by: Andrii Nakryiko <andriin@fb.com> Link: https://lore.kernel.org/bpf/20191028122902.9763-1-iii@linux.ibm.com
This commit is contained in:
parent
050668c100
commit
7541c87c9b
|
@ -1311,12 +1311,12 @@ static bool sysctl_is_valid_access(int off, int size, enum bpf_access_type type,
|
||||||
return false;
|
return false;
|
||||||
|
|
||||||
switch (off) {
|
switch (off) {
|
||||||
case offsetof(struct bpf_sysctl, write):
|
case bpf_ctx_range(struct bpf_sysctl, write):
|
||||||
if (type != BPF_READ)
|
if (type != BPF_READ)
|
||||||
return false;
|
return false;
|
||||||
bpf_ctx_record_field_size(info, size_default);
|
bpf_ctx_record_field_size(info, size_default);
|
||||||
return bpf_ctx_narrow_access_ok(off, size, size_default);
|
return bpf_ctx_narrow_access_ok(off, size, size_default);
|
||||||
case offsetof(struct bpf_sysctl, file_pos):
|
case bpf_ctx_range(struct bpf_sysctl, file_pos):
|
||||||
if (type == BPF_READ) {
|
if (type == BPF_READ) {
|
||||||
bpf_ctx_record_field_size(info, size_default);
|
bpf_ctx_record_field_size(info, size_default);
|
||||||
return bpf_ctx_narrow_access_ok(off, size, size_default);
|
return bpf_ctx_narrow_access_ok(off, size, size_default);
|
||||||
|
|
|
@ -161,9 +161,14 @@ static struct sysctl_test tests[] = {
|
||||||
.descr = "ctx:file_pos sysctl:read read ok narrow",
|
.descr = "ctx:file_pos sysctl:read read ok narrow",
|
||||||
.insns = {
|
.insns = {
|
||||||
/* If (file_pos == X) */
|
/* If (file_pos == X) */
|
||||||
|
#if __BYTE_ORDER == __LITTLE_ENDIAN
|
||||||
BPF_LDX_MEM(BPF_B, BPF_REG_7, BPF_REG_1,
|
BPF_LDX_MEM(BPF_B, 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),
|
#else
|
||||||
|
BPF_LDX_MEM(BPF_B, BPF_REG_7, BPF_REG_1,
|
||||||
|
offsetof(struct bpf_sysctl, file_pos) + 3),
|
||||||
|
#endif
|
||||||
|
BPF_JMP_IMM(BPF_JNE, BPF_REG_7, 4, 2),
|
||||||
|
|
||||||
/* return ALLOW; */
|
/* return ALLOW; */
|
||||||
BPF_MOV64_IMM(BPF_REG_0, 1),
|
BPF_MOV64_IMM(BPF_REG_0, 1),
|
||||||
|
@ -176,6 +181,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 = 4,
|
||||||
.result = SUCCESS,
|
.result = SUCCESS,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
|
|
Loading…
Reference in New Issue