* [PATCH] xfrm: Fix kernel panic when flush and dump SPD entries @ 2008-12-01 2:00 Wei Yongjun 2008-12-01 2:10 ` [PATCHv2] " Wei Yongjun 0 siblings, 1 reply; 15+ messages in thread From: Wei Yongjun @ 2008-12-01 2:00 UTC (permalink / raw) To: David Miller, netdev@vger.kernel.org, Herbert Xu After flush the SPD entries, dump the SPD entries will cause kernel painc. Used the following commands to reproduct: ah/tunnel/3ffe:501:ffff:ff00:200:ff:fe00:b0b0-3ffe:501:ffff:ff02:200:ff:fe00:a1a1/require;\ spddump;' | setkey -c ah/tunnel/3ffe:501:ffff:ff00:200:ff:fe00:b0b0-3ffe:501:ffff:ff02:200:ff:fe00:a1a1/require;\ spddump;' | setkey -c This is because when flush the SPD entries, the SPD entry is not remove from the list. This patch fix the problem by remove the SPD entry from the list. Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com> --- net/xfrm/xfrm_policy.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c index 058f04f..fb216c9 100644 --- a/net/xfrm/xfrm_policy.c +++ b/net/xfrm/xfrm_policy.c @@ -817,6 +817,7 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info) continue; hlist_del(&pol->bydst); hlist_del(&pol->byidx); + list_del(&pol->walk.all); write_unlock_bh(&xfrm_policy_lock); xfrm_audit_policy_delete(pol, 1, audit_info->loginuid, -- 1.6.0.2.530.g67faa ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCHv2] xfrm: Fix kernel panic when flush and dump SPD entries 2008-12-01 2:00 [PATCH] xfrm: Fix kernel panic when flush and dump SPD entries Wei Yongjun @ 2008-12-01 2:10 ` Wei Yongjun 2008-12-01 2:17 ` Yang Hongyang 2008-12-01 5:00 ` Herbert Xu 0 siblings, 2 replies; 15+ messages in thread From: Wei Yongjun @ 2008-12-01 2:10 UTC (permalink / raw) To: Wei Yongjun; +Cc: David Miller, netdev@vger.kernel.org, Herbert Xu After flush the SPD entries, dump the SPD entries will cause kernel painc. Used the following commands to reproduct: - echo 'spdflush;' | setkey -c - echo 'spdadd 3ffe:501:ffff:ff01::/64 3ffe:501:ffff:ff04::/64 any -P out ipsec \ ah/tunnel/3ffe:501:ffff:ff00:200:ff:fe00:b0b0-3ffe:501:ffff:ff02:200:ff:fe00:a1a1/require;\ spddump;' | setkey -c - echo 'spdflush; spddump;' | setkey -c - echo 'spdadd 3ffe:501:ffff:ff01::/64 3ffe:501:ffff:ff04::/64 any -P out ipsec \ ah/tunnel/3ffe:501:ffff:ff00:200:ff:fe00:b0b0-3ffe:501:ffff:ff02:200:ff:fe00:a1a1/require;\ spddump;' | setkey -c This is because when flush the SPD entries, the SPD entry is not remove from the list. This patch fix the problem by remove the SPD entry from the list. Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com> --- net/xfrm/xfrm_policy.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c index 058f04f..fb216c9 100644 --- a/net/xfrm/xfrm_policy.c +++ b/net/xfrm/xfrm_policy.c @@ -817,6 +817,7 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info) continue; hlist_del(&pol->bydst); hlist_del(&pol->byidx); + list_del(&pol->walk.all); write_unlock_bh(&xfrm_policy_lock); xfrm_audit_policy_delete(pol, 1, audit_info->loginuid, -- 1.6.0.2.530.g67faa ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCHv2] xfrm: Fix kernel panic when flush and dump SPD entries 2008-12-01 2:10 ` [PATCHv2] " Wei Yongjun @ 2008-12-01 2:17 ` Yang Hongyang 2008-12-01 5:00 ` Herbert Xu 1 sibling, 0 replies; 15+ messages in thread From: Yang Hongyang @ 2008-12-01 2:17 UTC (permalink / raw) To: Wei Yongjun; +Cc: David Miller, netdev@vger.kernel.org, Herbert Xu Wei Yongjun wrote: > After flush the SPD entries, dump the SPD entries will cause kernel painc. > > Used the following commands to reproduct: > > - echo 'spdflush;' | setkey -c > - echo 'spdadd 3ffe:501:ffff:ff01::/64 3ffe:501:ffff:ff04::/64 any -P out ipsec \ > ah/tunnel/3ffe:501:ffff:ff00:200:ff:fe00:b0b0-3ffe:501:ffff:ff02:200:ff:fe00:a1a1/require;\ > spddump;' | setkey -c > - echo 'spdflush; spddump;' | setkey -c > - echo 'spdadd 3ffe:501:ffff:ff01::/64 3ffe:501:ffff:ff04::/64 any -P out ipsec \ > ah/tunnel/3ffe:501:ffff:ff00:200:ff:fe00:b0b0-3ffe:501:ffff:ff02:200:ff:fe00:a1a1/require;\ > spddump;' | setkey -c > > This is because when flush the SPD entries, the SPD entry is not remove > from the list. > > This patch fix the problem by remove the SPD entry from the list. > > Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com> > --- > net/xfrm/xfrm_policy.c | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > > diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c > index 058f04f..fb216c9 100644 > --- a/net/xfrm/xfrm_policy.c > +++ b/net/xfrm/xfrm_policy.c > @@ -817,6 +817,7 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info) > continue; > hlist_del(&pol->bydst); > hlist_del(&pol->byidx); > + list_del(&pol->walk.all); > write_unlock_bh(&xfrm_policy_lock); > > xfrm_audit_policy_delete(pol, 1, audit_info->loginuid, Ack. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCHv2] xfrm: Fix kernel panic when flush and dump SPD entries 2008-12-01 2:10 ` [PATCHv2] " Wei Yongjun 2008-12-01 2:17 ` Yang Hongyang @ 2008-12-01 5:00 ` Herbert Xu 2008-12-01 6:25 ` Wei Yongjun 2008-12-01 6:27 ` [PATCHv3] " Wei Yongjun 1 sibling, 2 replies; 15+ messages in thread From: Herbert Xu @ 2008-12-01 5:00 UTC (permalink / raw) To: Wei Yongjun; +Cc: David Miller, netdev@vger.kernel.org On Mon, Dec 01, 2008 at 10:10:16AM +0800, Wei Yongjun wrote: > > diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c > index 058f04f..fb216c9 100644 > --- a/net/xfrm/xfrm_policy.c > +++ b/net/xfrm/xfrm_policy.c > @@ -817,6 +817,7 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info) > continue; > hlist_del(&pol->bydst); > hlist_del(&pol->byidx); > + list_del(&pol->walk.all); Good catch. In fact there so man occurrences of these three calls that perhaps we should put them (and any other relevant code) in a helper. Cheers, -- Visit Openswan at http://www.openswan.org/ Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCHv2] xfrm: Fix kernel panic when flush and dump SPD entries 2008-12-01 5:00 ` Herbert Xu @ 2008-12-01 6:25 ` Wei Yongjun 2008-12-01 6:27 ` [PATCHv3] " Wei Yongjun 1 sibling, 0 replies; 15+ messages in thread From: Wei Yongjun @ 2008-12-01 6:25 UTC (permalink / raw) To: Herbert Xu; +Cc: David Miller, netdev@vger.kernel.org Herbert Xu wrote: > On Mon, Dec 01, 2008 at 10:10:16AM +0800, Wei Yongjun wrote: > >> diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c >> index 058f04f..fb216c9 100644 >> --- a/net/xfrm/xfrm_policy.c >> +++ b/net/xfrm/xfrm_policy.c >> @@ -817,6 +817,7 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info) >> continue; >> hlist_del(&pol->bydst); >> hlist_del(&pol->byidx); >> + list_del(&pol->walk.all); >> > > Good catch. In fact there so man occurrences of these three calls > that perhaps we should put them (and any other relevant code) in a > helper. > I think you mean used __xfrm_policy_unlink() function to instead those codes. 1098 static struct xfrm_policy *__xfrm_policy_unlink(struct xfrm_policy *pol, 1099 int dir) 1100 { 1101 if (hlist_unhashed(&pol->bydst)) 1102 return NULL; 1103 1104 hlist_del(&pol->bydst); 1105 hlist_del(&pol->byidx); 1106 list_del(&pol->walk.all); 1107 xfrm_policy_count[dir]--; 1108 1109 return pol; 1110 } I will post the patch later. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCHv3] xfrm: Fix kernel panic when flush and dump SPD entries 2008-12-01 5:00 ` Herbert Xu 2008-12-01 6:25 ` Wei Yongjun @ 2008-12-01 6:27 ` Wei Yongjun 2008-12-01 7:50 ` [PATCH v4] " Wei Yongjun 1 sibling, 1 reply; 15+ messages in thread From: Wei Yongjun @ 2008-12-01 6:27 UTC (permalink / raw) To: Herbert Xu, David Miller; +Cc: netdev@vger.kernel.org, Wei Yongjun After flush the SPD entries, dump the SPD entries will cause kernel painc. Used the following commands to reproduct: - echo 'spdflush;' | setkey -c - echo 'spdadd 3ffe:501:ffff:ff01::/64 3ffe:501:ffff:ff04::/64 any -P out ipsec \ ah/tunnel/3ffe:501:ffff:ff00:200:ff:fe00:b0b0-3ffe:501:ffff:ff02:200:ff:fe00:a1a1/require;\ spddump;' | setkey -c - echo 'spdflush; spddump;' | setkey -c - echo 'spdadd 3ffe:501:ffff:ff01::/64 3ffe:501:ffff:ff04::/64 any -P out ipsec \ ah/tunnel/3ffe:501:ffff:ff00:200:ff:fe00:b0b0-3ffe:501:ffff:ff02:200:ff:fe00:a1a1/require;\ spddump;' | setkey -c This is because when flush the SPD entries, the SPD entry is not remove from the list. This patch fix the problem by remove the SPD entry from the list. And also do clean up of remove SPD entry. Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com> --- net/xfrm/xfrm_policy.c | 35 ++++++++++------------------------- 1 files changed, 10 insertions(+), 25 deletions(-) diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c index 058f04f..54e7efe 100644 --- a/net/xfrm/xfrm_policy.c +++ b/net/xfrm/xfrm_policy.c @@ -63,6 +63,9 @@ static struct xfrm_policy_afinfo *xfrm_policy_get_afinfo(unsigned short family); static void xfrm_policy_put_afinfo(struct xfrm_policy_afinfo *afinfo); static void xfrm_init_pmtu(struct dst_entry *dst); +static struct xfrm_policy *__xfrm_policy_unlink(struct xfrm_policy *pol, + int dir); + static inline int __xfrm4_selector_match(struct xfrm_selector *sel, struct flowi *fl) { @@ -602,12 +605,8 @@ int xfrm_policy_insert(int dir, struct xfrm_policy *policy, int excl) xfrm_pol_hold(policy); xfrm_policy_count[dir]++; atomic_inc(&flow_cache_genid); - if (delpol) { - hlist_del(&delpol->bydst); - hlist_del(&delpol->byidx); - list_del(&delpol->walk.all); - xfrm_policy_count[dir]--; - } + if (delpol) + __xfrm_policy_unlink(delpol, dir); policy->index = delpol ? delpol->index : xfrm_gen_index(policy->type, dir); hlist_add_head(&policy->byidx, xfrm_policy_byidx+idx_hash(policy->index)); policy->curlft.add_time = get_seconds(); @@ -679,10 +678,7 @@ struct xfrm_policy *xfrm_policy_bysel_ctx(u8 type, int dir, write_unlock_bh(&xfrm_policy_lock); return pol; } - hlist_del(&pol->bydst); - hlist_del(&pol->byidx); - list_del(&pol->walk.all); - xfrm_policy_count[dir]--; + __xfrm_policy_unlink(pol, dir); } ret = pol; break; @@ -723,10 +719,7 @@ struct xfrm_policy *xfrm_policy_byid(u8 type, int dir, u32 id, int delete, write_unlock_bh(&xfrm_policy_lock); return pol; } - hlist_del(&pol->bydst); - hlist_del(&pol->byidx); - list_del(&pol->walk.all); - xfrm_policy_count[dir]--; + __xfrm_policy_unlink(pol, dir); } ret = pol; break; @@ -807,16 +800,14 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info) for (dir = 0; dir < XFRM_POLICY_MAX; dir++) { struct xfrm_policy *pol; struct hlist_node *entry; - int i, killed; + int i; - killed = 0; again1: hlist_for_each_entry(pol, entry, &xfrm_policy_inexact[dir], bydst) { if (pol->type != type) continue; - hlist_del(&pol->bydst); - hlist_del(&pol->byidx); + __xfrm_policy_unlink(pol, dir); write_unlock_bh(&xfrm_policy_lock); xfrm_audit_policy_delete(pol, 1, audit_info->loginuid, @@ -824,7 +815,6 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info) audit_info->secid); xfrm_policy_kill(pol); - killed++; write_lock_bh(&xfrm_policy_lock); goto again1; @@ -837,9 +827,7 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info) bydst) { if (pol->type != type) continue; - hlist_del(&pol->bydst); - hlist_del(&pol->byidx); - list_del(&pol->walk.all); + __xfrm_policy_unlink(pol, dir); write_unlock_bh(&xfrm_policy_lock); xfrm_audit_policy_delete(pol, 1, @@ -847,14 +835,11 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info) audit_info->sessionid, audit_info->secid); xfrm_policy_kill(pol); - killed++; write_lock_bh(&xfrm_policy_lock); goto again2; } } - - xfrm_policy_count[dir] -= killed; } atomic_inc(&flow_cache_genid); out: -- 1.6.0.2.530.g67faa ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v4] xfrm: Fix kernel panic when flush and dump SPD entries 2008-12-01 6:27 ` [PATCHv3] " Wei Yongjun @ 2008-12-01 7:50 ` Wei Yongjun 2008-12-01 8:11 ` Herbert Xu 0 siblings, 1 reply; 15+ messages in thread From: Wei Yongjun @ 2008-12-01 7:50 UTC (permalink / raw) To: Herbert Xu, David Miller; +Cc: netdev@vger.kernel.org After flush the SPD entries, dump the SPD entries will cause kernel painc. Used the following commands to reproduct: - echo 'spdflush;' | setkey -c - echo 'spdadd 3ffe:501:ffff:ff01::/64 3ffe:501:ffff:ff04::/64 any -P out ipsec \ ah/tunnel/3ffe:501:ffff:ff00:200:ff:fe00:b0b0-3ffe:501:ffff:ff02:200:ff:fe00:a1a1/require;\ spddump;' | setkey -c - echo 'spdflush; spddump;' | setkey -c - echo 'spdadd 3ffe:501:ffff:ff01::/64 3ffe:501:ffff:ff04::/64 any -P out ipsec \ ah/tunnel/3ffe:501:ffff:ff00:200:ff:fe00:b0b0-3ffe:501:ffff:ff02:200:ff:fe00:a1a1/require;\ spddump;' | setkey -c This is because when flush the SPD entries, the SPD entry is not remove from the list. This patch fix the problem by remove the SPD entry from the list. And also do clean up of remove SPD entry. Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com> --- net/xfrm/xfrm_policy.c | 35 ++++++++++------------------------- 1 files changed, 10 insertions(+), 25 deletions(-) diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c index 058f04f..54e7efe 100644 --- a/net/xfrm/xfrm_policy.c +++ b/net/xfrm/xfrm_policy.c @@ -63,6 +63,9 @@ static struct xfrm_policy_afinfo *xfrm_policy_get_afinfo(unsigned short family); static void xfrm_policy_put_afinfo(struct xfrm_policy_afinfo *afinfo); static void xfrm_init_pmtu(struct dst_entry *dst); +static struct xfrm_policy *__xfrm_policy_unlink(struct xfrm_policy *pol, + int dir); + static inline int __xfrm4_selector_match(struct xfrm_selector *sel, struct flowi *fl) { @@ -602,12 +605,8 @@ int xfrm_policy_insert(int dir, struct xfrm_policy *policy, int excl) xfrm_pol_hold(policy); xfrm_policy_count[dir]++; atomic_inc(&flow_cache_genid); - if (delpol) { - hlist_del(&delpol->bydst); - hlist_del(&delpol->byidx); - list_del(&delpol->walk.all); - xfrm_policy_count[dir]--; - } + if (delpol) + __xfrm_policy_unlink(delpol, dir); policy->index = delpol ? delpol->index : xfrm_gen_index(policy->type, dir); hlist_add_head(&policy->byidx, xfrm_policy_byidx+idx_hash(policy->index)); policy->curlft.add_time = get_seconds(); @@ -679,10 +678,7 @@ struct xfrm_policy *xfrm_policy_bysel_ctx(u8 type, int dir, write_unlock_bh(&xfrm_policy_lock); return pol; } - hlist_del(&pol->bydst); - hlist_del(&pol->byidx); - list_del(&pol->walk.all); - xfrm_policy_count[dir]--; + __xfrm_policy_unlink(pol, dir); } ret = pol; break; @@ -723,10 +719,7 @@ struct xfrm_policy *xfrm_policy_byid(u8 type, int dir, u32 id, int delete, write_unlock_bh(&xfrm_policy_lock); return pol; } - hlist_del(&pol->bydst); - hlist_del(&pol->byidx); - list_del(&pol->walk.all); - xfrm_policy_count[dir]--; + __xfrm_policy_unlink(pol, dir); } ret = pol; break; @@ -807,16 +800,14 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info) for (dir = 0; dir < XFRM_POLICY_MAX; dir++) { struct xfrm_policy *pol; struct hlist_node *entry; - int i, killed; + int i; - killed = 0; again1: hlist_for_each_entry(pol, entry, &xfrm_policy_inexact[dir], bydst) { if (pol->type != type) continue; - hlist_del(&pol->bydst); - hlist_del(&pol->byidx); + __xfrm_policy_unlink(pol, dir); write_unlock_bh(&xfrm_policy_lock); xfrm_audit_policy_delete(pol, 1, audit_info->loginuid, @@ -824,7 +815,6 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info) audit_info->secid); xfrm_policy_kill(pol); - killed++; write_lock_bh(&xfrm_policy_lock); goto again1; @@ -837,9 +827,7 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info) bydst) { if (pol->type != type) continue; - hlist_del(&pol->bydst); - hlist_del(&pol->byidx); - list_del(&pol->walk.all); + __xfrm_policy_unlink(pol, dir); write_unlock_bh(&xfrm_policy_lock); xfrm_audit_policy_delete(pol, 1, @@ -847,14 +835,11 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info) audit_info->sessionid, audit_info->secid); xfrm_policy_kill(pol); - killed++; write_lock_bh(&xfrm_policy_lock); goto again2; } } - - xfrm_policy_count[dir] -= killed; } atomic_inc(&flow_cache_genid); out: -- 1.6.0.2.530.g67faa ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v4] xfrm: Fix kernel panic when flush and dump SPD entries 2008-12-01 7:50 ` [PATCH v4] " Wei Yongjun @ 2008-12-01 8:11 ` Herbert Xu 2008-12-01 8:38 ` David Miller 0 siblings, 1 reply; 15+ messages in thread From: Herbert Xu @ 2008-12-01 8:11 UTC (permalink / raw) To: Wei Yongjun; +Cc: David Miller, netdev@vger.kernel.org On Mon, Dec 01, 2008 at 03:50:54PM +0800, Wei Yongjun wrote: > After flush the SPD entries, dump the SPD entries will cause kernel painc. > > Used the following commands to reproduct: > > - echo 'spdflush;' | setkey -c > - echo 'spdadd 3ffe:501:ffff:ff01::/64 3ffe:501:ffff:ff04::/64 any -P out ipsec \ > ah/tunnel/3ffe:501:ffff:ff00:200:ff:fe00:b0b0-3ffe:501:ffff:ff02:200:ff:fe00:a1a1/require;\ > spddump;' | setkey -c > - echo 'spdflush; spddump;' | setkey -c > - echo 'spdadd 3ffe:501:ffff:ff01::/64 3ffe:501:ffff:ff04::/64 any -P out ipsec \ > ah/tunnel/3ffe:501:ffff:ff00:200:ff:fe00:b0b0-3ffe:501:ffff:ff02:200:ff:fe00:a1a1/require;\ > spddump;' | setkey -c > > This is because when flush the SPD entries, the SPD entry is not remove > from the list. > > This patch fix the problem by remove the SPD entry from the list. And > also do clean up of remove SPD entry. I like the clean-up but it's best if you separate the two changes into two patches. That way it's easy to see exactly what the bug fix patch is trying to fix and to verify that the clean-up patch really doesn't change behaviour. Thanks, -- Visit Openswan at http://www.openswan.org/ Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v4] xfrm: Fix kernel panic when flush and dump SPD entries 2008-12-01 8:11 ` Herbert Xu @ 2008-12-01 8:38 ` David Miller 2008-12-01 9:50 ` [PATCH 1/2] " Wei Yongjun 2008-12-01 9:55 ` [PATCH 1/2] xfrm: Cleanup for unlink SPD entry Wei Yongjun 0 siblings, 2 replies; 15+ messages in thread From: David Miller @ 2008-12-01 8:38 UTC (permalink / raw) To: herbert; +Cc: yjwei, netdev From: Herbert Xu <herbert@gondor.apana.org.au> Date: Mon, 1 Dec 2008 16:11:11 +0800 > On Mon, Dec 01, 2008 at 03:50:54PM +0800, Wei Yongjun wrote: > > After flush the SPD entries, dump the SPD entries will cause kernel painc. > > > > Used the following commands to reproduct: > > > > - echo 'spdflush;' | setkey -c > > - echo 'spdadd 3ffe:501:ffff:ff01::/64 3ffe:501:ffff:ff04::/64 any -P out ipsec \ > > ah/tunnel/3ffe:501:ffff:ff00:200:ff:fe00:b0b0-3ffe:501:ffff:ff02:200:ff:fe00:a1a1/require;\ > > spddump;' | setkey -c > > - echo 'spdflush; spddump;' | setkey -c > > - echo 'spdadd 3ffe:501:ffff:ff01::/64 3ffe:501:ffff:ff04::/64 any -P out ipsec \ > > ah/tunnel/3ffe:501:ffff:ff00:200:ff:fe00:b0b0-3ffe:501:ffff:ff02:200:ff:fe00:a1a1/require;\ > > spddump;' | setkey -c > > > > This is because when flush the SPD entries, the SPD entry is not remove > > from the list. > > > > This patch fix the problem by remove the SPD entry from the list. And > > also do clean up of remove SPD entry. > > I like the clean-up but it's best if you separate the two changes > into two patches. That way it's easy to see exactly what the bug > fix patch is trying to fix and to verify that the clean-up patch > really doesn't change behaviour. Agreed. Wei, please do this. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/2] xfrm: Fix kernel panic when flush and dump SPD entries 2008-12-01 8:38 ` David Miller @ 2008-12-01 9:50 ` Wei Yongjun 2008-12-03 8:28 ` David Miller 2008-12-01 9:55 ` [PATCH 1/2] xfrm: Cleanup for unlink SPD entry Wei Yongjun 1 sibling, 1 reply; 15+ messages in thread From: Wei Yongjun @ 2008-12-01 9:50 UTC (permalink / raw) To: David Miller, herbert; +Cc: netdev After flush the SPD entries, dump the SPD entries will cause kernel painc. Used the following commands to reproduct: - echo 'spdflush;' | setkey -c - echo 'spdadd 3ffe:501:ffff:ff01::/64 3ffe:501:ffff:ff04::/64 any -P out ipsec \ ah/tunnel/3ffe:501:ffff:ff00:200:ff:fe00:b0b0-3ffe:501:ffff:ff02:200:ff:fe00:a1a1/require;\ spddump;' | setkey -c - echo 'spdflush; spddump;' | setkey -c - echo 'spdadd 3ffe:501:ffff:ff01::/64 3ffe:501:ffff:ff04::/64 any -P out ipsec \ ah/tunnel/3ffe:501:ffff:ff00:200:ff:fe00:b0b0-3ffe:501:ffff:ff02:200:ff:fe00:a1a1/require;\ spddump;' | setkey -c This is because when flush the SPD entries, the SPD entry is not remove from the list. This patch fix the problem by remove the SPD entry from the list. Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com> --- net/xfrm/xfrm_policy.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c index 058f04f..fb216c9 100644 --- a/net/xfrm/xfrm_policy.c +++ b/net/xfrm/xfrm_policy.c @@ -817,6 +817,7 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info) continue; hlist_del(&pol->bydst); hlist_del(&pol->byidx); + list_del(&pol->walk.all); write_unlock_bh(&xfrm_policy_lock); xfrm_audit_policy_delete(pol, 1, audit_info->loginuid, -- 1.6.0.2.530.g67faa ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] xfrm: Fix kernel panic when flush and dump SPD entries 2008-12-01 9:50 ` [PATCH 1/2] " Wei Yongjun @ 2008-12-03 8:28 ` David Miller 0 siblings, 0 replies; 15+ messages in thread From: David Miller @ 2008-12-03 8:28 UTC (permalink / raw) To: yjwei; +Cc: herbert, netdev From: Wei Yongjun <yjwei@cn.fujitsu.com> Date: Mon, 01 Dec 2008 17:50:52 +0800 > After flush the SPD entries, dump the SPD entries will cause kernel painc. ... > This is because when flush the SPD entries, the SPD entry is not remove > from the list. > > This patch fix the problem by remove the SPD entry from the list. > > Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com> Applied to net-2.6, thanks a lot Wei. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/2] xfrm: Cleanup for unlink SPD entry 2008-12-01 8:38 ` David Miller 2008-12-01 9:50 ` [PATCH 1/2] " Wei Yongjun @ 2008-12-01 9:55 ` Wei Yongjun 2008-12-01 9:57 ` Wei Yongjun ` (2 more replies) 1 sibling, 3 replies; 15+ messages in thread From: Wei Yongjun @ 2008-12-01 9:55 UTC (permalink / raw) To: David Miller; +Cc: herbert, netdev Used __xfrm_policy_unlink() to instead of the dup codes when unlink SPD entry. Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com> --- net/xfrm/xfrm_policy.c | 36 ++++++++++-------------------------- 1 files changed, 10 insertions(+), 26 deletions(-) diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c index fb216c9..0de2e1f 100644 --- a/net/xfrm/xfrm_policy.c +++ b/net/xfrm/xfrm_policy.c @@ -63,6 +63,9 @@ static struct xfrm_policy_afinfo *xfrm_policy_get_afinfo(unsigned short family); static void xfrm_policy_put_afinfo(struct xfrm_policy_afinfo *afinfo); static void xfrm_init_pmtu(struct dst_entry *dst); +static struct xfrm_policy *__xfrm_policy_unlink(struct xfrm_policy *pol, + int dir); + static inline int __xfrm4_selector_match(struct xfrm_selector *sel, struct flowi *fl) { @@ -602,12 +605,8 @@ int xfrm_policy_insert(int dir, struct xfrm_policy *policy, int excl) xfrm_pol_hold(policy); xfrm_policy_count[dir]++; atomic_inc(&flow_cache_genid); - if (delpol) { - hlist_del(&delpol->bydst); - hlist_del(&delpol->byidx); - list_del(&delpol->walk.all); - xfrm_policy_count[dir]--; - } + if (delpol) + __xfrm_policy_unlink(delpol, dir); policy->index = delpol ? delpol->index : xfrm_gen_index(policy->type, dir); hlist_add_head(&policy->byidx, xfrm_policy_byidx+idx_hash(policy->index)); policy->curlft.add_time = get_seconds(); @@ -679,10 +678,7 @@ struct xfrm_policy *xfrm_policy_bysel_ctx(u8 type, int dir, write_unlock_bh(&xfrm_policy_lock); return pol; } - hlist_del(&pol->bydst); - hlist_del(&pol->byidx); - list_del(&pol->walk.all); - xfrm_policy_count[dir]--; + __xfrm_policy_unlink(pol, dir); } ret = pol; break; @@ -723,10 +719,7 @@ struct xfrm_policy *xfrm_policy_byid(u8 type, int dir, u32 id, int delete, write_unlock_bh(&xfrm_policy_lock); return pol; } - hlist_del(&pol->bydst); - hlist_del(&pol->byidx); - list_del(&pol->walk.all); - xfrm_policy_count[dir]--; + __xfrm_policy_unlink(pol, dir); } ret = pol; break; @@ -807,17 +800,14 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info) for (dir = 0; dir < XFRM_POLICY_MAX; dir++) { struct xfrm_policy *pol; struct hlist_node *entry; - int i, killed; + int i; - killed = 0; again1: hlist_for_each_entry(pol, entry, &xfrm_policy_inexact[dir], bydst) { if (pol->type != type) continue; - hlist_del(&pol->bydst); - hlist_del(&pol->byidx); - list_del(&pol->walk.all); + __xfrm_policy_unlink(pol, dir); write_unlock_bh(&xfrm_policy_lock); xfrm_audit_policy_delete(pol, 1, audit_info->loginuid, @@ -825,7 +815,6 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info) audit_info->secid); xfrm_policy_kill(pol); - killed++; write_lock_bh(&xfrm_policy_lock); goto again1; @@ -838,9 +827,7 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info) bydst) { if (pol->type != type) continue; - hlist_del(&pol->bydst); - hlist_del(&pol->byidx); - list_del(&pol->walk.all); + __xfrm_policy_unlink(pol, dir); write_unlock_bh(&xfrm_policy_lock); xfrm_audit_policy_delete(pol, 1, @@ -848,14 +835,11 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info) audit_info->sessionid, audit_info->secid); xfrm_policy_kill(pol); - killed++; write_lock_bh(&xfrm_policy_lock); goto again2; } } - - xfrm_policy_count[dir] -= killed; } atomic_inc(&flow_cache_genid); out: -- 1.6.0.2.530.g67faa ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] xfrm: Cleanup for unlink SPD entry 2008-12-01 9:55 ` [PATCH 1/2] xfrm: Cleanup for unlink SPD entry Wei Yongjun @ 2008-12-01 9:57 ` Wei Yongjun 2008-12-02 6:53 ` Herbert Xu 2008-12-03 8:33 ` David Miller 2 siblings, 0 replies; 15+ messages in thread From: Wei Yongjun @ 2008-12-01 9:57 UTC (permalink / raw) To: David Miller; +Cc: herbert, netdev Sorry, the title should be this: [PATCH 2/2] xfrm: Cleanup for unlink SPD entry Wei Yongjun wrote: > Used __xfrm_policy_unlink() to instead of the dup codes when unlink > SPD entry. > > Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com> > --- > net/xfrm/xfrm_policy.c | 36 ++++++++++-------------------------- > 1 files changed, 10 insertions(+), 26 deletions(-) > > diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c > index fb216c9..0de2e1f 100644 > --- a/net/xfrm/xfrm_policy.c > +++ b/net/xfrm/xfrm_policy.c > @@ -63,6 +63,9 @@ static struct xfrm_policy_afinfo *xfrm_policy_get_afinfo(unsigned short family); > static void xfrm_policy_put_afinfo(struct xfrm_policy_afinfo *afinfo); > static void xfrm_init_pmtu(struct dst_entry *dst); > > +static struct xfrm_policy *__xfrm_policy_unlink(struct xfrm_policy *pol, > + int dir); > + > static inline int > __xfrm4_selector_match(struct xfrm_selector *sel, struct flowi *fl) > { > @@ -602,12 +605,8 @@ int xfrm_policy_insert(int dir, struct xfrm_policy *policy, int excl) > xfrm_pol_hold(policy); > xfrm_policy_count[dir]++; > atomic_inc(&flow_cache_genid); > - if (delpol) { > - hlist_del(&delpol->bydst); > - hlist_del(&delpol->byidx); > - list_del(&delpol->walk.all); > - xfrm_policy_count[dir]--; > - } > + if (delpol) > + __xfrm_policy_unlink(delpol, dir); > policy->index = delpol ? delpol->index : xfrm_gen_index(policy->type, dir); > hlist_add_head(&policy->byidx, xfrm_policy_byidx+idx_hash(policy->index)); > policy->curlft.add_time = get_seconds(); > @@ -679,10 +678,7 @@ struct xfrm_policy *xfrm_policy_bysel_ctx(u8 type, int dir, > write_unlock_bh(&xfrm_policy_lock); > return pol; > } > - hlist_del(&pol->bydst); > - hlist_del(&pol->byidx); > - list_del(&pol->walk.all); > - xfrm_policy_count[dir]--; > + __xfrm_policy_unlink(pol, dir); > } > ret = pol; > break; > @@ -723,10 +719,7 @@ struct xfrm_policy *xfrm_policy_byid(u8 type, int dir, u32 id, int delete, > write_unlock_bh(&xfrm_policy_lock); > return pol; > } > - hlist_del(&pol->bydst); > - hlist_del(&pol->byidx); > - list_del(&pol->walk.all); > - xfrm_policy_count[dir]--; > + __xfrm_policy_unlink(pol, dir); > } > ret = pol; > break; > @@ -807,17 +800,14 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info) > for (dir = 0; dir < XFRM_POLICY_MAX; dir++) { > struct xfrm_policy *pol; > struct hlist_node *entry; > - int i, killed; > + int i; > > - killed = 0; > again1: > hlist_for_each_entry(pol, entry, > &xfrm_policy_inexact[dir], bydst) { > if (pol->type != type) > continue; > - hlist_del(&pol->bydst); > - hlist_del(&pol->byidx); > - list_del(&pol->walk.all); > + __xfrm_policy_unlink(pol, dir); > write_unlock_bh(&xfrm_policy_lock); > > xfrm_audit_policy_delete(pol, 1, audit_info->loginuid, > @@ -825,7 +815,6 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info) > audit_info->secid); > > xfrm_policy_kill(pol); > - killed++; > > write_lock_bh(&xfrm_policy_lock); > goto again1; > @@ -838,9 +827,7 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info) > bydst) { > if (pol->type != type) > continue; > - hlist_del(&pol->bydst); > - hlist_del(&pol->byidx); > - list_del(&pol->walk.all); > + __xfrm_policy_unlink(pol, dir); > write_unlock_bh(&xfrm_policy_lock); > > xfrm_audit_policy_delete(pol, 1, > @@ -848,14 +835,11 @@ int xfrm_policy_flush(u8 type, struct xfrm_audit *audit_info) > audit_info->sessionid, > audit_info->secid); > xfrm_policy_kill(pol); > - killed++; > > write_lock_bh(&xfrm_policy_lock); > goto again2; > } > } > - > - xfrm_policy_count[dir] -= killed; > } > atomic_inc(&flow_cache_genid); > out: > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] xfrm: Cleanup for unlink SPD entry 2008-12-01 9:55 ` [PATCH 1/2] xfrm: Cleanup for unlink SPD entry Wei Yongjun 2008-12-01 9:57 ` Wei Yongjun @ 2008-12-02 6:53 ` Herbert Xu 2008-12-03 8:33 ` David Miller 2 siblings, 0 replies; 15+ messages in thread From: Herbert Xu @ 2008-12-02 6:53 UTC (permalink / raw) To: Wei Yongjun; +Cc: David Miller, netdev On Mon, Dec 01, 2008 at 05:55:20PM +0800, Wei Yongjun wrote: > Used __xfrm_policy_unlink() to instead of the dup codes when unlink > SPD entry. > > Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com> Both patches look good to me. Thanks! -- Visit Openswan at http://www.openswan.org/ Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] xfrm: Cleanup for unlink SPD entry 2008-12-01 9:55 ` [PATCH 1/2] xfrm: Cleanup for unlink SPD entry Wei Yongjun 2008-12-01 9:57 ` Wei Yongjun 2008-12-02 6:53 ` Herbert Xu @ 2008-12-03 8:33 ` David Miller 2 siblings, 0 replies; 15+ messages in thread From: David Miller @ 2008-12-03 8:33 UTC (permalink / raw) To: yjwei; +Cc: herbert, netdev From: Wei Yongjun <yjwei@cn.fujitsu.com> Date: Mon, 01 Dec 2008 17:55:20 +0800 > Used __xfrm_policy_unlink() to instead of the dup codes when unlink > SPD entry. > > Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com> I forwarded ported this to net-next-2.6 and added it there. Thanks! ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2008-12-03 13:24 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-12-01 2:00 [PATCH] xfrm: Fix kernel panic when flush and dump SPD entries Wei Yongjun 2008-12-01 2:10 ` [PATCHv2] " Wei Yongjun 2008-12-01 2:17 ` Yang Hongyang 2008-12-01 5:00 ` Herbert Xu 2008-12-01 6:25 ` Wei Yongjun 2008-12-01 6:27 ` [PATCHv3] " Wei Yongjun 2008-12-01 7:50 ` [PATCH v4] " Wei Yongjun 2008-12-01 8:11 ` Herbert Xu 2008-12-01 8:38 ` David Miller 2008-12-01 9:50 ` [PATCH 1/2] " Wei Yongjun 2008-12-03 8:28 ` David Miller 2008-12-01 9:55 ` [PATCH 1/2] xfrm: Cleanup for unlink SPD entry Wei Yongjun 2008-12-01 9:57 ` Wei Yongjun 2008-12-02 6:53 ` Herbert Xu 2008-12-03 8:33 ` David Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).