Re: [PATCH ipsec v2] xfrm: policy: preallocate inexact bins before xfrm_hash_rebuild reinsert

[Manuel Ebner <manuelebner@mailbox.org>]

Hi Xiang Mei

On Fri, 2026-07-03 at 05:19 +0000, Xiang Mei (Microsoft) wrote:
> xfrm_hash_rebuild()'s first loop preallocates the bins/chains the reinsert
> loop needs, so the reinsert (after hlist_del_rcu()) cannot allocate or
> fail. 

This sentence is difficult to understand for me. Can it be simplified or changed? 
This would help a little:

xfrm_hash_rebuild()'s first loop preallocates the bins/chains which the reinsert
loop needs. In this case the reinsert ...


> But its guard is inverted: it skips policies with prefixlen <
> threshold and preallocates for the rest.
> 
> prefixlen < threshold is exactly when policy_hash_bysel() returns NULL and
> the reinsert takes the allocating xfrm_policy_inexact_insert() path. So the
> loop preallocates for the exact policies (which never allocate) and skips
> the inexact ones, whose bin/node is then allocated GFP_ATOMIC during
> reinsert. On failure the error path only WARN_ONCE()s and continues,
> leaving a poisoned bydst node; 

what's 'bydst'?

> the next rebuild's hlist_del_rcu()
> dereferences LIST_POISON2 and takes a GPF. 

/GPF/GFP/

Thanks
 Manuel

> Reachable under memory pressure,
> deterministic via failslab.
> 
> Invert the guard so preallocation covers exactly the reinserted policies;
> the reinsert then allocates nothing and cannot fail.
> 
> Crash:
>   Oops: general protection fault, probably for non-canonical address
>   0xfbd59c0000000024: 0000 [#1] SMP KASAN NOPTI
>   KASAN: maybe wild-memory-access in range [0xdead...]
>   ...
>   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: 24969facd704 ("xfrm: policy: store inexact policies in an rhashtable")
> Reported-by: AutonomousCodeSecurity@microsoft.com
> Signed-off-by: Xiang Mei (Microsoft) <xmei5@asu.edu>
> ---
> v2: fix the inverted preallocation guard (root cause)
>     instead of avoiding crash
> 
>  net/xfrm/xfrm_policy.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
> index 7ef861a0e823..932a313b9460 100644
> --- a/net/xfrm/xfrm_policy.c
> +++ b/net/xfrm/xfrm_policy.c
> @@ -1329,8 +1329,8 @@ static void xfrm_hash_rebuild(struct work_struct *work)
>  			}
>  		}
>  
> -		if (policy->selector.prefixlen_d < dbits ||
> -		    policy->selector.prefixlen_s < sbits)
> +		if (policy->selector.prefixlen_d >= dbits &&
> +		    policy->selector.prefixlen_s >= sbits)
>  			continue;
>  
>  		bin = xfrm_policy_inexact_alloc_bin(policy, dir);