[bpf,v4,3/7] bpf: Set need_defer as false when clearing fd array during map free

Commit Message

Hou Tao Nov. 30, 2023, 2:01 p.m. UTC

From: Hou Tao <[e-mail]>

Both map deletion operation, map release and map free operation use
fd_array_map_delete_elem() to remove the element from fd array and
need_defer is always true in fd_array_map_delete_elem(). For the map
deletion operation and map release operation, need_defer=true is
necessary, because the bpf program, which accesses the element in fd
array, may still alive. However for map free operation, it is certain
that the bpf program which owns the fd array has already been exited, so
setting need_defer as false is appropriate for map free operation.

So fix it by adding need_defer parameter to bpf_fd_array_map_clear() and
adding a new helper fd_array_map_delete_elem_with_deferred_free() to
handle the map deletion, map release and map free operations
correspondingly.

Signed-off-by: Hou Tao <[e-mail]>
---
 kernel/bpf/arraymap.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

Comments

Alexei Starovoitov Dec. 3, 2023, 6:57 p.m. UTC | #1

On Thu, Nov 30, 2023 at 10:01:16PM +0800, Hou Tao wrote:
>  
> -static long fd_array_map_delete_elem(struct bpf_map *map, void *key)
> +static long fd_array_map_delete_elem_with_deferred_free(struct bpf_map *map, void *key,
> +							bool need_defer)

way too verbose.
Tomorrow we will add another bool and would have to rename it?
Just use __fd_array_map_delete_elem().

Hou Tao Dec. 4, 2023, 8:57 a.m. UTC | #2

Hi,

On 12/4/2023 2:57 AM, Alexei Starovoitov wrote:
> On Thu, Nov 30, 2023 at 10:01:16PM +0800, Hou Tao wrote:
>>  
>> -static long fd_array_map_delete_elem(struct bpf_map *map, void *key)
>> +static long fd_array_map_delete_elem_with_deferred_free(struct bpf_map *map, void *key,
>> +							bool need_defer)
> way too verbose.
> Tomorrow we will add another bool and would have to rename it?
> Just use __fd_array_map_delete_elem().

Will do. Thanks.

Patch

diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index f9aed5909d6e..fb0462b84ab6 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -871,7 +871,8 @@  int bpf_fd_array_map_update_elem(struct bpf_map *map, struct file *map_file,
 	return 0;
 }
 
-static long fd_array_map_delete_elem(struct bpf_map *map, void *key)
+static long fd_array_map_delete_elem_with_deferred_free(struct bpf_map *map, void *key,
+							bool need_defer)
 {
 	struct bpf_array *array = container_of(map, struct bpf_array, map);
 	void *old_ptr;
@@ -890,13 +891,18 @@  static long fd_array_map_delete_elem(struct bpf_map *map, void *key)
 	}
 
 	if (old_ptr) {
-		map->ops->map_fd_put_ptr(map, old_ptr, true);
+		map->ops->map_fd_put_ptr(map, old_ptr, need_defer);
 		return 0;
 	} else {
 		return -ENOENT;
 	}
 }
 
+static long fd_array_map_delete_elem(struct bpf_map *map, void *key)
+{
+	return fd_array_map_delete_elem_with_deferred_free(map, key, true);
+}
+
 static void *prog_fd_array_get_ptr(struct bpf_map *map,
 				   struct file *map_file, int fd)
 {
@@ -925,13 +931,13 @@  static u32 prog_fd_array_sys_lookup_elem(void *ptr)
 }
 
 /* decrement refcnt of all bpf_progs that are stored in this map */
-static void bpf_fd_array_map_clear(struct bpf_map *map)
+static void bpf_fd_array_map_clear(struct bpf_map *map, bool need_defer)
 {
 	struct bpf_array *array = container_of(map, struct bpf_array, map);
 	int i;
 
 	for (i = 0; i < array->map.max_entries; i++)
-		fd_array_map_delete_elem(map, &i);
+		fd_array_map_delete_elem_with_deferred_free(map, &i, need_defer);
 }
 
 static void prog_array_map_seq_show_elem(struct bpf_map *map, void *key,
@@ -1110,7 +1116,7 @@  static void prog_array_map_clear_deferred(struct work_struct *work)
 {
 	struct bpf_map *map = container_of(work, struct bpf_array_aux,
 					   work)->map;
-	bpf_fd_array_map_clear(map);
+	bpf_fd_array_map_clear(map, true);
 	bpf_map_put(map);
 }
 
@@ -1260,7 +1266,7 @@  static void perf_event_fd_array_release(struct bpf_map *map,
 	for (i = 0; i < array->map.max_entries; i++) {
 		ee = READ_ONCE(array->ptrs[i]);
 		if (ee && ee->map_file == map_file)
-			fd_array_map_delete_elem(map, &i);
+			fd_array_map_delete_elem_with_deferred_free(map, &i, true);
 	}
 	rcu_read_unlock();
 }
@@ -1268,7 +1274,7 @@  static void perf_event_fd_array_release(struct bpf_map *map,
 static void perf_event_fd_array_map_free(struct bpf_map *map)
 {
 	if (map->map_flags & BPF_F_PRESERVE_ELEMS)
-		bpf_fd_array_map_clear(map);
+		bpf_fd_array_map_clear(map, false);
 	fd_array_map_free(map);
 }
 
@@ -1304,7 +1310,7 @@  static void cgroup_fd_array_put_ptr(struct bpf_map *map, void *ptr, bool need_de
 
 static void cgroup_fd_array_free(struct bpf_map *map)
 {
-	bpf_fd_array_map_clear(map);
+	bpf_fd_array_map_clear(map, false);
 	fd_array_map_free(map);
 }
 
@@ -1349,7 +1355,7 @@  static void array_of_map_free(struct bpf_map *map)
 	 * is protected by fdget/fdput.
 	 */
 	bpf_map_meta_free(map->inner_map_meta);
-	bpf_fd_array_map_clear(map);
+	bpf_fd_array_map_clear(map, false);
 	fd_array_map_free(map);
 }