xfrm: Wait for RCU readers during policy netns exit
Subsystems (3)
Steffen Klassert
maintainerPATCH 1/2 xfrm: Wait for RCU readers during policy netns exit
xfrm_policy_fini() frees the policy_bydst hash tables after flushing the policy work items and deleting all policies, but it does not wait for concurrent RCU readers to leave their read-side critical sections first.
The policy_bydst tables are published via rcu_assign_pointer() and are looked up through rcu_dereference_check(), so netns teardown must also wait for an RCU grace period before freeing the table memory.
Fix this by adding synchronize_rcu() before freeing the policy hash tables.
--- net/xfrm/xfrm_policy.c | 2 ++ 1 file changed, 2 insertions(+)
net/xfrm/xfrm_policy.c +2 -0
@@ -4290,6 +4290,8 @@ static void xfrm_policy_fini(struct net *net)#endif xfrm_policy_flush (net,XFRM_POLICY_TYPE_MAIN , false); +synchronize_rcu (); +WARN_ON (!list_empty (&net->xfrm.policy_all));for (dir =0 ; dir <XFRM_POLICY_MAX ; dir++) {
Signature · 1 line
2.43.0
Steffen Klassert
maintainerPATCH 2/2 xfrm: Wait for RCU readers during state netns exit
xfrm_state_fini() flushes the resize and GC work, destroys all states, and then frees the state hash tables and the inbound percpu state cache. Those objects can still be observed by concurrent RCU readers. We need to wait for a RCU grace period before freeing the hash tables and the percpu cache to avoid netns teardown racing with lockless lookups.
Fix this by adding synchronize_rcu() before freeing the state hash tables and the inbound percpu state cache.
--- net/xfrm/xfrm_state.c | 2 ++ 1 file changed, 2 insertions(+)
net/xfrm/xfrm_state.c +2 -0
@@ -3327,6 +3327,8 @@ void xfrm_state_fini(struct net *net)xfrm_state_flush (net,0 , false);flush_work (&xfrm_state_gc_work); +synchronize_rcu (); +WARN_ON (!list_empty (&net->xfrm.state_all));for (i =0 ; i <= net->xfrm.state_hmask; i++) {
Signature · 1 line
2.43.0
Re: [PATCH ipsec 2/2] xfrm: Wait for RCU readers during state netns exit
Quoted 20 lines · Steffen Klassert
xfrm_state_fini()flushes the resize and GC work, destroys all states, and then frees the state hash tables and the inbound percpu state cache. Those objects can still be observed by concurrent RCU readers. We need to wait for a RCU grace period before freeing the hash tables and the percpu cache to avoid netns teardown racing with lockless lookups.Fix this by adding
synchronize_rcu()before freeing the state hash tables and the inbound percpu state cache.Fixes c8406998b801 ("xfrm: state: use rcu_deref and assign_pointer helpers")Signed-off-by Steffen Klassert <[email protected]>--- net/xfrm/xfrm_state.c | 2 ++ 1 file changed, 2 insertions(+)net/xfrm/xfrm_state.c +0
-0
net/xfrm/xfrm_state.c void xfrm_state_fini(struct net *net) · line 3331
Quoted 5 lines
net/xfrm/xfrm_state.c +2
-0@@ -3327,6 +3327,8 @@ void xfrm_state_fini(struct net *net)xfrm_state_flush (net,0 , false);flush_work (&xfrm_state_gc_work); +synchronize_rcu (); +
xfrm_state_flush() is expected to relink all states to the gc list for the gc worker to handle.
The xfrm state worker calls synchronize_rcu() and then frees states on the state gc list.
What scenario does this additional synchronize_rcu() protect against?
AFAICS this one is not needed, but maybe i am missing something?
Steffen Klassert
maintainerRe: [PATCH ipsec 2/2] xfrm: Wait for RCU readers during state netns exit
Quoted 24 lines · Florian Westphal
Quoted 23 lines · Steffen Klassert
xfrm_state_fini()flushes the resize and GC work, destroys all states, and then frees the state hash tables and the inbound percpu state cache. Those objects can still be observed by concurrent RCU readers. We need to wait for a RCU grace period before freeing the hash tables and the percpu cache to avoid netns teardown racing with lockless lookups.Fix this by adding
synchronize_rcu()before freeing the state hash tables and the inbound percpu state cache.Fixes c8406998b801 ("xfrm: state: use rcu_deref and assign_pointer helpers")Signed-off-by Steffen Klassert <[email protected]>--- net/xfrm/xfrm_state.c | 2 ++ 1 file changed, 2 insertions(+)net/xfrm/xfrm_state.c +0
-0@@ -3327,6 +3327,8 @@ void xfrm_state_fini(struct net *net)xfrm_state_flush (net,0 , false);flush_work (&xfrm_state_gc_work);
Quoted 8 lines
net/xfrm/xfrm_state.c void xfrm_state_fini(struct net *net) · line 3331
Quoted 2 lines
net/xfrm/xfrm_state.c +2
-0+synchronize_rcu (); +
xfrm_state_flush()is expected to relink all states to the gc list for the gc worker to handle.The xfrm state worker calls
synchronize_rcu()and then frees states on the state gc list.
Yes, xfrm_state_gc_task() does the synchronize_rcu() already. I found the ploicy thing and had the impression we need it here as well.
I'll skip this one out.
Thanks!
Quoted 11 lines · Steffen Klassert
xfrm_policy_fini()frees thepolicy_bydsthash tables after flushing the policy work items and deleting all policies, but it does not wait for concurrent RCU readers to leave their read-side critical sections first.The
policy_bydsttables are published viarcu_assign_pointer()and are looked up throughrcu_dereference_check(), so netns teardown must also wait for an RCU grace period before freeing the table memory.Fix this by adding
synchronize_rcu()before freeing the policy hash tables.Fixes e1e551bc5630 ("xfrm: policy: prepare policy_bydst hash for rcu lookups")
Steffen Klassert
maintainerQuoted 9 lines · Florian Westphal
Quoted 8 lines · Steffen Klassert
xfrm_policy_fini()frees thepolicy_bydsthash tables after flushing the policy work items and deleting all policies, but it does not wait for concurrent RCU readers to leave their read-side critical sections first.The
policy_bydsttables are published viarcu_assign_pointer()and are looked up throughrcu_dereference_check(), so netns teardown must also wait for an RCU grace period before freeing the table memory.
Quoted 5 lines
Quoted 3 lines
Fix this by adding
synchronize_rcu()before freeing the policy hash tables.Fixes e1e551bc5630 ("xfrm: policy: prepare policy_bydst hash for rcu lookups")Reviewed-by Florian Westphal <[email protected]>
This is now applied to the ipsec tree.