[Openswan dev] mangled packets, ESP on mast0, after ip_select_ident() fix

David McCullough david_mccullough at mcafee.com
Thu Jul 21 06:21:24 EDT 2011


Jivin Paul Wouters lays it down ...
> On Tue, 19 Jul 2011, David McCullough wrote:
> 
> > Jivin Paul Wouters lays it down ...
> >>
> >> I've tracked down the recent issues with protostack=mast, and found that the
> >> following commit seems to have badly broken the mast stack:
> 
> > I'm a bit confused,  does the patch below fix something to do with the
> > change above ?  They seem completely unrelated to me.  If thats the only
> > change needed to fix it then go for it ;-)
> 
> Oops, that was not the entire patch. It was the diff after reverting, so
> the full undo action was this:

Ok,  the reordering of the ipsec_is_mast_device in this patch caused the
problems.  Easy fix was to use ixs->mast_mode instead ;-)

It was only host-host connections that seemed to be affected,  net-net
connections seemed to be ok before the fix and are still ok ;-)

The problem showed as xmit packets recursing into mast,

Cheers,
Davidm


> diff --git a/linux/include/openswan/ipsec_xmit.h b/linux/include/openswan/ipsec_xmit.h
> index ccee7ce..09e2a17 100644
> --- a/linux/include/openswan/ipsec_xmit.h
> +++ b/linux/include/openswan/ipsec_xmit.h
> @@ -135,7 +135,6 @@ struct ipsec_xmit_state
>   	 * xmit flags
>   	 */
>   	uint16_t mast_mode:1;
> -	uint16_t set_dst:1;
> 
>   	/* if carrying IPv6,  IPPROTO_IPV6, else IPPROTO_IPIP */
>   	uint8_t ipip_proto;
> @@ -185,8 +184,11 @@ extern atomic_t ipsec_ixs_cnt;
> 
>   extern void ipsec_extract_ports(struct sk_buff *skb, unsigned char nexthdr, int nexthdroff, struct sockaddr_encap * er);
> 
> +/* avoid forward reference complain on < 2.5 */
> +struct flowi;
> +
>   extern enum ipsec_xmit_value
> -ipsec_xmit_send(struct ipsec_xmit_state *ixs);
> +ipsec_xmit_send(struct ipsec_xmit_state*ixs, struct flowi *fl);
> 
>   extern enum ipsec_xmit_value
>   ipsec_nat_encap(struct ipsec_xmit_state*ixs);
> diff --git a/linux/net/ipsec/ipsec_mast.c b/linux/net/ipsec/ipsec_mast.c
> index 7d5596f..c49f7c3 100644
> --- a/linux/net/ipsec/ipsec_mast.c
> +++ b/linux/net/ipsec/ipsec_mast.c
> @@ -278,7 +278,17 @@ ipsec_mast_xsm_complete(
>   	}
>   #endif
> 
> -	ipsec_xmit_send(ixs);
> +#ifdef NETDEV_25	/* 2.6 kernels */
> +	/* now send the packet again */
> +	{
> +		struct flowi fl;
> + 
> +		memset(&fl, 0, sizeof(fl));
> +		ipsec_xmit_send(ixs, &fl);
> +	}
> +#else
> +	ipsec_xmit_send(ixs, NULL);
> +#endif
> 
>   cleanup:
>   	ipsec_xmit_cleanup(ixs);
> diff --git a/linux/net/ipsec/ipsec_xmit.c b/linux/net/ipsec/ipsec_xmit.c
> index 4fbb1bb..3408a7a 100644
> --- a/linux/net/ipsec/ipsec_xmit.c
> +++ b/linux/net/ipsec/ipsec_xmit.c
> @@ -109,8 +109,6 @@ static __u32 zeroes[64];
>   #endif
>   #endif
> 
> -static int ipsec_set_dst(struct ipsec_xmit_state *ixs);
> -
>   int ipsec_xmit_trap_count = 0;
>   int ipsec_xmit_trap_sendcount = 0;
> 
> @@ -1181,7 +1179,6 @@ ipsec_xmit_ah(struct ipsec_xmit_state *ixs)
>   enum ipsec_xmit_value
>   ipsec_xmit_ipip(struct ipsec_xmit_state *ixs)
>   {
> -	int error;
>   #ifdef CONFIG_KLIPS_IPV6
>   	if (ip_address_family(&ixs->ipsp->ips_said.dst) == AF_INET6) {
>   		ixs->skb->ip_summed = CHECKSUM_NONE;
> @@ -1196,9 +1193,6 @@ ipsec_xmit_ipip(struct ipsec_xmit_state *ixs)
>   		osw_ip6_hdr(ixs)->saddr    = ((struct sockaddr_in6*)(ixs->ipsp->ips_addr_s))->sin6_addr;
>   		osw_ip6_hdr(ixs)->daddr    = ((struct sockaddr_in6*)(ixs->ipsp->ips_addr_d))->sin6_addr;
>   		osw_ip6_hdr(ixs)->nexthdr  = ixs->ipip_proto;
> -		error = ipsec_set_dst(ixs);
> -		if (error != IPSEC_XMIT_OK)
> -			return error;
>   		/* DAVIDM No identification/fragment code here yet */
>   		skb_set_transport_header(ixs->skb, ipsec_skb_offset(ixs->skb, ixs->iph));
>   	} else
> @@ -1225,11 +1219,9 @@ ipsec_xmit_ipip(struct ipsec_xmit_state *ixs)
>   		osw_ip4_hdr(ixs)->daddr    = ((struct sockaddr_in*)(ixs->ipsp->ips_addr_d))->sin_addr.s_addr;
>   		osw_ip4_hdr(ixs)->protocol = ixs->ipip_proto;
>   		osw_ip4_hdr(ixs)->ihl      = sizeof(struct iphdr) >> 2;
> +
>   		/* newer kernels require skb->dst to be set in KLIPS_IP_SELECT_IDENT */
>   		/* we need to do this before any HASH generation is done */
> -		error = ipsec_set_dst(ixs);
> -		if (error != IPSEC_XMIT_OK)
> -			return error;
>   		KLIPS_IP_SELECT_IDENT(osw_ip4_hdr(ixs), ixs->skb);
>   #ifdef NET_21
>   		skb_set_transport_header(ixs->skb, ipsec_skb_offset(ixs->skb, ixs->iph));
> @@ -2509,64 +2501,61 @@ enum ipsec_xmit_value ipsec_nat_encap(struct ipsec_xmit_state *ixs)
>   #endif
> 
> 
> -static int ipsec_set_dst(struct ipsec_xmit_state *ixs)
> +/* avoid forward reference complain on <2.5 */
> +struct flowi;
> +
> +enum ipsec_xmit_value
> +ipsec_xmit_send(struct ipsec_xmit_state*ixs, struct flowi *fl)
>   {
> +	int error;
> +	int is_mast_packet;
>   	struct dst_entry *dst = NULL;
> -	int error = 0;
> -#ifdef NETDEV_25
> -	struct flowi fl;
> -#endif
> 
> -	if (ixs->set_dst)
> -		return IPSEC_XMIT_OK;
> +	/* check if this packet is sent from the mast, before we route */
> +	is_mast_packet = ipsec_is_mast_device(ixs->skb->dev);
> 
>   #ifdef NETDEV_25
> -	memset(&fl, 0, sizeof(fl));
> -
> -	/* new route/dst cache code from James Morris */
> -	ixs->skb->dev = ixs->physdev;
> -	fl.flowi_oif = ixs->physdev ? ixs->physdev->ifindex : 0;
> 
>   #ifdef CONFIG_KLIPS_IPV6
> -	if (osw_ip_hdr_version(ixs) == 6) {
> +	if (ip_hdr(ixs->skb)->version == 6) {
>   		if (ixs->pass)
> -			memset(&fl.nl_u.ip6_u.saddr, 0, sizeof(fl.nl_u.ip6_u.saddr));
> +			memset(&fl->nl_u.ip6_u.saddr, 0, sizeof(fl->nl_u.ip6_u.saddr));
>   		else
> -			fl.nl_u.ip6_u.saddr = osw_ip6_hdr(ixs)->saddr;
> -		fl.nl_u.ip6_u.daddr = osw_ip6_hdr(ixs)->daddr;
> -		/* fl->nl_u.ip6_u.tos = RT_TOS(osw_ip6_hdr(ixs)->tos); */
> -		fl.flowi_proto = IPPROTO_IPV6;
> +			fl->nl_u.ip6_u.saddr = ipv6_hdr(ixs->skb)->saddr;
> +		fl->nl_u.ip6_u.daddr = ipv6_hdr(ixs->skb)->daddr;
> +		/* fl->nl_u.ip6_u.tos = RT_TOS(ipv6_hdr(ixs->skb)->tos); */
> +		fl->flowi_proto = IPPROTO_IPV6;
>   #ifndef FLOW_HAS_NO_MARK
> -		fl.flowi_mark = ixs->skb->mark;
> +		fl->flowi_mark = ixs->skb->mark;
>   #endif
>   #if LINUX_VERSION_CODE <= KERNEL_VERSION(2,6,24)
> -		dst = ip6_route_output(NULL, &fl);
> +		dst = ip6_route_output(NULL, fl);
>   #elif LINUX_VERSION_CODE < KERNEL_VERSION(2,6,39)
> -		dst = ip6_route_output(&init_net, NULL, &fl);
> +		dst = ip6_route_output(&init_net, NULL, fl);
>   #else
> -		dst = ip6_route_output(&init_net, NULL, &fl.nl_u.ip6_u);
> +		dst = ip6_route_output(&init_net, NULL, fl->nl_u.ip6_u);
>   #endif
>   		error = dst->error;
>   	} else
>   #endif /* CONFIG_KLIPS_IPV6 */
>   	{
> -		fl.nl_u.ip4_u.daddr = osw_ip4_hdr(ixs)->daddr;
> -		fl.nl_u.ip4_u.saddr = ixs->pass ? 0 : osw_ip4_hdr(ixs)->saddr;
> -		fl.flowi_tos = RT_TOS(osw_ip4_hdr(ixs)->tos);
> -		fl.flowi_proto = osw_ip4_hdr(ixs)->protocol;
> +               fl->nl_u.ip4_u.daddr = ip_hdr(ixs->skb)->daddr;
> +               fl->nl_u.ip4_u.saddr = ixs->pass ? 0 : ip_hdr(ixs->skb)->saddr;
> +               fl->flowi_tos = RT_TOS(ip_hdr(ixs->skb)->tos);
> +               fl->flowi_proto = ip_hdr(ixs->skb)->protocol;
>   #ifndef FLOW_HAS_NO_MARK
> -		fl.flowi_mark = ixs->skb->mark;
> +		fl->flowi_mark = ixs->skb->mark;
>   #endif
>   #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,39)
>   # if LINUX_VERSION_CODE <= KERNEL_VERSION(2,6,24)
> -		error = ip_route_output_key(&ixs->route, &fl);
> +		error = ip_route_output_key(&ixs->route, fl);
>   # else
> -		error = ip_route_output_key(&init_net, &ixs->route, &fl);
> +		error = ip_route_output_key(&init_net, &ixs->route, fl);
>   # endif
>   		if (ixs->route)
>   			dst = &ipsec_route_dst(ixs->route);
>   #else
> -		ixs->route = ip_route_output_key(&init_net, &fl.u.ip4);
> +		ixs->route = ip_route_output_key(&init_net, &fl->u.ip4);
>   		if (IS_ERR(ixs->route)) {
>   			error = PTR_ERR(ixs->route);
>   			ixs->route = NULL;
> @@ -2582,9 +2571,9 @@ static int ipsec_set_dst(struct ipsec_xmit_state *ixs)
>   #endif
>   	/*skb_orphan(ixs->skb);*/
>   	error = ip_route_output(&ixs->route,
> -				    osw_ip4_hdr(ixs)->daddr,
> -				    ixs->pass ? 0 : osw_ip4_hdr(ixs)->saddr,
> -				    RT_TOS(osw_ip4_hdr(ixs)->tos),
> +				    ip_hdr(ixs->skb)->daddr,
> +				    ixs->pass ? 0 : ip_hdr(ixs->skb)->saddr,
> +				    RT_TOS(ip_hdr(ixs->skb)->tos),
>                                       /* mcr->rgb: should this be 0 instead? */
>   				    ixs->physdev->ifindex);
>   	if (ixs->route)
> @@ -2615,7 +2604,7 @@ static int ipsec_set_dst(struct ipsec_xmit_state *ixs)
>   		ixs->skb->dev = dst->dev;
> 
>   	if(ixs->dev == dst->dev) {
> -		if (osw_ip_hdr_version(ixs) == 6)
> +		if (ip_hdr(ixs->skb)->version == 6)
>   			dst_release(dst);
>   		else
>   			ip_rt_put(ixs->route);
> @@ -2632,33 +2621,6 @@ static int ipsec_set_dst(struct ipsec_xmit_state *ixs)
>   	skb_dst_drop(ixs->skb);
>   	skb_dst_set(ixs->skb, dst);
> 
> -	ixs->set_dst = 1;
> -
> -	return IPSEC_XMIT_OK;
> -}
> -
> -
> -enum ipsec_xmit_value
> -ipsec_xmit_send(struct ipsec_xmit_state *ixs)
> -{
> -	int error;
> -	int is_mast_packet;
> -
> -	if (ixs->skb == NULL || ixs->skb->dev == NULL)
> -		return IPSEC_XMIT_NODEV;
> -
> -	/* check if this packet is sent from the mast, before we route */
> -	is_mast_packet = ipsec_is_mast_device(ixs->skb->dev);
> -
> -	/*
> -	 * ipsec_set_dst may have been done in the IPIP code,  or we do it now.
> -	 * DAVIDM - I actually think it must have always been done before
> -	 *          now,  but ESP without IPIP and no AH may be the
> -	 *          exception.  You cannot hash and then do ipsec_set_dst :-)
> -	 */
> -	error = ipsec_set_dst(ixs);
> -	if (error != IPSEC_XMIT_OK)
> -		return error;
> 
>   	if(ixs->stats) {
>   		ixs->stats->tx_bytes += ixs->skb->len;
> @@ -2735,11 +2697,26 @@ ipsec_xmit_send(struct ipsec_xmit_state *ixs)
>   	return IPSEC_XMIT_OK;
>   }
> 
> +#ifdef NETDEV_25
>   enum ipsec_xmit_value
>   ipsec_tunnel_send(struct ipsec_xmit_state *ixs)
>   {
> -	return ipsec_xmit_send(ixs);
> +	struct flowi fl;
> +	memset(&fl, 0, sizeof(fl));
> +
> +	/* new route/dst cache code from James Morris */
> +	ixs->skb->dev = ixs->physdev;
> + 	fl.flowi_oif = ixs->physdev->ifindex;
> +
> +	return ipsec_xmit_send(ixs, &fl);
>   }
> +#else
> +enum ipsec_xmit_value
> +ipsec_tunnel_send(struct ipsec_xmit_state *ixs)
> +{
> +	return ipsec_xmit_send(ixs, NULL);
> +}
> +#endif
> 
> 
>   /*
> 
> 

-- 
David McCullough,      david_mccullough at mcafee.com,  Ph:+61 734352815
McAfee - SnapGear      http://www.mcafee.com         http://www.uCdot.org


More information about the Dev mailing list