Re: [PATCH] xfrm: iptfs: fix ABBA deadlock in iptfs_destroy_state()
From: Steffen Klassert <steffen.klassert@secunet.com>
Date: 2026-06-02 10:06:11
Also in:
lkml, stable
On Thu, May 28, 2026 at 04:03:18PM +0000, Tristan Madani wrote:
quoted hunk ↗ jump to hunk
iptfs_destroy_state() calls hrtimer_cancel() while holding a spinlock that the timer callback also acquires, leading to an ABBA deadlock on SMP systems. For the output timer (iptfs_timer): - iptfs_destroy_state() holds x->lock, calls hrtimer_cancel() - iptfs_delay_timer() callback takes x->lock For the drop timer (drop_timer): - iptfs_destroy_state() holds drop_lock, calls hrtimer_cancel() - iptfs_drop_timer() callback takes drop_lock Both timers use HRTIMER_MODE_REL_SOFT, so their callbacks run in softirq context. When hrtimer_cancel() is called for a soft timer that is currently executing on another CPU, hrtimer_cancel_wait_running() spins on softirq_expiry_lock -- the same lock held by the softirq running the callback. If the callback is blocked waiting for the spinlock held by the caller of hrtimer_cancel(), a circular dependency forms: CPU 0: holds lock_A -> waits for softirq_expiry_lock CPU 1: holds softirq_expiry_lock -> waits for lock_A Fix this by cancelling both timers before acquiring their respective locks. hrtimer_cancel() is safe to call without holding any lock and will wait for any in-progress callback to complete. The locks are still acquired afterwards to synchronize with any in-flight packet processing before tearing down the state. Found by source code audit. Fixes: 4b3faf610cc6 ("xfrm: iptfs: add new iptfs xfrm mode impl") Cc: Christian Hopps <redacted> Cc: Steffen Klassert <steffen.klassert@secunet.com> Cc: stable@vger.kernel.org Signed-off-by: Tristan Madani <redacted> --- net/xfrm/xfrm_iptfs.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)diff --git a/net/xfrm/xfrm_iptfs.c b/net/xfrm/xfrm_iptfs.c index 97bc979e55baf..fd25b2b230793 100644 --- a/net/xfrm/xfrm_iptfs.c +++ b/net/xfrm/xfrm_iptfs.c@@ -2708,8 +2708,9 @@ static void iptfs_destroy_state(struct xfrm_state *x) if (!xtfs) return; - spin_lock_bh(&xtfs->x->lock); hrtimer_cancel(&xtfs->iptfs_timer); + + spin_lock_bh(&xtfs->x->lock); __skb_queue_head_init(&list); skb_queue_splice_init(&xtfs->queue, &list); spin_unlock_bh(&xtfs->x->lock);@@ -2717,8 +2718,9 @@ static void iptfs_destroy_state(struct xfrm_state *x) while ((skb = __skb_dequeue(&list))) kfree_skb(skb); - spin_lock_bh(&xtfs->drop_lock); hrtimer_cancel(&xtfs->drop_timer); + + spin_lock_bh(&xtfs->drop_lock); spin_unlock_bh(&xtfs->drop_lock);
What is this? You take the drop_lock just to release it in the next line.