From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.remote-ip=100.103.45.18 ARC-Seal: i=1; d=subspace.kernel.org; s=arc20261015; a=rsa-sha256; cv=none; t=1793212518; b=Lq74gTj9MJKgWk7B/FVcXeEIyZI20ezA6joJK1Tckvm7hwaE7ZjeF8o0JnXS01nyhRjP zX/ypJAA3w3SVV9m5G9mOjHs4AZGcd3S0YOa1qOb4zBdHMttck9g+wQEeaI9/T886Ux4p uMw9XE/o/O1ElvLzYaodCXMQCLsjg+F7CjFQQfj2v0mKCw3e/gTBbNOkXXrgVb3rQThC3 LjQykFw+34WOIp/GDuJSzMRaBvC8VdMn/lFLokbF2s39R5GossLmF+bkxbCQCPpSnQZ+B GO0iJXbrO+knnJuEShify6BVfh6MUFvxjxdqED1bTgPoGlpqoOlseH6IJuH+6FvPu+w== ARC-Message-Signature: i=1; d=subspace.kernel.org; s=arc20261015; a=rsa-sha256; c=relaxed/relaxed; t=1793212518; h=Received:DKIM-Signature:From:To:Cc:Subject:Date:Message-ID:X-Mailer: In-Reply-To:References:User-Agent:X-stable:X-Patchwork-Hint: MIME-Version:Content-Transfer-Encoding; bh=XPWBkhY2MBEaczG/WCn/AML9708/JZqBEIgZMycA3hE=; b=dmql8imNo71Ux0uJ4CSZtsNCQnKv9mqcfYa++tK7X24DSMk2HaPnv6Osv3KUW/vZNGlv KodGt3xlBDDUyqQOE2s00rgim6pt7IH6xlhEmuu4VPgOwWrWvQYQeCdUQkPscMvKly/Er BEK2Jze3uRtHQM5ahMuGG/9E5Em4zLpISZzNPL1YdMpkbLMk40gkLY6HsyRWEraSWZr08 h3uZGmudalilirCYydMO+WGW+aPXMj5uzrxrcBHbwytV+2r5BDyK/HrJ45UyMlOyCp1dW zJQ0SKAN64X7MMJbubxDkvN2g0urHrvgkrke85KAO9jWdG24rv8V0ZX1V6thrKnyzHg== ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass header.d=linuxfoundation.org header.i=@linuxfoundation.org header.a=rsa-sha256 header.s=korg header.b=FHVxfCid; arc=none smtp.remote-ip=100.103.45.18 Received: from smtp.kernel.org (quasi.space.kernel.org [100.103.45.18]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7C45B606A31A; Wed, 28 Oct 2026 18:35:18 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key, unprotected) header.d=linuxfoundation.org header.i=@linuxfoundation.org header.a=rsa-sha256 header.s=korg header.b=FHVxfCid Received: by smtp.kernel.org (Postfix) with ESMTPSA id C93FC1F01825; Wed, 28 Oct 2026 18:35:17 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linuxfoundation.org; s=korg; t=1793212518; bh=XPWBkhY2MBEaczG/WCn/AML9708/JZqBEIgZMycA3hE=; h=From:To:Cc:Subject:Date:In-Reply-To:References; b=FHVxfCidvehKEW3t21Q/9o+DkQe3YChWO8Co3u8Qn8FnbQHKf0QuxtRjkBHiqVAQF xrB2FkqAoCy5jq/Js5KG4MVBg8lL+kD0JQzJw89TD+/jlOlAburKxiMYeTI5JaBp4Z /hoUl4qKvvATb33eRgj+yp2FKkhF3cqqC/HWYypY= From: Greg Kroah-Hartman To: stable@vger.kernel.org Cc: Greg Kroah-Hartman , patches@lists.linux.dev, Kumar Kartikeya Dwivedi , Alexei Starovoitov , Artem Dinaburg , Sasha Levin Subject: [PATCH 6.1 0182/1887] bpf: Defer work in bpf_timer_cancel_and_free Date: Wed, 28 Oct 2026 17:37:58 +0100 Message-ID: <20261028170503.377357879@linuxfoundation.org> X-Mailer: git-send-email 2.56.0 In-Reply-To: <20261028170459.200466217@linuxfoundation.org> References: <20261028170459.200466217@linuxfoundation.org> User-Agent: quilt/0.69 X-stable: review X-Patchwork-Hint: ignore X-Mailing-List: patches@lists.linux.dev List-Id: List-Post: List-Help: List-Subscribe: List-Unsubscribe: List-Owner: Precedence: list MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 6.1-stable review patch. If anyone has any objections, please let me know. ------------------ From: Kumar Kartikeya Dwivedi [ Upstream commit a6fcd19d7eac1335eb76bc16b6a66b7f574d1d69 ] Currently, the same case as previous patch (two timer callbacks trying to cancel each other) can be invoked through bpf_map_update_elem as well, or more precisely, freeing map elements containing timers. Since this relies on hrtimer_cancel as well, it is prone to the same deadlock situation as the previous patch. It would be sufficient to use hrtimer_try_to_cancel to fix this problem, as the timer cannot be enqueued after async_cancel_and_free. Once async_cancel_and_free has been done, the timer must be reinitialized before it can be armed again. The callback running in parallel trying to arm the timer will fail, and freeing bpf_hrtimer without waiting is sufficient (given kfree_rcu), and bpf_timer_cb will return HRTIMER_NORESTART, preventing the timer from being rearmed again. However, there exists a UAF scenario where the callback arms the timer before entering this function, such that if cancellation fails (due to timer callback invoking this routine, or the target timer callback running concurrently). In such a case, if the timer expiration is significantly far in the future, the RCU grace period expiration happening before it will free the bpf_hrtimer state and along with it the struct hrtimer, that is enqueued. Hence, it is clear cancellation needs to occur after async_cancel_and_free, and yet it cannot be done inline due to deadlock issues. We thus modify bpf_timer_cancel_and_free to defer work to the global workqueue, adding a work_struct alongside rcu_head (both used at _different_ points of time, so can share space). Update existing code comments to reflect the new state of affairs. [ Backport to 6.1.y: mapped deferred cancellation onto the older pre-BPF- workqueue timer object layout. ] Fixes: b00628b1c7d5 ("bpf: Introduce bpf timers.") Signed-off-by: Kumar Kartikeya Dwivedi Link: https://lore.kernel.org/r/20240709185440.1104957-3-memxor@gmail.com Signed-off-by: Alexei Starovoitov Assisted-by: LLM Signed-off-by: Artem Dinaburg Signed-off-by: Sasha Levin --- kernel/bpf/helpers.c | 58 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 46 insertions(+), 12 deletions(-) diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index d5dcc172647c8..a5ab200c48663 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -1115,7 +1115,10 @@ struct bpf_hrtimer { struct bpf_prog *prog; void __rcu *callback_fn; void *value; - struct rcu_head rcu; + union { + struct rcu_head rcu; + struct work_struct delete_work; + }; }; /* the actual struct hidden inside uapi struct bpf_timer */ @@ -1169,6 +1172,22 @@ static enum hrtimer_restart bpf_timer_cb(struct hrtimer *hrtimer) return HRTIMER_NORESTART; } +static void bpf_timer_delete_work(struct work_struct *work) +{ + struct bpf_hrtimer *t = container_of(work, struct bpf_hrtimer, + delete_work); + + /* Cancel the timer and wait for callback to complete if it was running. + * If hrtimer_cancel() can be safely called it's safe to call + * kfree_rcu(t) right after for both preallocated and non-preallocated + * maps. The timer->timer = NULL was already done and no code path can see + * address 't' anymore. A timer armed before bpf_timer_cancel_and_free() + * will have been cancelled. + */ + hrtimer_cancel(&t->timer); + kfree_rcu(t, rcu); +} + BPF_CALL_3(bpf_timer_init, struct bpf_timer_kern *, timer, struct bpf_map *, map, u64, flags) { @@ -1206,6 +1225,7 @@ BPF_CALL_3(bpf_timer_init, struct bpf_timer_kern *, timer, struct bpf_map *, map t->prog = NULL; rcu_assign_pointer(t->callback_fn, NULL); atomic_set(&t->cancelling, 0); + INIT_WORK(&t->delete_work, bpf_timer_delete_work); hrtimer_init(&t->timer, clockid, HRTIMER_MODE_REL_SOFT); t->timer.function = bpf_timer_cb; WRITE_ONCE(timer->timer, t); @@ -1426,14 +1446,8 @@ void bpf_timer_cancel_and_free(void *val) __bpf_spin_unlock_irqrestore(&timer->lock); if (!t) return; - /* Cancel the timer and wait for callback to complete if it was running. - * If hrtimer_cancel() can be safely called it's safe to call kfree(t) - * right after for both preallocated and non-preallocated maps. - * The timer->timer = NULL was already done and no code path can - * see address 't' anymore. - * - * Check that bpf_map_delete/update_elem() wasn't called from timer - * callback_fn. In such case don't call hrtimer_cancel() (since it will + /* We check that bpf_map_delete/update_elem() was called from timer + * callback_fn. In such case we don't call hrtimer_cancel() (since it will * deadlock) and don't call hrtimer_try_to_cancel() (since it will just * return -1). Though callback_fn is still running on this cpu it's * safe to do kfree(t) because bpf_timer_cb() read everything it needed @@ -1441,10 +1455,30 @@ void bpf_timer_cancel_and_free(void *val) * since timer->timer = NULL was already done. The timer will be * effectively cancelled because bpf_timer_cb() will return * HRTIMER_NORESTART. + * + * However, it is possible the timer callback_fn calling us armed the + * timer _before_ calling us, such that failing to cancel it here will + * cause it to possibly use struct hrtimer after freeing bpf_hrtimer. + * Therefore, we _need_ to cancel any outstanding timers before we do + * kfree_rcu, even though no more timers can be armed. + * + * Moreover, we need to schedule work even if timer does not belong to + * the calling callback_fn, as on two different CPUs, we can end up in a + * situation where both sides run in parallel, try to cancel one + * another, and we end up waiting on both sides in hrtimer_cancel + * without making forward progress, since timer1 depends on timer2 + * callback to finish, and vice versa. + * + * CPU 1 (timer1_cb) CPU 2 (timer2_cb) + * bpf_timer_cancel_and_free(timer2) bpf_timer_cancel_and_free(timer1) + * + * To avoid these issues, punt to workqueue context when we are in a + * timer callback. */ - if (this_cpu_read(hrtimer_running) != t) - hrtimer_cancel(&t->timer); - kfree_rcu(t, rcu); + if (this_cpu_read(hrtimer_running)) + queue_work(system_unbound_wq, &t->delete_work); + else + bpf_timer_delete_work(&t->delete_work); } BPF_CALL_2(bpf_kptr_xchg, void *, map_value, void *, ptr) -- 2.53.0