[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