On Mon, Aug 29, 2022 at 10:27:50PM +0800, Hou Tao wrote:
> From: Hou Tao <houtao1@xxxxxxxxxx>
>
> Now migrate_disable() does not disable preemption and under some
> architecture (e.g. arm64) __this_cpu_{inc|dec|inc_return} are neither
> preemption-safe nor IRQ-safe, so the concurrent lookups or updates on
> the same task local storage and the same CPU may make
> bpf_task_storage_busy be imbalanced, and bpf_task_storage_trylock()
> will always fail.
>
> Fixing it by using this_cpu_{inc|dec|inc_return} when manipulating
> bpf_task_storage_busy.
>
> Fixes: bc235cdb423a ("bpf: Prevent deadlock from recursive bpf_task_storage_[get|delete]")
> Signed-off-by: Hou Tao <houtao1@xxxxxxxxxx>
> ---
> kernel/bpf/bpf_local_storage.c | 4 ++--
> kernel/bpf/bpf_task_storage.c | 8 ++++----
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/bpf/bpf_local_storage.c b/kernel/bpf/bpf_local_storage.c
> index 4ee2e7286c23..802fc15b0d73 100644
> --- a/kernel/bpf/bpf_local_storage.c
> +++ b/kernel/bpf/bpf_local_storage.c
> @@ -555,11 +555,11 @@ void bpf_local_storage_map_free(struct bpf_local_storage_map *smap,
> struct bpf_local_storage_elem, map_node))) {
> if (busy_counter) {
> migrate_disable();
> - __this_cpu_inc(*busy_counter);
> + this_cpu_inc(*busy_counter);
> }
> bpf_selem_unlink(selem, false);
> if (busy_counter) {
> - __this_cpu_dec(*busy_counter);
> + this_cpu_dec(*busy_counter);
> migrate_enable();
> }
> cond_resched_rcu();
> diff --git a/kernel/bpf/bpf_task_storage.c b/kernel/bpf/bpf_task_storage.c
> index e9014dc62682..6f290623347e 100644
> --- a/kernel/bpf/bpf_task_storage.c
> +++ b/kernel/bpf/bpf_task_storage.c
> @@ -26,20 +26,20 @@ static DEFINE_PER_CPU(int, bpf_task_storage_busy);
> static void bpf_task_storage_lock(void)
> {
> migrate_disable();
> - __this_cpu_inc(bpf_task_storage_busy);
> + this_cpu_inc(bpf_task_storage_busy);
> }
>
> static void bpf_task_storage_unlock(void)
> {
> - __this_cpu_dec(bpf_task_storage_busy);
> + this_cpu_dec(bpf_task_storage_busy);
> migrate_enable();
> }
>
> static bool bpf_task_storage_trylock(void)
> {
> migrate_disable();
> - if (unlikely(__this_cpu_inc_return(bpf_task_storage_busy) != 1)) {
> - __this_cpu_dec(bpf_task_storage_busy);
> + if (unlikely(this_cpu_inc_return(bpf_task_storage_busy) != 1)) {
> + this_cpu_dec(bpf_task_storage_busy);
This change is only needed here but not in the htab fix [0]
or you are planning to address it separately ?
[0]: https://lore.kernel.org/bpf/20220829023709.1958204-2-houtao@xxxxxxxxxxxxxxx/
