bpf: Delay freeing fields in local storage

Currently, when use_kmalloc_nolock is false, the freeing of fields for a
local storage selem is done eagerly before waiting for the RCU or RCU
tasks trace grace period to elapse. This opens up a window where the
program which has access to the selem can recreate the fields after the
freeing of fields is done eagerly, causing memory leaks when the element
is finally freed and returned to the kernel.

Make a few changes to address this. First, delay the freeing of fields
until after the grace periods have expired using a __bpf_selem_free_rcu
wrapper which is eventually invoked after transitioning through the
necessary number of grace period waits. Replace usage of the kfree_rcu
with call_rcu to be able to take a custom callback. Finally, care needs
to be taken to extend the rcu barriers for all cases, and not just when
use_kmalloc_nolock is true, as RCU and RCU tasks trace callbacks can be
in flight for either case and access the smap field, which is used to
obtain the BTF record to walk over special fields in the map value.

While we're at it, drop migrate_disable() from bpf_selem_free_rcu, since
migration should be disabled for RCU callbacks already.

Fixes: 9bac675e6368 ("bpf: Postpone bpf_obj_free_fields to the rcu callback")
Reviewed-by: Amery Hung <ameryhung@gmail.com>
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Link: https://lore.kernel.org/r/20260227224806.646888-4-memxor@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>

diff --git a/kernel/bpf/bpf_local_storage.c b/kernel/bpf/bpf_local_storage.c
index 2bf5ca5ae0dfd5ccdc3104ccd56b6810c65a16aa..d7db1ada2dadc1bbaec8c7d3dc07d2744b2dbdb6 100644 (file)
--- a/kernel/bpf/bpf_local_storage.c
+++ b/kernel/bpf/bpf_local_storage.c
@@ -164,16 +164,28 @@ static void bpf_local_storage_free(struct bpf_local_storage *local_storage,
                             bpf_local_storage_free_trace_rcu);
 }
 
-/* rcu tasks trace callback for use_kmalloc_nolock == false */
-static void __bpf_selem_free_trace_rcu(struct rcu_head *rcu)
+/* rcu callback for use_kmalloc_nolock == false */
+static void __bpf_selem_free_rcu(struct rcu_head *rcu)
 {
        struct bpf_local_storage_elem *selem;
+       struct bpf_local_storage_map *smap;
 
        selem = container_of(rcu, struct bpf_local_storage_elem, rcu);
+       /* bpf_selem_unlink_nofail may have already cleared smap and freed fields. */
+       smap = rcu_dereference_check(SDATA(selem)->smap, 1);
+
+       if (smap)
+               bpf_obj_free_fields(smap->map.record, SDATA(selem)->data);
+       kfree(selem);
+}
+
+/* rcu tasks trace callback for use_kmalloc_nolock == false */
+static void __bpf_selem_free_trace_rcu(struct rcu_head *rcu)
+{
        if (rcu_trace_implies_rcu_gp())
-               kfree(selem);
+               __bpf_selem_free_rcu(rcu);
        else
-               kfree_rcu(selem, rcu);
+               call_rcu(rcu, __bpf_selem_free_rcu);
 }
 
 /* Handle use_kmalloc_nolock == false */
@@ -181,7 +193,7 @@ static void __bpf_selem_free(struct bpf_local_storage_elem *selem,
                             bool vanilla_rcu)
 {
        if (vanilla_rcu)
-               kfree_rcu(selem, rcu);
+               call_rcu(&selem->rcu, __bpf_selem_free_rcu);
        else
                call_rcu_tasks_trace(&selem->rcu, __bpf_selem_free_trace_rcu);
 }
@@ -195,11 +207,8 @@ static void bpf_selem_free_rcu(struct rcu_head *rcu)
        /* The bpf_local_storage_map_free will wait for rcu_barrier */
        smap = rcu_dereference_check(SDATA(selem)->smap, 1);
 
-       if (smap) {
-               migrate_disable();
+       if (smap)
                bpf_obj_free_fields(smap->map.record, SDATA(selem)->data);
-               migrate_enable();
-       }
        kfree_nolock(selem);
 }
 
@@ -214,18 +223,12 @@ static void bpf_selem_free_trace_rcu(struct rcu_head *rcu)
 void bpf_selem_free(struct bpf_local_storage_elem *selem,
                    bool reuse_now)
 {
-       struct bpf_local_storage_map *smap;
-
-       smap = rcu_dereference_check(SDATA(selem)->smap, bpf_rcu_lock_held());
-
        if (!selem->use_kmalloc_nolock) {
                /*
                 * No uptr will be unpin even when reuse_now == false since uptr
                 * is only supported in task local storage, where
                 * smap->use_kmalloc_nolock == true.
                 */
-               if (smap)
-                       bpf_obj_free_fields(smap->map.record, SDATA(selem)->data);
                __bpf_selem_free(selem, reuse_now);
                return;
        }
@@ -958,10 +961,9 @@ restart:
         */
        synchronize_rcu();
 
-       if (smap->use_kmalloc_nolock) {
-               rcu_barrier_tasks_trace();
-               rcu_barrier();
-       }
+       /* smap remains in use regardless of kmalloc_nolock, so wait unconditionally. */
+       rcu_barrier_tasks_trace();
+       rcu_barrier();
        kvfree(smap->buckets);
        bpf_map_area_free(smap);
 }