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);
newpos = NULL;
dir = xfrm_policy_id2dir(policy->index);
--
2.43.0