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

login
register
mail settings
Submitter Wei Yongjun
Date Dec. 1, 2008, 2:10 a.m.
Message ID <49334788.4030409@cn.fujitsu.com>
Download mbox | patch
Permalink /patch/11498/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Wei Yongjun - Dec. 1, 2008, 2:10 a.m.
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(-)
Yang Hongyang - Dec. 1, 2008, 2:17 a.m.
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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Herbert Xu - Dec. 1, 2008, 5 a.m.
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,
Wei Yongjun - Dec. 1, 2008, 6:25 a.m.
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.


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

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,