* [PATCH] ip6_gre: clear IPCB in ip6gre_xmit2 in case dst_link_failure called @ 2016-02-09 4:07 Bernie Harris 2016-02-16 1:10 ` [PATCH v2] gre: Avoid kernel panic by clearing IPCB before " Bernie Harris 2016-02-17 15:26 ` [PATCH] ip6_gre: clear IPCB in ip6gre_xmit2 in case " David Miller 0 siblings, 2 replies; 9+ messages in thread From: Bernie Harris @ 2016-02-09 4:07 UTC (permalink / raw) To: netdev; +Cc: davem, kuznet, stable, bernie.harris skb->cb may contain data from previous layers (in the observed case the qdisc layer). In the observed scenario, the data was misinterpreted as ip header options, which later caused the ihl to be set to an invalid value (<5). This resulted in an infinite loop in the mips implementation of ip_fast_csum. This patch clears IPCB before dst_link_failure is called, similar to what commit 11c21a30 does for the ipv4 case. Signed-off-by: Bernie Harris <bernie.harris@alliedtelesis.co.nz> --- net/ipv6/ip6_gre.c | 1 + 1 file changed, 1 insertion(+) diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c index f37f18b..e820345 100644 --- a/net/ipv6/ip6_gre.c +++ b/net/ipv6/ip6_gre.c @@ -678,6 +678,7 @@ static netdev_tx_t ip6gre_xmit2(struct sk_buff *skb, tunnel->err_time + IP6TUNNEL_ERR_TIMEO)) { tunnel->err_count--; + memset(IPCB(skb), 0, sizeof(*IPCB(skb))); dst_link_failure(skb); } else tunnel->err_count = 0; -- 2.7.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2] gre: Avoid kernel panic by clearing IPCB before dst_link_failure called 2016-02-09 4:07 [PATCH] ip6_gre: clear IPCB in ip6gre_xmit2 in case dst_link_failure called Bernie Harris @ 2016-02-16 1:10 ` Bernie Harris 2016-02-17 21:31 ` David Miller ` (2 more replies) 2016-02-17 15:26 ` [PATCH] ip6_gre: clear IPCB in ip6gre_xmit2 in case " David Miller 1 sibling, 3 replies; 9+ messages in thread From: Bernie Harris @ 2016-02-16 1:10 UTC (permalink / raw) To: netdev; +Cc: davem, kuznet, stable, bernie.harris skb->cb may contain data from previous layers (in the observed case the qdisc layer). In the observed scenario, the data was misinterpreted as ip header options, which later caused the ihl to be set to an invalid value (<5). This resulted in an infinite loop in the mips implementation of ip_fast_csum. This patch clears IPCB before dst_link_failure is called from the functions ip_tunnel_xmit and ip6gre_xmit2, similar to what commit 11c21a30 does for an ipv4 case. Signed-off-by: Bernie Harris <bernie.harris@alliedtelesis.co.nz> --- net/ipv4/ip_tunnel.c | 1 + net/ipv6/ip6_gre.c | 2 ++ 2 files changed, 3 insertions(+) diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c index 89e8861..946091a 100644 --- a/net/ipv4/ip_tunnel.c +++ b/net/ipv4/ip_tunnel.c @@ -799,6 +799,7 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev, #if IS_ENABLED(CONFIG_IPV6) tx_error_icmp: + memset(IPCB(skb), 0, sizeof(*IPCB(skb))); dst_link_failure(skb); #endif tx_error: diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c index f37f18b..93fc6f9 100644 --- a/net/ipv6/ip6_gre.c +++ b/net/ipv6/ip6_gre.c @@ -678,6 +678,7 @@ static netdev_tx_t ip6gre_xmit2(struct sk_buff *skb, tunnel->err_time + IP6TUNNEL_ERR_TIMEO)) { tunnel->err_count--; + memset(IPCB(skb), 0, sizeof(*IPCB(skb))); dst_link_failure(skb); } else tunnel->err_count = 0; @@ -761,6 +762,7 @@ static netdev_tx_t ip6gre_xmit2(struct sk_buff *skb, return 0; tx_err_link_failure: stats->tx_carrier_errors++; + memset(IPCB(skb), 0, sizeof(*IPCB(skb))); dst_link_failure(skb); tx_err_dst_release: dst_release(dst); -- 2.7.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] gre: Avoid kernel panic by clearing IPCB before dst_link_failure called 2016-02-16 1:10 ` [PATCH v2] gre: Avoid kernel panic by clearing IPCB before " Bernie Harris @ 2016-02-17 21:31 ` David Miller 2016-02-22 4:57 ` Bernie Harris 2016-02-21 23:24 ` Bernie Harris 2016-02-21 23:58 ` Bernie Harris 2 siblings, 1 reply; 9+ messages in thread From: David Miller @ 2016-02-17 21:31 UTC (permalink / raw) To: bernie.harris; +Cc: netdev, kuznet, stable From: Bernie Harris <bernie.harris@alliedtelesis.co.nz> Date: Tue, 16 Feb 2016 14:10:16 +1300 > skb->cb may contain data from previous layers (in the observed case the > qdisc layer). In the observed scenario, the data was misinterpreted as > ip header options, which later caused the ihl to be set to an invalid > value (<5). This resulted in an infinite loop in the mips implementation > of ip_fast_csum. > > This patch clears IPCB before dst_link_failure is called from the functions > ip_tunnel_xmit and ip6gre_xmit2, similar to what commit 11c21a30 does for > an ipv4 case. > > Signed-off-by: Bernie Harris <bernie.harris@alliedtelesis.co.nz> Again, I want to see this implemented in a way which causes things to be treated consistently across all tunneling types. Which means fixing the exact problem, IPCB(skb)->opt needing initilization. Thanks. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] gre: Avoid kernel panic by clearing IPCB before dst_link_failure called 2016-02-17 21:31 ` David Miller @ 2016-02-22 4:57 ` Bernie Harris 0 siblings, 0 replies; 9+ messages in thread From: Bernie Harris @ 2016-02-22 4:57 UTC (permalink / raw) To: David Miller Cc: netdev@vger.kernel.org, kuznet@ms2.inr.ac.ru, stable@vger.kernel.org > Again, I want to see this implemented in a way which causes things to be > treated consistently across all tunneling types. > > Which means fixing the exact problem, IPCB(skb)->opt needing initilization. > > Thanks. Thanks for the reply. I have revised my patch to apply to the range of tunnel types, and to only clear the opt field. ^ permalink raw reply [flat|nested] 9+ messages in thread
* (no subject) 2016-02-16 1:10 ` [PATCH v2] gre: Avoid kernel panic by clearing IPCB before " Bernie Harris 2016-02-17 21:31 ` David Miller @ 2016-02-21 23:24 ` Bernie Harris 2016-02-21 23:24 ` [PATCH v3] tunnel: Clear IPCB(skb)->opt before dst_link_failure called Bernie Harris 2016-02-21 23:58 ` Bernie Harris 2 siblings, 1 reply; 9+ messages in thread From: Bernie Harris @ 2016-02-21 23:24 UTC (permalink / raw) To: netdev; +Cc: davem, kuznet, stable, bernie.harris Thank you for the reply. I have revised the patch to apply to the range of tunnel types, and so only the opt field is cleared. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3] tunnel: Clear IPCB(skb)->opt before dst_link_failure called 2016-02-21 23:24 ` Bernie Harris @ 2016-02-21 23:24 ` Bernie Harris 0 siblings, 0 replies; 9+ messages in thread From: Bernie Harris @ 2016-02-21 23:24 UTC (permalink / raw) To: netdev; +Cc: davem, kuznet, stable, bernie.harris IPCB may contain data from previous layers (in the observed case the qdisc layer). In the observed scenario, the data was misinterpreted as ip header options, which later caused the ihl to be set to an invalid value (<5). This resulted in an infinite loop in the mips implementation of ip_fast_csum. This patch clears IPCB(skb)->opt before dst_link_failure can be called for various types of tunnels. This change only applies to encapsulated ipv4 packets. The code introduced in 11c21a30 which clears all of IPCB has been removed to be consistent with these changes, and instead the opt field is cleared unconditionally in ip_tunnel_xmit. The change in ip_tunnel_xmit applies to SIT, GRE, and IPIP tunnels. The relevant vti, l2tp, and pptp functions already contain similar code for clearing the IPCB. Signed-off-by: Bernie Harris <bernie.harris@alliedtelesis.co.nz> --- net/ipv4/ip_tunnel.c | 3 ++- net/ipv4/udp_tunnel.c | 2 ++ net/ipv6/ip6_gre.c | 2 ++ net/ipv6/ip6_tunnel.c | 2 ++ 4 files changed, 8 insertions(+), 1 deletion(-) diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c index 89e8861..336e689 100644 --- a/net/ipv4/ip_tunnel.c +++ b/net/ipv4/ip_tunnel.c @@ -661,6 +661,8 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev, inner_iph = (const struct iphdr *)skb_inner_network_header(skb); connected = (tunnel->parms.iph.daddr != 0); + memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt)); + dst = tnl_params->daddr; if (dst == 0) { /* NBMA tunnel */ @@ -758,7 +760,6 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev, tunnel->err_time + IPTUNNEL_ERR_TIMEO)) { tunnel->err_count--; - memset(IPCB(skb), 0, sizeof(*IPCB(skb))); dst_link_failure(skb); } else tunnel->err_count = 0; diff --git a/net/ipv4/udp_tunnel.c b/net/ipv4/udp_tunnel.c index 0ec0881..96599d1 100644 --- a/net/ipv4/udp_tunnel.c +++ b/net/ipv4/udp_tunnel.c @@ -89,6 +89,8 @@ void udp_tunnel_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *skb uh->source = src_port; uh->len = htons(skb->len); + memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt)); + udp_set_csum(nocheck, skb, src, dst, skb->len); iptunnel_xmit(sk, rt, skb, src, dst, IPPROTO_UDP, tos, ttl, df, xnet); diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c index f37f18b..c4123aa 100644 --- a/net/ipv6/ip6_gre.c +++ b/net/ipv6/ip6_gre.c @@ -777,6 +777,8 @@ static inline int ip6gre_xmit_ipv4(struct sk_buff *skb, struct net_device *dev) __u32 mtu; int err; + memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt)); + if (!(t->parms.flags & IP6_TNL_F_IGN_ENCAP_LIMIT)) encap_limit = t->parms.encap_limit; diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c index 137fca4..6c5dfec 100644 --- a/net/ipv6/ip6_tunnel.c +++ b/net/ipv6/ip6_tunnel.c @@ -1180,6 +1180,8 @@ ip4ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev) u8 tproto; int err; + memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt)); + tproto = ACCESS_ONCE(t->parms.proto); if (tproto != IPPROTO_IPIP && tproto != 0) return -1; -- 2.7.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v3] tunnel: Clear IPCB(skb)->opt before dst_link_failure called 2016-02-16 1:10 ` [PATCH v2] gre: Avoid kernel panic by clearing IPCB before " Bernie Harris 2016-02-17 21:31 ` David Miller 2016-02-21 23:24 ` Bernie Harris @ 2016-02-21 23:58 ` Bernie Harris 2016-02-24 0:12 ` David Miller 2 siblings, 1 reply; 9+ messages in thread From: Bernie Harris @ 2016-02-21 23:58 UTC (permalink / raw) To: netdev; +Cc: davem, kuznet, stable, bernie.harris IPCB may contain data from previous layers (in the observed case the qdisc layer). In the observed scenario, the data was misinterpreted as ip header options, which later caused the ihl to be set to an invalid value (<5). This resulted in an infinite loop in the mips implementation of ip_fast_csum. This patch clears IPCB(skb)->opt before dst_link_failure can be called for various types of tunnels. This change only applies to encapsulated ipv4 packets. The code introduced in 11c21a30 which clears all of IPCB has been removed to be consistent with these changes, and instead the opt field is cleared unconditionally in ip_tunnel_xmit. The change in ip_tunnel_xmit applies to SIT, GRE, and IPIP tunnels. The relevant vti, l2tp, and pptp functions already contain similar code for clearing the IPCB. Signed-off-by: Bernie Harris <bernie.harris@alliedtelesis.co.nz> --- net/ipv4/ip_tunnel.c | 3 ++- net/ipv4/udp_tunnel.c | 2 ++ net/ipv6/ip6_gre.c | 2 ++ net/ipv6/ip6_tunnel.c | 2 ++ 4 files changed, 8 insertions(+), 1 deletion(-) diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c index 89e8861..336e689 100644 --- a/net/ipv4/ip_tunnel.c +++ b/net/ipv4/ip_tunnel.c @@ -661,6 +661,8 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev, inner_iph = (const struct iphdr *)skb_inner_network_header(skb); connected = (tunnel->parms.iph.daddr != 0); + memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt)); + dst = tnl_params->daddr; if (dst == 0) { /* NBMA tunnel */ @@ -758,7 +760,6 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev, tunnel->err_time + IPTUNNEL_ERR_TIMEO)) { tunnel->err_count--; - memset(IPCB(skb), 0, sizeof(*IPCB(skb))); dst_link_failure(skb); } else tunnel->err_count = 0; diff --git a/net/ipv4/udp_tunnel.c b/net/ipv4/udp_tunnel.c index 0ec0881..96599d1 100644 --- a/net/ipv4/udp_tunnel.c +++ b/net/ipv4/udp_tunnel.c @@ -89,6 +89,8 @@ void udp_tunnel_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *skb uh->source = src_port; uh->len = htons(skb->len); + memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt)); + udp_set_csum(nocheck, skb, src, dst, skb->len); iptunnel_xmit(sk, rt, skb, src, dst, IPPROTO_UDP, tos, ttl, df, xnet); diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c index f37f18b..c4123aa 100644 --- a/net/ipv6/ip6_gre.c +++ b/net/ipv6/ip6_gre.c @@ -777,6 +777,8 @@ static inline int ip6gre_xmit_ipv4(struct sk_buff *skb, struct net_device *dev) __u32 mtu; int err; + memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt)); + if (!(t->parms.flags & IP6_TNL_F_IGN_ENCAP_LIMIT)) encap_limit = t->parms.encap_limit; diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c index 137fca4..6c5dfec 100644 --- a/net/ipv6/ip6_tunnel.c +++ b/net/ipv6/ip6_tunnel.c @@ -1180,6 +1180,8 @@ ip4ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev) u8 tproto; int err; + memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt)); + tproto = ACCESS_ONCE(t->parms.proto); if (tproto != IPPROTO_IPIP && tproto != 0) return -1; -- 2.7.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3] tunnel: Clear IPCB(skb)->opt before dst_link_failure called 2016-02-21 23:58 ` Bernie Harris @ 2016-02-24 0:12 ` David Miller 0 siblings, 0 replies; 9+ messages in thread From: David Miller @ 2016-02-24 0:12 UTC (permalink / raw) To: bernie.harris; +Cc: netdev, kuznet, stable From: Bernie Harris <bernie.harris@alliedtelesis.co.nz> Date: Mon, 22 Feb 2016 12:58:05 +1300 > IPCB may contain data from previous layers (in the observed case the > qdisc layer). In the observed scenario, the data was misinterpreted as > ip header options, which later caused the ihl to be set to an invalid > value (<5). This resulted in an infinite loop in the mips implementation > of ip_fast_csum. > > This patch clears IPCB(skb)->opt before dst_link_failure can be called for > various types of tunnels. This change only applies to encapsulated ipv4 > packets. > > The code introduced in 11c21a30 which clears all of IPCB has been removed > to be consistent with these changes, and instead the opt field is cleared > unconditionally in ip_tunnel_xmit. The change in ip_tunnel_xmit applies to > SIT, GRE, and IPIP tunnels. > > The relevant vti, l2tp, and pptp functions already contain similar code for > clearing the IPCB. > > Signed-off-by: Bernie Harris <bernie.harris@alliedtelesis.co.nz> Applied and queued up for -stable, thanks! ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ip6_gre: clear IPCB in ip6gre_xmit2 in case dst_link_failure called 2016-02-09 4:07 [PATCH] ip6_gre: clear IPCB in ip6gre_xmit2 in case dst_link_failure called Bernie Harris 2016-02-16 1:10 ` [PATCH v2] gre: Avoid kernel panic by clearing IPCB before " Bernie Harris @ 2016-02-17 15:26 ` David Miller 1 sibling, 0 replies; 9+ messages in thread From: David Miller @ 2016-02-17 15:26 UTC (permalink / raw) To: bernie.harris; +Cc: netdev, kuznet, stable From: Bernie Harris <bernie.harris@alliedtelesis.co.nz> Date: Tue, 9 Feb 2016 17:07:54 +1300 > @@ -678,6 +678,7 @@ static netdev_tx_t ip6gre_xmit2(struct sk_buff *skb, > tunnel->err_time + IP6TUNNEL_ERR_TIMEO)) { > tunnel->err_count--; > > + memset(IPCB(skb), 0, sizeof(*IPCB(skb))); > dst_link_failure(skb); > } else > tunnel->err_count = 0; > -- > 2.7.1 > I have a hard time accepting this because I see no other place in ipv6 tunnel handling where we have to handle this kind of case. If anything, this code should mimick the code in ip6_udp_tunnel6_xmit_skb() which clears only the IPCH(skb)->opt field, and does so unconditionally. If this happens for GRE ipv6 tunnels, it potentially could happen for any tunnel. Therefore we should apply consistent handling of this problem to every ipv6 tunnel implementation rather than just one spot. Thanks. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-02-24 0:12 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-02-09 4:07 [PATCH] ip6_gre: clear IPCB in ip6gre_xmit2 in case dst_link_failure called Bernie Harris 2016-02-16 1:10 ` [PATCH v2] gre: Avoid kernel panic by clearing IPCB before " Bernie Harris 2016-02-17 21:31 ` David Miller 2016-02-22 4:57 ` Bernie Harris 2016-02-21 23:24 ` Bernie Harris 2016-02-21 23:24 ` [PATCH v3] tunnel: Clear IPCB(skb)->opt before dst_link_failure called Bernie Harris 2016-02-21 23:58 ` Bernie Harris 2016-02-24 0:12 ` David Miller 2016-02-17 15:26 ` [PATCH] ip6_gre: clear IPCB in ip6gre_xmit2 in case " David Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).