Re: [PATCH ipsec] xfrm: policy: use hlist_del_init_rcu in xfrm_hash_rebuild to avoid bydst poison
From: Florian Westphal <fw@strlen.de>
Date: 2026-07-02 19:19:09
Xiang Mei (Microsoft) [off-list ref] wrote:
quoted hunk ↗ jump to hunk
xfrm_hash_rebuild() unlinks each policy from its bydst chain with hlist_del_rcu() and re-inserts it. For an inexact policy the re-insert goes through xfrm_policy_inexact_insert(), which can fail on a GFP_ATOMIC allocation; on failure the error path only WARN_ONCE()s and continues, so the policy is left with a poisoned bydst node (LIST_POISON2). The next rebuild calls hlist_del_rcu() on that node again, dereferences the poison, and takes a general protection fault. Use hlist_del_init_rcu() instead, so a failed-reinsert node is left unhashed (pprev == NULL) rather than poisoned. The next rebuild's hlist_del_init_rcu() is then a no-op for it, and the non-failing case is unchanged. The reinsert allocation is GFP_ATOMIC (it runs under xfrm_policy_lock), so in practice this is only reached under memory pressure; the crash below was reproduced deterministically by forcing that allocation to fail with fault injection (failslab). Crash: Oops: general protection fault, probably for non-canonical address 0xfbd59c0000000024: 0000 [#1] SMP KASAN NOPTI KASAN: maybe wild-memory-access in range [0xdead000000000120-0xdead000000000127] ... Workqueue: events xfrm_hash_rebuild RIP: 0010:xfrm_hash_rebuild+0x5b3/0x1190 RAX: dead000000000122 (LIST_POISON2 + offset) ... Call Trace: hlist_del_rcu (include/linux/rculist.h:599) xfrm_hash_rebuild (net/xfrm/xfrm_policy.c:1365) process_one_work (kernel/workqueue.c:3322) worker_thread (kernel/workqueue.c:3486) kthread (kernel/kthread.c:436) ret_from_fork (arch/x86/kernel/process.c:158) ret_from_fork_asm (arch/x86/entry/entry_64.S:245) ... Kernel panic - not syncing: Fatal exception in interrupt Fixes: 563d5ca93e88 ("xfrm: switch migrate to xfrm_policy_lookup_bytype") Reported-by: AutonomousCodeSecurity@microsoft.com Signed-off-by: Xiang Mei (Microsoft) <redacted> --- net/xfrm/xfrm_policy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c index 7ef861a0e823..2612a405542b 100644 --- a/net/xfrm/xfrm_policy.c +++ b/net/xfrm/xfrm_policy.c@@ -1362,7 +1362,7 @@ static void xfrm_hash_rebuild(struct work_struct *work) if (xfrm_policy_is_dead_or_sk(policy)) continue; - hlist_del_rcu(&policy->bydst); + hlist_del_init_rcu(&policy->bydst);
This patch is dubious. I looks to me as if it papers over the actual bug. Why is there a memory allocation error? The first loop -- before unlink -- is supposed to preallocate the new bins and chain heads. This is also why there is a WARN. No memory allocations are supposed to occur after the hlist_del_rcu(), there is supposed to be a guarantee that the insertion succeeds.