From: Josef Bacik bpf_local_storage_map_free() pins the owner with refcount_inc_not_zero() while it uncharges an element. The pin counts when bpf_local_storage_destroy() could not get local_storage->lock: destroy() then goes on, drops the owner's reference and spins until the count is zero. So the pin is the last reference whenever it is needed, and refcount_dec() takes that for a bug: it warns "decrement hit 0; leaking memory" and saturates the count. Unless destroy() happens to read the count between the two stores it spins for ever, under rcu_read_lock(). Drop the pin with refcount_dec_and_test(). Fixes: 5d800f87d0a5 ("bpf: Support lockless unlink when freeing map or local storage") Signed-off-by: Josef Bacik Signed-off-by: Alexei Starovoitov --- include/linux/bpf_local_storage.h | 13 +++++++++++++ kernel/bpf/bpf_local_storage.c | 6 +++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/include/linux/bpf_local_storage.h b/include/linux/bpf_local_storage.h index 9e4f5c45c974..6c643ac51079 100644 --- a/include/linux/bpf_local_storage.h +++ b/include/linux/bpf_local_storage.h @@ -104,6 +104,19 @@ struct bpf_local_storage { refcount_t owner_refcnt;/* Used to pin owner when map_free is uncharging */ }; +/* + * Drop a reference got by refcount_inc_not_zero(&local_storage->owner_refcnt). + * bpf_local_storage_destroy() may have dropped the owner's reference meanwhile + * and wait for the count to reach zero, so this one can be the last. + */ +static inline void +bpf_local_storage_unpin_owner(struct bpf_local_storage *local_storage) +{ + bool last __maybe_unused; + + last = refcount_dec_and_test(&local_storage->owner_refcnt); +} + /* U16_MAX is much more than enough for sk local storage * considering a tcp_sock is ~2k. */ diff --git a/kernel/bpf/bpf_local_storage.c b/kernel/bpf/bpf_local_storage.c index 4642da062f0f..7721834e4bff 100644 --- a/kernel/bpf/bpf_local_storage.c +++ b/kernel/bpf/bpf_local_storage.c @@ -199,7 +199,7 @@ static void bpf_selem_unlink_storage_nolock_misc(struct bpf_local_storage_elem * } if (pin_owner) - refcount_dec(&local_storage->owner_refcnt); + bpf_local_storage_unpin_owner(local_storage); } /* local_storage->lock must be held and selem->local_storage == local_storage. @@ -753,8 +753,8 @@ u32 bpf_local_storage_destroy(struct bpf_local_storage *local_storage) while (refcount_read(&local_storage->owner_refcnt)) cpu_relax(); /* - * Paired with refcount_dec() in bpf_selem_unlink_nofail() - * to make sure destroy() sees the correct local_storage->mem_charge. + * Paired with bpf_local_storage_unpin_owner() to make sure + * destroy() sees the correct local_storage->mem_charge. */ smp_mb(); } -- 2.55.0