[PATCH bpf-next v4 2/4] bpf: Dissociate struct_ops progs before the grace period of the map

[Qiliang Yuan]

A struct_ops prog is associated with its map without a reference on the
map, and finds the struct_ops through bpf_prog_get_assoc_struct_ops()
under RCU. For a struct_ops map, bpf_map_put() waits for an RCU grace
period before it calls bpf_struct_ops_map_free(), which clears the
association and then frees the trampoline and the kdata right away.

Nothing waits for a prog that reads the association after that grace
period started and before the association is cleared. A timer callback
of a struct_ops prog that calls test_1 of bpf_testmod's multi_st_ops
through the association while the map is freed returns into the freed
trampoline, whose image is filled with int3. RAX holds MAP_MAGIC (1234),
which test_1 has just returned:

  Oops: int3: 0000 [#1] SMP KASAN NOPTI
  CPU: 2 UID: 0 PID: 31 Comm: ksoftirqd/2 ...
  RIP: 0010:0xffffffffc043114d
  Code: cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc cc <cc> cc cc cc
  RAX: 00000000000004d2 ...
  Call Trace:
   bpf_prog_ec591f4f9d13f364_timer_cb+0x96/0x1e2
   bpf_timer_cb+0x15d/0x240
   __hrtimer_run_queues+0x264/0x5a0
   hrtimer_run_softirq+0x1a7/0x3b0
   handle_softirqs+0x18c/0x5b0

Dissociate the progs in map_free_pre_rcu(), so that the grace period of
bpf_map_put() follows it, and make that an RCU tasks trace grace period
for every struct_ops map. It then also waits for the progs that read
the association under RCU tasks trace, e.g. from a bpf_wq callback or a
sleepable prog, as a tasks trace grace period implies an RCU one.

The st_ops_assoc_in_timer_free subtest of test_progs reproduces it. Its
timer callback keeps calling test_1 through the association while the
map is freed, and test_1 spins in bpf_loop() when called from there:

  $ cd tools/testing/selftests/bpf
  $ make test_progs
  $ ./test_progs -t struct_ops_assoc/st_ops_assoc_in_timer_free

On kernels of the same bpf-next commit with KASAN enabled, x86_64 VM
with 32 vCPUs, before and after this patch:

            before                     after
  outcome   Oops (int3) on run 1       100 of 100 runs passed, no Oops

Fixes: 5db69b0fbdc8 ("bpf: Make struct_ops tasks_rcu grace period optional")
Signed-off-by: Qiliang Yuan <odys.y...@gmail.com>
---
 kernel/bpf/bpf_struct_ops.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
index a1ae30b145a03..7d1a8514ba341 100644
--- a/kernel/bpf/bpf_struct_ops.c
+++ b/kernel/bpf/bpf_struct_ops.c
@@ -1030,6 +1030,7 @@ static void bpf_struct_ops_map_free_pre_rcu(struct 
bpf_map *map)
        struct bpf_struct_ops_map *st_map = (struct bpf_struct_ops_map *)map;
 
        bpf_struct_ops_map_del_ksyms(st_map);
+       bpf_struct_ops_map_dissoc_progs(st_map);
 }
 
 static void bpf_struct_ops_map_free(struct bpf_map *map)
@@ -1045,8 +1046,6 @@ static void bpf_struct_ops_map_free(struct bpf_map *map)
        if (btf_is_module(st_map->btf))
                module_put(st_map->st_ops_desc->st_ops->owner);
 
-       bpf_struct_ops_map_dissoc_progs(st_map);
-
        if (tasks_rcu && IS_ENABLED(CONFIG_TASKS_RCU))
                synchronize_rcu_tasks();
 
@@ -1162,7 +1161,11 @@ static struct bpf_map *bpf_struct_ops_map_alloc(union 
bpf_attr *attr)
 
        mutex_init(&st_map->lock);
        bpf_map_init_from_attr(map, attr);
-       map->free_after_mult_rcu_gp = 
st_ops_desc->st_ops->free_after_mult_rcu_gp;
+       /*
+        * A prog dissociated in map_free_pre_rcu() may still run the struct_ops
+        * from a bpf_wq callback or a sleepable prog, under RCU tasks trace.
+        */
+       map->free_after_mult_rcu_gp = true;
        map->free_after_rcu_gp = true;
 
        return map;

-- 
2.43.0