bpf: propagate __user annotations properly #489
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Master branch: 34da872 |
__htab_map_lookup_and_delete_batch() stores a user pointer in the local
variable ubatch and uses that in copy_{from,to}_user(), but ubatch misses a
__user annotation.
So, sparse warns in the various assignments and uses of ubatch:
kernel/bpf/hashtab.c:1415:24: warning: incorrect type in initializer
(different address spaces)
kernel/bpf/hashtab.c:1415:24: expected void *ubatch
kernel/bpf/hashtab.c:1415:24: got void [noderef] __user *
kernel/bpf/hashtab.c:1444:46: warning: incorrect type in argument 2
(different address spaces)
kernel/bpf/hashtab.c:1444:46: expected void const [noderef] __user *from
kernel/bpf/hashtab.c:1444:46: got void *ubatch
kernel/bpf/hashtab.c:1608:16: warning: incorrect type in assignment
(different address spaces)
kernel/bpf/hashtab.c:1608:16: expected void *ubatch
kernel/bpf/hashtab.c:1608:16: got void [noderef] __user *
kernel/bpf/hashtab.c:1609:26: warning: incorrect type in argument 1
(different address spaces)
kernel/bpf/hashtab.c:1609:26: expected void [noderef] __user *to
kernel/bpf/hashtab.c:1609:26: got void *ubatch
Add the __user annotation to repair this chain of propagating __user
annotations in __htab_map_lookup_and_delete_batch().
Signed-off-by: Lukas Bulwahn <[email protected]>
Acked-by: Yonghong Song <[email protected]>
|
Master branch: e1868b9 |
kernel-patches-bot
force-pushed
the
series/397533=>bpf-next
branch
from
2d5c854 to
250885e
Compare
|
At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=397533 irrelevant now. Closing PR. |
kernel-patches-daemon-bpf bot
pushed a commit
that referenced
this pull request
Sep 5, 2024
Given we got rid of ARG_PTR_TO_LONG, change the test case description to avoid potential confusion: # ./vmtest.sh -- ./test_progs -t verifier_int_ptr [...] ./test_progs -t verifier_int_ptr [ 1.610563] bpf_testmod: loading out-of-tree module taints kernel. [ 1.611049] bpf_testmod: module verification failed: signature and/or required key missing - tainting kernel #489/1 verifier_int_ptr/arg pointer to long uninitialized:OK #489/2 verifier_int_ptr/arg pointer to long half-uninitialized:OK #489/3 verifier_int_ptr/arg pointer to long misaligned:OK #489/4 verifier_int_ptr/arg pointer to long size < sizeof(long):OK #489/5 verifier_int_ptr/arg pointer to long initialized:OK #489 verifier_int_ptr:OK Summary: 1/5 PASSED, 0 SKIPPED, 0 FAILED Signed-off-by: Daniel Borkmann <[email protected]>
kernel-patches-daemon-bpf bot
pushed a commit
that referenced
this pull request
Sep 5, 2024
Given we got rid of ARG_PTR_TO_LONG, change the test case description to avoid potential confusion: # ./vmtest.sh -- ./test_progs -t verifier_int_ptr [...] ./test_progs -t verifier_int_ptr [ 1.610563] bpf_testmod: loading out-of-tree module taints kernel. [ 1.611049] bpf_testmod: module verification failed: signature and/or required key missing - tainting kernel #489/1 verifier_int_ptr/arg pointer to long uninitialized:OK #489/2 verifier_int_ptr/arg pointer to long half-uninitialized:OK #489/3 verifier_int_ptr/arg pointer to long misaligned:OK #489/4 verifier_int_ptr/arg pointer to long size < sizeof(long):OK #489/5 verifier_int_ptr/arg pointer to long initialized:OK #489 verifier_int_ptr:OK Summary: 1/5 PASSED, 0 SKIPPED, 0 FAILED Signed-off-by: Daniel Borkmann <[email protected]>
kernel-patches-daemon-bpf bot
pushed a commit
that referenced
this pull request
Sep 6, 2024
Given we got rid of ARG_PTR_TO_LONG, change the test case description to avoid potential confusion: # ./vmtest.sh -- ./test_progs -t verifier_int_ptr [...] ./test_progs -t verifier_int_ptr [ 1.610563] bpf_testmod: loading out-of-tree module taints kernel. [ 1.611049] bpf_testmod: module verification failed: signature and/or required key missing - tainting kernel #489/1 verifier_int_ptr/arg pointer to long uninitialized:OK #489/2 verifier_int_ptr/arg pointer to long half-uninitialized:OK #489/3 verifier_int_ptr/arg pointer to long misaligned:OK #489/4 verifier_int_ptr/arg pointer to long size < sizeof(long):OK #489/5 verifier_int_ptr/arg pointer to long initialized:OK #489 verifier_int_ptr:OK Summary: 1/5 PASSED, 0 SKIPPED, 0 FAILED Signed-off-by: Daniel Borkmann <[email protected]>
kernel-patches-daemon-bpf bot
pushed a commit
that referenced
this pull request
Sep 6, 2024
Given we got rid of ARG_PTR_TO_LONG, change the test case description to avoid potential confusion: # ./vmtest.sh -- ./test_progs -t verifier_int_ptr [...] ./test_progs -t verifier_int_ptr [ 1.610563] bpf_testmod: loading out-of-tree module taints kernel. [ 1.611049] bpf_testmod: module verification failed: signature and/or required key missing - tainting kernel #489/1 verifier_int_ptr/arg pointer to long uninitialized:OK #489/2 verifier_int_ptr/arg pointer to long half-uninitialized:OK #489/3 verifier_int_ptr/arg pointer to long misaligned:OK #489/4 verifier_int_ptr/arg pointer to long size < sizeof(long):OK #489/5 verifier_int_ptr/arg pointer to long initialized:OK #489 verifier_int_ptr:OK Summary: 1/5 PASSED, 0 SKIPPED, 0 FAILED Signed-off-by: Daniel Borkmann <[email protected]>
kernel-patches-daemon-bpf bot
pushed a commit
that referenced
this pull request
Sep 13, 2024
Given we got rid of ARG_PTR_TO_LONG, change the test case description to avoid potential confusion: # ./vmtest.sh -- ./test_progs -t verifier_int_ptr [...] ./test_progs -t verifier_int_ptr [ 1.610563] bpf_testmod: loading out-of-tree module taints kernel. [ 1.611049] bpf_testmod: module verification failed: signature and/or required key missing - tainting kernel #489/1 verifier_int_ptr/arg pointer to long uninitialized:OK #489/2 verifier_int_ptr/arg pointer to long half-uninitialized:OK #489/3 verifier_int_ptr/arg pointer to long misaligned:OK #489/4 verifier_int_ptr/arg pointer to long size < sizeof(long):OK #489/5 verifier_int_ptr/arg pointer to long initialized:OK #489 verifier_int_ptr:OK Summary: 1/5 PASSED, 0 SKIPPED, 0 FAILED Signed-off-by: Daniel Borkmann <[email protected]>
kernel-patches-daemon-bpf bot
pushed a commit
that referenced
this pull request
Sep 13, 2024
Given we got rid of ARG_PTR_TO_LONG, change the test case description to avoid potential confusion: # ./vmtest.sh -- ./test_progs -t verifier_int_ptr [...] ./test_progs -t verifier_int_ptr [ 1.610563] bpf_testmod: loading out-of-tree module taints kernel. [ 1.611049] bpf_testmod: module verification failed: signature and/or required key missing - tainting kernel #489/1 verifier_int_ptr/arg pointer to long uninitialized:OK #489/2 verifier_int_ptr/arg pointer to long half-uninitialized:OK #489/3 verifier_int_ptr/arg pointer to long misaligned:OK #489/4 verifier_int_ptr/arg pointer to long size < sizeof(long):OK #489/5 verifier_int_ptr/arg pointer to long initialized:OK #489 verifier_int_ptr:OK Summary: 1/5 PASSED, 0 SKIPPED, 0 FAILED Signed-off-by: Daniel Borkmann <[email protected]>
kernel-patches-daemon-bpf bot
pushed a commit
that referenced
this pull request
Sep 13, 2024
Given we got rid of ARG_PTR_TO_LONG, change the test case description to avoid potential confusion: # ./vmtest.sh -- ./test_progs -t verifier_int_ptr [...] ./test_progs -t verifier_int_ptr [ 1.610563] bpf_testmod: loading out-of-tree module taints kernel. [ 1.611049] bpf_testmod: module verification failed: signature and/or required key missing - tainting kernel #489/1 verifier_int_ptr/arg pointer to long uninitialized:OK #489/2 verifier_int_ptr/arg pointer to long half-uninitialized:OK #489/3 verifier_int_ptr/arg pointer to long misaligned:OK #489/4 verifier_int_ptr/arg pointer to long size < sizeof(long):OK #489/5 verifier_int_ptr/arg pointer to long initialized:OK #489 verifier_int_ptr:OK Summary: 1/5 PASSED, 0 SKIPPED, 0 FAILED Signed-off-by: Daniel Borkmann <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Alexei Starovoitov <[email protected]>
kernel-patches-daemon-bpf bot
pushed a commit
that referenced
this pull request
Sep 20, 2025
With latest llvm22, I got the following verification failure: ... ; int big_alloc2(void *ctx) @ verifier_arena_large.c:207 0: (b4) w6 = 1 ; R6_w=1 ... ; if (err) @ verifier_arena_large.c:233 53: (56) if w6 != 0x0 goto pc+62 ; R6=0 54: (b7) r7 = -4 ; R7_w=-4 55: (18) r8 = 0x7f4000000000 ; R8_w=scalar() 57: (bf) r9 = addr_space_cast(r8, 0, 1) ; R8_w=scalar() R9_w=arena 58: (b4) w6 = 5 ; R6_w=5 ; pg = page[i]; @ verifier_arena_large.c:238 59: (bf) r1 = r7 ; R1_w=-4 R7_w=-4 60: (07) r1 += 4 ; R1_w=0 61: (79) r2 = *(u64 *)(r9 +0) ; R2_w=scalar() R9_w=arena ; if (*pg != i) @ verifier_arena_large.c:239 62: (bf) r3 = addr_space_cast(r2, 0, 1) ; R2_w=scalar() R3_w=arena 63: (71) r3 = *(u8 *)(r3 +0) ; R3_w=scalar(smin=smin32=0,smax=umax=smax32=umax32=255,var_off=(0x0; 0xff)) 64: (5d) if r1 != r3 goto pc+51 ; R1_w=0 R3_w=0 ; bpf_arena_free_pages(&arena, (void __arena *)pg, 2); @ verifier_arena_large.c:241 65: (18) r1 = 0xff11000114548000 ; R1_w=map_ptr(map=arena,ks=0,vs=0) 67: (b4) w3 = 2 ; R3_w=2 68: (85) call bpf_arena_free_pages#72675 ; 69: (b7) r1 = 0 ; R1_w=0 ; page[i + 1] = NULL; @ verifier_arena_large.c:243 70: (7b) *(u64 *)(r8 +8) = r1 R8 invalid mem access 'scalar' processed 61 insns (limit 1000000) max_states_per_insn 0 total_states 6 peak_states 6 mark_read 2 ============= #489/5 verifier_arena_large/big_alloc2:FAIL The main reason is that 'r8' in insn '70' is not an arena pointer. Further debugging at llvm side shows that llvm commit ([1]) caused the failure. For the original code: page[i] = NULL; page[i + 1] = NULL; the llvm transformed it to something like below at source level: __builtin_memset(&page[i], 0, 16) Such transformation prevents llvm BPFCheckAndAdjustIR pass from generating proper addr_space_cast insns ([2]). Adding support in llvm BPFCheckAndAdjustIR pass should work, but not sure that such a pattern exists or not in real applications. At the same time, simply adding a memory barrier between two 'page' assignment can fix the issue. [1] llvm/llvm-project#155415 [2] llvm/llvm-project#84410 Cc: Eduard Zingerman <[email protected]> Signed-off-by: Yonghong Song <[email protected]>
kernel-patches-daemon-bpf bot
pushed a commit
that referenced
this pull request
Sep 20, 2025
With latest llvm22, I got the following verification failure: ... ; int big_alloc2(void *ctx) @ verifier_arena_large.c:207 0: (b4) w6 = 1 ; R6_w=1 ... ; if (err) @ verifier_arena_large.c:233 53: (56) if w6 != 0x0 goto pc+62 ; R6=0 54: (b7) r7 = -4 ; R7_w=-4 55: (18) r8 = 0x7f4000000000 ; R8_w=scalar() 57: (bf) r9 = addr_space_cast(r8, 0, 1) ; R8_w=scalar() R9_w=arena 58: (b4) w6 = 5 ; R6_w=5 ; pg = page[i]; @ verifier_arena_large.c:238 59: (bf) r1 = r7 ; R1_w=-4 R7_w=-4 60: (07) r1 += 4 ; R1_w=0 61: (79) r2 = *(u64 *)(r9 +0) ; R2_w=scalar() R9_w=arena ; if (*pg != i) @ verifier_arena_large.c:239 62: (bf) r3 = addr_space_cast(r2, 0, 1) ; R2_w=scalar() R3_w=arena 63: (71) r3 = *(u8 *)(r3 +0) ; R3_w=scalar(smin=smin32=0,smax=umax=smax32=umax32=255,var_off=(0x0; 0xff)) 64: (5d) if r1 != r3 goto pc+51 ; R1_w=0 R3_w=0 ; bpf_arena_free_pages(&arena, (void __arena *)pg, 2); @ verifier_arena_large.c:241 65: (18) r1 = 0xff11000114548000 ; R1_w=map_ptr(map=arena,ks=0,vs=0) 67: (b4) w3 = 2 ; R3_w=2 68: (85) call bpf_arena_free_pages#72675 ; 69: (b7) r1 = 0 ; R1_w=0 ; page[i + 1] = NULL; @ verifier_arena_large.c:243 70: (7b) *(u64 *)(r8 +8) = r1 R8 invalid mem access 'scalar' processed 61 insns (limit 1000000) max_states_per_insn 0 total_states 6 peak_states 6 mark_read 2 ============= #489/5 verifier_arena_large/big_alloc2:FAIL The main reason is that 'r8' in insn '70' is not an arena pointer. Further debugging at llvm side shows that llvm commit ([1]) caused the failure. For the original code: page[i] = NULL; page[i + 1] = NULL; the llvm transformed it to something like below at source level: __builtin_memset(&page[i], 0, 16) Such transformation prevents llvm BPFCheckAndAdjustIR pass from generating proper addr_space_cast insns ([2]). Adding support in llvm BPFCheckAndAdjustIR pass should work, but not sure that such a pattern exists or not in real applications. At the same time, simply adding a memory barrier between two 'page' assignment can fix the issue. [1] llvm/llvm-project#155415 [2] llvm/llvm-project#84410 Cc: Eduard Zingerman <[email protected]> Signed-off-by: Yonghong Song <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Alexei Starovoitov <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull request for series with
subject: bpf: propagate __user annotations properly
version: 1
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=397533