Netdev List
 help / color / mirror / Atom feed
* [PATCH ipsec v2] xfrm: policy: preallocate inexact bins before xfrm_hash_rebuild reinsert
@ 2026-07-03  5:19 Xiang Mei (Microsoft)
  2026-07-03  5:47 ` Florian Westphal
  2026-07-03  6:18 ` Manuel Ebner
  0 siblings, 2 replies; 4+ messages in thread
From: Xiang Mei (Microsoft) @ 2026-07-03  5:19 UTC (permalink / raw)
  To: fw, steffen.klassert, herbert, davem, netdev
  Cc: horms, edumazet, kuba, pabeni, AutonomousCodeSecurity, tgopinath,
	kys, Xiang Mei (Microsoft)

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. 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; the next rebuild's hlist_del_rcu()
dereferences LIST_POISON2 and takes a GPF. 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);
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH ipsec v2] xfrm: policy: preallocate inexact bins before xfrm_hash_rebuild reinsert
  2026-07-03  5:19 [PATCH ipsec v2] xfrm: policy: preallocate inexact bins before xfrm_hash_rebuild reinsert Xiang Mei (Microsoft)
@ 2026-07-03  5:47 ` Florian Westphal
  2026-07-07  6:35   ` Steffen Klassert
  2026-07-03  6:18 ` Manuel Ebner
  1 sibling, 1 reply; 4+ messages in thread
From: Florian Westphal @ 2026-07-03  5:47 UTC (permalink / raw)
  To: Xiang Mei (Microsoft)
  Cc: steffen.klassert, herbert, davem, netdev, horms, edumazet, kuba,
	pabeni, AutonomousCodeSecurity, tgopinath, kys

Xiang Mei (Microsoft) <xmei5@asu.edu> 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. 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; the next rebuild's hlist_del_rcu()
> dereferences LIST_POISON2 and takes a GPF. 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>

Reviewed-by: Florian Westphal <fw@strlen.de>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH ipsec v2] xfrm: policy: preallocate inexact bins before xfrm_hash_rebuild reinsert
  2026-07-03  5:19 [PATCH ipsec v2] xfrm: policy: preallocate inexact bins before xfrm_hash_rebuild reinsert Xiang Mei (Microsoft)
  2026-07-03  5:47 ` Florian Westphal
@ 2026-07-03  6:18 ` Manuel Ebner
  1 sibling, 0 replies; 4+ messages in thread
From: Manuel Ebner @ 2026-07-03  6:18 UTC (permalink / raw)
  To: Xiang Mei (Microsoft), fw, steffen.klassert, herbert, davem,
	netdev
  Cc: horms, edumazet, kuba, pabeni, AutonomousCodeSecurity, tgopinath,
	kys

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);

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH ipsec v2] xfrm: policy: preallocate inexact bins before xfrm_hash_rebuild reinsert
  2026-07-03  5:47 ` Florian Westphal
@ 2026-07-07  6:35   ` Steffen Klassert
  0 siblings, 0 replies; 4+ messages in thread
From: Steffen Klassert @ 2026-07-07  6:35 UTC (permalink / raw)
  To: Florian Westphal
  Cc: Xiang Mei (Microsoft), herbert, davem, netdev, horms, edumazet,
	kuba, pabeni, AutonomousCodeSecurity, tgopinath, kys

On Fri, Jul 03, 2026 at 07:47:19AM +0200, Florian Westphal wrote:
> Xiang Mei (Microsoft) <xmei5@asu.edu> 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. 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; the next rebuild's hlist_del_rcu()
> > dereferences LIST_POISON2 and takes a GPF. 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>
> 
> Reviewed-by: Florian Westphal <fw@strlen.de>

Applied, thanks everyone!

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-07-07  6:35 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-03  5:19 [PATCH ipsec v2] xfrm: policy: preallocate inexact bins before xfrm_hash_rebuild reinsert Xiang Mei (Microsoft)
2026-07-03  5:47 ` Florian Westphal
2026-07-07  6:35   ` Steffen Klassert
2026-07-03  6:18 ` Manuel Ebner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox