Skip to content

Commit 78ae538

Browse files
committed
bpf: Fix test errors with *_is_valid_access() verifier hooks on 32-bit
Sequence of ctx checks and rewrites in real world: 1. Possible CO-RE rewrites of access size & offset, maybe breaking #2 2. Verifier env->ops->is_valid_access(), testing access size == sizeof(u64) 3. Verifier env->ops->convert_ctx_access(), rewrite size & offset Position of access check above is strange, really only works on 64-bit and likely unnoticed for lack of systematic 32-bit testing. Test changing *is_valid_access() to always check size != sizeof(u64), but on 32-bit systems also check size != sizeof(u32) On 32-bit armhf, test_progs hits ~100 instances of failures such as: (NOTE: this results from CO-RE relocation patching to u32 load size) libbpf: prog 'change_tcp_cc': -- BEGIN PROG LOAD LOG -- 0: R1=ctx() R10=fp0 ; int change_tcp_cc(struct bpf_iter__tcp *ctx) @ bpf_iter_setsockopt.c:40 0: (b4) w2 = 0 ; R2_w=0 ; if (!bpf_tcp_sk(ctx->sk_common)) @ bpf_iter_setsockopt.c:46 1: (61) r1 = *(u32 *)(r1 +8) func 'bpf_iter_tcp' size 4 must be 8 invalid bpf_context access off=8 size=4 is_valid_access=tracing_prog_is_valid_access processed 2 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0 -- END PROG LOAD LOG -- libbpf: prog 'change_tcp_cc': failed to load: -EACCES libbpf: failed to load object 'bpf_iter_setsockopt' libbpf: failed to load BPF skeleton 'bpf_iter_setsockopt': -EACCES serial_test_bpf_iter_setsockopt:FAIL:iter_skel unexpected error: -13 torvalds#21 bpf_iter_setsockopt:FAIL (NOTE: no error for tests without CO-RE patching which have u64 load size) This means *_is_valid_access() can't check ctx pointers simply using: if (size != sizeof(__u64)) return false; or if (size != sizeof(void *) return false; And what's required instead is a combo like: if (size != sizeof(__u64) && size != sizeof(long)) return false; Implement above as convenience function and use in: - btf_ctx_access() - cg_sockopt_is_valid_access() - bpf_skb_is_valid_access() - sock_addr_is_valid_access() - sock_ops_is_valid_access() - sk_msg_is_valid_access() - flow_dissector_is_valid_access() This eliminates all 'invalid bpf_context access" errors on 32-bit armhf except one with nf_is_valid_access() which is fixed in the next patch. Signed-off-by: Tony Ambardar <[email protected]>
1 parent 9a9760b commit 78ae538

File tree

3 files changed

+19
-15
lines changed

3 files changed

+19
-15
lines changed

kernel/bpf/btf.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6774,9 +6774,12 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
67746774
return false;
67756775
}
67766776

6777-
if (size != sizeof(u64)) {
6778-
bpf_log(log, "func '%s' size %d must be 8\n",
6779-
tname, size);
6777+
/* check for invalid pointer size on either 64-bit/32-bit system */
6778+
if (size != sizeof(__u64) && size != sizeof(long)) {
6779+
char *exp = sizeof(long) == sizeof(__u64) ? "8" : "8 or 4";
6780+
6781+
bpf_log(log, "func '%s' size %d must be %s\n",
6782+
tname, size, exp);
67806783
return false;
67816784
}
67826785

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

net/core/filter.c

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8700,7 +8700,8 @@ static bool bpf_skb_is_valid_access(int off, int size, enum bpf_access_type type
87008700
return false;
87018701
break;
87028702
case bpf_ctx_range_ptr(struct __sk_buff, sk):
8703-
if (type == BPF_WRITE || size != sizeof(__u64))
8703+
if (type == BPF_WRITE ||
8704+
(size != sizeof(__u64) && size != sizeof(long)))
87048705
return false;
87058706
info->reg_type = PTR_TO_SOCK_COMMON_OR_NULL;
87068707
break;
@@ -9280,7 +9281,7 @@ static bool sock_addr_is_valid_access(int off, int size,
92809281
case bpf_ctx_range_ptr(struct bpf_sock_addr, sk):
92819282
if (type != BPF_READ)
92829283
return false;
9283-
if (size != sizeof(__u64))
9284+
if (size != sizeof(__u64) && size != sizeof(long))
92849285
return false;
92859286
info->reg_type = PTR_TO_SOCKET;
92869287
break;
@@ -9328,17 +9329,17 @@ static bool sock_ops_is_valid_access(int off, int size,
93289329
return false;
93299330
break;
93309331
case bpf_ctx_range_ptr(struct bpf_sock_ops, sk):
9331-
if (size != sizeof(__u64))
9332+
if (size != sizeof(__u64) && size != sizeof(long))
93329333
return false;
93339334
info->reg_type = PTR_TO_SOCKET_OR_NULL;
93349335
break;
93359336
case bpf_ctx_range_ptr(struct bpf_sock_ops, skb_data):
9336-
if (size != sizeof(__u64))
9337+
if (size != sizeof(__u64) && size != sizeof(long))
93379338
return false;
93389339
info->reg_type = PTR_TO_PACKET;
93399340
break;
93409341
case bpf_ctx_range_ptr(struct bpf_sock_ops, skb_data_end):
9341-
if (size != sizeof(__u64))
9342+
if (size != sizeof(__u64) && size != sizeof(long))
93429343
return false;
93439344
info->reg_type = PTR_TO_PACKET_END;
93449345
break;
@@ -9418,16 +9419,16 @@ static bool sk_msg_is_valid_access(int off, int size,
94189419
switch (off) {
94199420
case bpf_ctx_range_ptr(struct sk_msg_md, data):
94209421
info->reg_type = PTR_TO_PACKET;
9421-
if (size != sizeof(__u64))
9422+
if (size != sizeof(__u64) && size != sizeof(long))
94229423
return false;
94239424
break;
94249425
case bpf_ctx_range_ptr(struct sk_msg_md, data_end):
94259426
info->reg_type = PTR_TO_PACKET_END;
9426-
if (size != sizeof(__u64))
9427+
if (size != sizeof(__u64) && size != sizeof(long))
94279428
return false;
94289429
break;
94299430
case bpf_ctx_range_ptr(struct sk_msg_md, sk):
9430-
if (size != sizeof(__u64))
9431+
if (size != sizeof(__u64) && size != sizeof(long))
94319432
return false;
94329433
info->reg_type = PTR_TO_SOCKET;
94339434
break;
@@ -9476,7 +9477,7 @@ static bool flow_dissector_is_valid_access(int off, int size,
94769477
info->reg_type = PTR_TO_PACKET_END;
94779478
return true;
94789479
case bpf_ctx_range_ptr(struct __sk_buff, flow_keys):
9479-
if (size != sizeof(__u64))
9480+
if (size != sizeof(__u64) && size != sizeof(long))
94809481
return false;
94819482
info->reg_type = PTR_TO_FLOW_KEYS;
94829483
return true;

0 commit comments

Comments
 (0)