Skip to content

bpf: Free special fields when update [lru_,]percpu_hash maps

Leon Hwang bpf lkml 4 versions – In review

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v6 Latest2 patches 6 repliesIn review 1 of 2 applied
  2. v52 patches 2 repliesSuperseded
  3. v44 patches 6 repliesApplied
Earlier versions · 1
VersionPostedPatchesRepliesStatus
v34 patches14 repliesSuperseded 3 of 4 applied

Changes in v6 from the cover letter

* Update the test name to include "refcounted_kptr".
* Update some local variables' name in the test (per Alexei).
* v5: https://lore.kernel.org/bpf/[email protected]/

Patches in v6 1 of 2 applied

#Subject and filesSizeRepliesApplied
1/2bpf: Free special fields when update [lru_,]percpu_hash mapskernel/bpf/hashtab.c+8 -21 replyApplied
2/2selftests/bpf: Add test to verify freeing the special fields when update [lru_,]percpu_hash mapstools/testing/selftests/bpf/prog_tests/refcounted_kptr.c tools/testing/selftests/bpf/progs/refcounted_kptr.c+117 -05 replies

Reviews trailers per patch

All 2 patches have a Reviewed-by or Acked-by.

PatchYonghong Song
1/2A-b
2/2A-b

Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.

Read the v6 thread

Cover letter [PATCH bpf-next v6 0/2] bpf: Free special fields when update [lru_,]percpu_hash maps

In the discussion thread
"[PATCH bpf-next v9 0/7] bpf: Introduce BPF_F_CPU and BPF_F_ALL_CPUS flags for percpu maps"[1], it was pointed out that missing calls to bpf_obj_free_fields() could
lead to memory leaks.

A selftest was added to confirm that this is indeed a real issue - the refcount of BPF_KPTR_REF field is not decremented when bpf_obj_free_fields() is missing after copy_map_value[,_long]().

Further inspection of copy_map_value[,_long]() call sites revealed two locations affected by this issue:

1. pcpu_copy_value()
2. htab_map_update_elem() when used with BPF_F_LOCK

Similar case happens when update local storage maps with BPF_F_LOCK.

This series fixes the cases where BPF_F_LOCK is not involved by properly calling bpf_obj_free_fields() after copy_map_value[,_long](), and adds a selftest to verify the fix.

The remaining cases involving BPF_F_LOCK will be addressed in a separate patch set after the series
"bpf: Introduce BPF_F_CPU and BPF_F_ALL_CPUS flags for percpu maps" is applied.

Changes:
v5 -> v6:
* Update the test name to include "refcounted_kptr".
* Update some local variables' name in the test (per Alexei).
* v5: https://lore.kernel.org/bpf/[email protected]/

v4 -> v5:
* Use a local variable to store the this_cpu_ptr()/per_cpu_ptr() result, and reuse it between copy_map_value[,_long]() and bpf_obj_free_fields() in patch #1 (per Andrii).
* Drop patch #2 and #3, because the combination of BPF_F_LOCK with other special fields (except for BPF_SPIN_LOCK) will be disallowed on the UAPI side in the future (per Alexei).
* v4: https://lore.kernel.org/bpf/[email protected]/

v3 -> v4:
* Target bpf-next tree.
* Address comments from Amery:
* Drop 'bpf_obj_free_fields()' in the path of updating local storage maps without BPF_F_LOCK.
* Drop the corresponding self test.
* Respin the other test of local storage maps using syscall BPF programs.
* v3: https://lore.kernel.org/bpf/[email protected]/

v2 -> v3:
* Free special fields when update local storage maps without BPF_F_LOCK.
* Add test to verify decrementing refcount when update cgroup local storage maps without BPF_F_LOCK.
* Address review from AI bot:
* Slow path with BPF_F_LOCK (around line 642-646) in

    'bpf_local_storage.c'.

* v2: https://lore.kernel.org/bpf/[email protected]/

v1 -> v2:
* Add test to verify decrementing refcount when update cgroup local storage maps with BPF_F_LOCK.
* Address review from AI bot:
* Fast path without bucket lock (around line 610) in

    'bpf_local_storage.c'.

* v1: https://lore.kernel.org/bpf/[email protected]/

Leon Hwang (2):
bpf: Free special fields when update [lru_,]percpu_hash maps selftests/bpf: Add test to verify freeing the special fields when
update [lru_,]percpu_hash maps

 kernel/bpf/hashtab.c                          | 10 +++-
 .../bpf/prog_tests/refcounted_kptr.c          | 57 ++++++++++++++++++
 .../selftests/bpf/progs/refcounted_kptr.c     | 60 +++++++++++++++++++
 3 files changed, 125 insertions(+), 2 deletions(-)

--
2.51.2