From: Mykyta Yatsenko Remove lock from the bpf_timer_cancel() helper. The lock does not protect from concurrent modification of the bpf_async_cb data fields as those are modified in the callback without locking. Use guard(rcu)() instead of pair of explicit lock()/unlock(). Signed-off-by: Mykyta Yatsenko Acked-by: Kumar Kartikeya Dwivedi --- kernel/bpf/helpers.c | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index cbacddc7101a82b2f72278034bba4188829fecd6..19ca6e772165dd5f0015ada560acd97b2ad2c24c 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -1465,7 +1465,7 @@ static void drop_prog_refcnt(struct bpf_async_cb *async) } } -BPF_CALL_1(bpf_timer_cancel, struct bpf_async_kern *, timer) +BPF_CALL_1(bpf_timer_cancel, struct bpf_async_kern *, async) { struct bpf_hrtimer *t, *cur_t; bool inc = false; @@ -1473,13 +1473,12 @@ BPF_CALL_1(bpf_timer_cancel, struct bpf_async_kern *, timer) if (in_nmi()) return -EOPNOTSUPP; - rcu_read_lock(); - __bpf_spin_lock_irqsave(&timer->lock); - t = timer->timer; - if (!t) { - ret = -EINVAL; - goto out; - } + + guard(rcu)(); + + t = READ_ONCE(async->timer); + if (!t) + return -EINVAL; cur_t = this_cpu_read(hrtimer_running); if (cur_t == t) { @@ -1487,8 +1486,7 @@ BPF_CALL_1(bpf_timer_cancel, struct bpf_async_kern *, timer) * its own timer the hrtimer_cancel() will deadlock * since it waits for callback_fn to finish. */ - ret = -EDEADLK; - goto out; + return -EDEADLK; } /* Only account in-flight cancellations when invoked from a timer @@ -1511,20 +1509,19 @@ BPF_CALL_1(bpf_timer_cancel, struct bpf_async_kern *, timer) * cancelling and waiting for it synchronously, since it might * do the same. Bail! */ - ret = -EDEADLK; - goto out; + atomic_dec(&t->cancelling); + return -EDEADLK; } + drop: - drop_prog_refcnt(&t->cb); -out: - __bpf_spin_unlock_irqrestore(&timer->lock); + __bpf_async_set_callback(async, NULL, NULL); /* Cancel the timer and wait for associated callback to finish * if it was running. */ - ret = ret ?: hrtimer_cancel(&t->timer); + ret = hrtimer_cancel(&t->timer); + if (inc) atomic_dec(&t->cancelling); - rcu_read_unlock(); return ret; } -- 2.52.0