Re: [PATCH net-2.6.23-rc5] ipsec interfamily route handling fix
On Friday 14 September 2007 23:42:52 David Miller wrote: From: Joakim Koskela [EMAIL PROTECTED] Date: Thu, 6 Sep 2007 19:00:10 +0300 This patch addresses a couple of issues related to interfamily ipsec modes. The problem is that the structure of the routing info changes with the family during the __xfrmX_bundle_create, which hasn't been taken properly into account. Seems that by coincidence it hasn't Since nobody else found time to review this, I did :-) Thanks for taking the time, and sorry for not getting back on this until now.. It sets encap_type in the inner loop, but what if we find multiple entries some ipv4 and some ipv6? This logic can't be right. Instead, we need to treat these objects on an individual basis, I think, and that requires a bit more changes. Yes, this is what I was worried about. But as I'm not that familiar with how these dst_entries are used down the line or with subpolicy transformations I didn't feel comfortable rewriting that completely. I'm trying to get this thing solved (any help is of course appreciated :), but in the meantime I think the following bit could actually be separated as, although related, fixes an issue not directly tied to the original problem (..and would be great to get applied as it makes interfamily work quite ok for a number of setups). What do you think? ..and for a short description: This patch resets the ipv4-related flags in the new flow as their content will otherwise depend on the bits of the ipv6 addresses the struct was previously used for. For example, fl4_tos might have RTO_ONLINK set, which usually prevents the right route from being found. -- diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c index 15aa4c5..b4a0b54 100644 --- a/net/ipv6/xfrm6_policy.c +++ b/net/ipv6/xfrm6_policy.c @@ -185,6 +185,8 @@ __xfrm6_bundle_create(struct xfrm_policy *policy, struct xfrm_state **xfrm, int case AF_INET: fl_tunnel.fl4_dst = xfrm[i]-id.daddr.a4; fl_tunnel.fl4_src = xfrm[i]-props.saddr.a4; + fl_tunnel.fl4_tos = 0; + fl_tunnel.fl4_scope = 0; break; case AF_INET6: ipv6_addr_copy(fl_tunnel.fl6_dst, __xfrm6_bundle_addr_remote(xfrm[i], fl-fl6_dst)); - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net-2.6.23-rc5] ipsec interfamily route handling fix
From: Joakim Koskela [EMAIL PROTECTED] Date: Thu, 6 Sep 2007 19:00:10 +0300 This patch addresses a couple of issues related to interfamily ipsec modes. The problem is that the structure of the routing info changes with the family during the __xfrmX_bundle_create, which hasn't been taken properly into account. Seems that by coincidence it hasn't caused problems on 32bit platforms, but crashes for example on x86_64 in 6-4 around line 209 of xfrm6_policy.c as rt doesn't point to a rt6_info anymore, but actually a struct rtable. With 64bit pointers, the rt-rt6i_node pointer seems to hit something usually not null in the rtable that rt now points to, making it go for the path_cookie assignment and subsequently crashing. Tested on both 32/64bit with all four (44/46/64/66) combinations of transformation. I'm still a bit worried about how for example nested transformations work with all of this and would appreciate if someone more familiar with the details of these structs could comment. Signed-off-by: Joakim Koskela [EMAIL PROTECTED] Since nobody else found time to review this, I did :-) It's line wrapped so doesn't apply cleanly, but it has technical issues too. It sets encap_type in the inner loop, but what if we find multiple entries some ipv4 and some ipv6? This logic can't be right. Instead, we need to treat these objects on an individual basis, I think, and that requires a bit more changes. These tunnel handling code blocks are getting messy, perhaps it's time for a little bit of indirection based upon AF type? - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net-2.6.23-rc5] ipsec interfamily route handling fix
From: Joakim Koskela [EMAIL PROTECTED] Date: Thu, 6 Sep 2007 19:00:10 +0300 This patch addresses a couple of issues related to interfamily ipsec modes. The problem is that the structure of the routing info changes with the family during the __xfrmX_bundle_create, which hasn't been taken properly into account. Seems that by coincidence it hasn't caused problems on 32bit platforms, but crashes for example on x86_64 in 6-4 around line 209 of xfrm6_policy.c as rt doesn't point to a rt6_info anymore, but actually a struct rtable. With 64bit pointers, the rt-rt6i_node pointer seems to hit something usually not null in the rtable that rt now points to, making it go for the path_cookie assignment and subsequently crashing. Tested on both 32/64bit with all four (44/46/64/66) combinations of transformation. I'm still a bit worried about how for example nested transformations work with all of this and would appreciate if someone more familiar with the details of these structs could comment. Signed-off-by: Joakim Koskela [EMAIL PROTECTED] This fix basically looks fine to me, but I'd like at least one other person to review it too. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
