[PATCH] xfrm: Fix kernel panic when flush and dump SPD entries

[PATCH] xfrm: Fix kernel panic when flush and dump SPD entries

[Wei Yongjun]

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

[PATCHv2] xfrm: Fix kernel panic when flush and dump SPD entries

[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.

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

Re: [PATCHv2] xfrm: Fix kernel panic when flush and dump SPD entries

[Yang Hongyang]

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.

Re: [PATCHv2] xfrm: Fix kernel panic when flush and dump SPD entries

[Herbert Xu]

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

Re: [PATCHv2] xfrm: Fix kernel panic when flush and dump SPD entries

[Wei Yongjun]

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.

[PATCHv3] xfrm: Fix kernel panic when flush and dump SPD entries

[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

[PATCH v4] xfrm: Fix kernel panic when flush and dump SPD entries

[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

Re: [PATCH v4] xfrm: Fix kernel panic when flush and dump SPD entries

[Herbert Xu]

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

Re: [PATCH v4] xfrm: Fix kernel panic when flush and dump SPD entries

[David Miller]

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.

[PATCH 1/2] xfrm: Fix kernel panic when flush and dump SPD entries

[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.

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

[PATCH 1/2] xfrm: Cleanup for unlink SPD entry

[Wei Yongjun]

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

Re: [PATCH 1/2] xfrm: Cleanup for unlink SPD entry

[Wei Yongjun]

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:
>   

Re: [PATCH 1/2] xfrm: Cleanup for unlink SPD entry

[Herbert Xu]

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

Re: [PATCH 1/2] xfrm: Fix kernel panic when flush and dump SPD entries

[David Miller]

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.

Re: [PATCH 1/2] xfrm: Cleanup for unlink SPD entry

[David Miller]

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!