Skip to content

Commit 88789da

Browse files
pchaignoKernel Patches Daemon
authored andcommitted
bpf: Reject narrower access to pointer ctx fields
The following BPF program, simplified from a syzkaller repro, causes a kernel warning: r0 = *(u8 *)(r1 + 169); exit; With pointer field sk being at offset 168 in __sk_buff. This access is detected as a narrower read in bpf_skb_is_valid_access because it doesn't match offsetof(struct __sk_buff, sk). It is therefore allowed and later proceeds to bpf_convert_ctx_access. At that point, target_size is null and the verifier errors with a kernel warning and: verifier bug: error during ctx access conversion(1) This patch fixes that to return a proper "invalid bpf_context" error on the load instruction. The same issue affects the sk field in multiple context structure, as well as data and data_end in bpf_sock_ops and optval and optval_end in bpf_sockopt. Note this syzkaller crash was reported in [1], which used to be about a different bug, fixed in commit fce7bd8 ("bpf/verifier: Handle BPF_LOAD_ACQ instructions in insn_def_regno()"). Because syzbot somehow confused the two bugs, the new crash and repro didn't get reported to the mailing list. Link: https://syzkaller.appspot.com/bug?extid=0ef84a7bdf5301d4cbec [1] Fixes: f96da09 ("bpf: simplify narrower ctx access") Fixes: 0df1a55 ("bpf: Warn on internal verifier errors") Reported-by: [email protected] Signed-off-by: Paul Chaignon <[email protected]>
1 parent 95edab2 commit 88789da

File tree

2 files changed

+10
-10
lines changed

2 files changed

+10
-10
lines changed

kernel/bpf/cgroup.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2577,17 +2577,17 @@ static bool cg_sockopt_is_valid_access(int off, int size,
25772577
}
25782578

25792579
switch (off) {
2580-
case offsetof(struct bpf_sockopt, sk):
2580+
case bpf_ctx_range_ptr(struct bpf_sockopt, sk):
25812581
if (size != sizeof(__u64))
25822582
return false;
25832583
info->reg_type = PTR_TO_SOCKET;
25842584
break;
2585-
case offsetof(struct bpf_sockopt, optval):
2585+
case bpf_ctx_range_ptr(struct bpf_sockopt, optval):
25862586
if (size != sizeof(__u64))
25872587
return false;
25882588
info->reg_type = PTR_TO_PACKET;
25892589
break;
2590-
case offsetof(struct bpf_sockopt, optval_end):
2590+
case bpf_ctx_range_ptr(struct bpf_sockopt, optval_end):
25912591
if (size != sizeof(__u64))
25922592
return false;
25932593
info->reg_type = PTR_TO_PACKET_END;

net/core/filter.c

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8690,7 +8690,7 @@ static bool bpf_skb_is_valid_access(int off, int size, enum bpf_access_type type
86908690
if (size != sizeof(__u64))
86918691
return false;
86928692
break;
8693-
case offsetof(struct __sk_buff, sk):
8693+
case bpf_ctx_range_ptr(struct __sk_buff, sk):
86948694
if (type == BPF_WRITE || size != sizeof(__u64))
86958695
return false;
86968696
info->reg_type = PTR_TO_SOCK_COMMON_OR_NULL;
@@ -9268,7 +9268,7 @@ static bool sock_addr_is_valid_access(int off, int size,
92689268
return false;
92699269
}
92709270
break;
9271-
case offsetof(struct bpf_sock_addr, sk):
9271+
case bpf_ctx_range_ptr(struct bpf_sock_addr, sk):
92729272
if (type != BPF_READ)
92739273
return false;
92749274
if (size != sizeof(__u64))
@@ -9318,17 +9318,17 @@ static bool sock_ops_is_valid_access(int off, int size,
93189318
if (size != sizeof(__u64))
93199319
return false;
93209320
break;
9321-
case offsetof(struct bpf_sock_ops, sk):
9321+
case bpf_ctx_range_ptr(struct bpf_sock_ops, sk):
93229322
if (size != sizeof(__u64))
93239323
return false;
93249324
info->reg_type = PTR_TO_SOCKET_OR_NULL;
93259325
break;
9326-
case offsetof(struct bpf_sock_ops, skb_data):
9326+
case bpf_ctx_range_ptr(struct bpf_sock_ops, skb_data):
93279327
if (size != sizeof(__u64))
93289328
return false;
93299329
info->reg_type = PTR_TO_PACKET;
93309330
break;
9331-
case offsetof(struct bpf_sock_ops, skb_data_end):
9331+
case bpf_ctx_range_ptr(struct bpf_sock_ops, skb_data_end):
93329332
if (size != sizeof(__u64))
93339333
return false;
93349334
info->reg_type = PTR_TO_PACKET_END;
@@ -9417,7 +9417,7 @@ static bool sk_msg_is_valid_access(int off, int size,
94179417
if (size != sizeof(__u64))
94189418
return false;
94199419
break;
9420-
case offsetof(struct sk_msg_md, sk):
9420+
case bpf_ctx_range_ptr(struct sk_msg_md, sk):
94219421
if (size != sizeof(__u64))
94229422
return false;
94239423
info->reg_type = PTR_TO_SOCKET;
@@ -11623,7 +11623,7 @@ static bool sk_lookup_is_valid_access(int off, int size,
1162311623
return false;
1162411624

1162511625
switch (off) {
11626-
case offsetof(struct bpf_sk_lookup, sk):
11626+
case bpf_ctx_range_ptr(struct bpf_sk_lookup, sk):
1162711627
info->reg_type = PTR_TO_SOCKET_OR_NULL;
1162811628
return size == sizeof(__u64);
1162911629

0 commit comments

Comments
 (0)