bpf: Fix resetting logic for unreferenced kptrs
PATCH v2 1/1 bpf: Fix resetting logic for unreferenced kptrs
Sparse reported a warning at bpf_map_free_kptrs()
"warning: Using plain integer as NULL pointer"
During the process of fixing this warning,
it was discovered that the current code
erroneously writes to the pointer variable
instead of deferencing and writing to the actual kptr. Hence, Sparse tool accidentally helped to uncover this problem.
Fix this by doing WRITE_ONCE(*p, 0) instead of WRITE_ONCE(p, 0).
Note that the effect of this bug is that
unreferenced kptrs will not be cleared during check_and_free_fields. It is not a problem if the clearing is not done during map_free stage, as there is nothing to free for them.
---
Changes in v2:
- Make commit message clearer
- Change commit headline
from Fixes: 14a324f6a67e ("bpf: Wire up freeing of referenced kptr") to bpf: Fix resetting logic for unreferenced kptrs
kernel/bpf/syscall.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/bpf/syscall.c +1 -1
@@ -598,7 +598,7 @@ void bpf_map_free_kptrs(struct bpf_map *map, void *map_value)if (off_desc->type ==BPF_KPTR_UNREF ) {u64 *p = (u64 *)btf_id_ptr; -WRITE_ONCE (p,0 ); +WRITE_ONCE (*p,0 );continue ; } old_ptr =xchg (btf_id_ptr,0 );
Signature · 1 line
2.35.1
Kumar Kartikeya Dwivedi
maintainerQuoted 11 lines · Jules Irenge
Sparse reported a warning at
bpf_map_free_kptrs()"warning: Using plain integer as NULL pointer"
During the process of fixing this warning,
it was discovered that the current code
erroneously writes to the pointer variable
instead of deferencing and writing to the actual kptr. Hence, Sparse tool accidentally helped to uncover this problem.
Quoted 7 lines
Fix this by doing
WRITE_ONCE(*p, 0)instead ofWRITE_ONCE(p, 0).Note that the effect of this bug is that
unreferenced kptrs will not be cleared duringcheck_and_free_fields. It is not a problem if the clearing is not done duringmap_freestage, as there is nothing to free for them.
You're still missing the fixes tag right before your Signed-off-By.
Instead of
It must be
Quoted 23 lines · Kumar Kartikeya Dwivedi
Quoted 11 lines · Jules Irenge
Sparse reported a warning at
bpf_map_free_kptrs()"warning: Using plain integer as NULL pointer"
During the process of fixing this warning,
it was discovered that the current code
erroneously writes to the pointer variable
instead of deferencing and writing to the actual kptr. Hence, Sparse tool accidentally helped to uncover this problem.Quoted 7 lines
Fix this by doing
WRITE_ONCE(*p, 0)instead ofWRITE_ONCE(p, 0).Note that the effect of this bug is that
unreferenced kptrs will not be cleared duringcheck_and_free_fields. It is not a problem if the clearing is not done duringmap_freestage, as there is nothing to free for them.You're still missing the fixes tag right before your Signed-off-By.
Quoted 8 lines
Instead of
Signed-off-by Jules Irenge <[email protected]>It must be
Fixes 14a324f6a67e ("bpf: Wire up freeing of referenced kptr")Signed-off-by Jules Irenge <[email protected]>
Added while applying.
This patch was applied to bpf/bpf-next.git (master) by Alexei Starovoitov <[email protected]>:
Hello:
This patch was applied to bpf/bpf-next.git (master) by Alexei Starovoitov <[email protected]>:
Quoted 11 lines · On Wed, 7 Sep 2022 16:24:20 +0100 you wrote:
Sparse reported a warning at
bpf_map_free_kptrs()"warning: Using plain integer as NULL pointer"
During the process of fixing this warning,
it was discovered that the current code
erroneously writes to the pointer variable
instead of deferencing and writing to the actual kptr. Hence, Sparse tool accidentally helped to uncover this problem.[...]
Here is the summary with links:
- [bpf-next,v2] bpf: Fix resetting logic for unreferenced kptrs
https://git.kernel.org/bpf/bpf-next/c/9fad7fe5b298
You are awesome, thank you!
Signature · 2 lines
Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html