Skip to contentSkip to conversation

pull request: ipsec 2013-01-22

PATCH New Steffen Klassert 2 participants 7 messages Jan 22, 2013

Subsystems (4)
Networking Network drivers IPv4/IPv6 Networking IPsec
lore
Share & tools
lore

Steffen Klassert

author maintainer
#0 lore raw

1) The transport header did not point to the right place after esp/ah processing on tunnel mode in the receive path. As a result, the ECN field of the inner header was not set correctly, fixes from Li RongQing.

2) We did a null check too late in one of the xfrm_replay advance functions. This can lead to a division by zero, fix from Nickolai Zeldovich.

3) The size calculation of the hash table missed the muiltplication with the actual struct size when the hash table is freed. We might call the wrong free function, fix from Michal Kubecek.

4) On IPsec pmtu events we can't access the transport headers of the original packet, so force a relookup for all routes to notify about the pmtu event.

Please pull or let me know if there are problems.

Thanks!

The following changes since commit c7e2e1d72ed7707239d20525e0ebcad7e3303659:

ipv4: fix NULL checking in devinet_ioctl() (2013-01-06 21:11:18 -0800)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec.git master

for you to fetch changes up to 05ab86c55683410593720003442dde629782aaac:

xfrm4: Invalidate all ipv4 routes on IPsec pmtu events (2013-01-21 12:43:54 +0100)

----------------------------------------------------------------
Li RongQing (2):
ah4/esp4: set transport header correctly for IPsec tunnel mode. ah6/esp6: set transport header correctly for IPsec tunnel mode.

Michal Kubecek (1):
xfrm: fix freed block size calculation in xfrm_policy_fini()

Nickolai Zeldovich (1):
net/xfrm/xfrm_replay: avoid division by zero

Steffen Klassert (1):
xfrm4: Invalidate all ipv4 routes on IPsec pmtu events

 net/ipv4/ah4.c         |   18 ++++++++++++++----
 net/ipv4/esp4.c        |   12 +++++++++---
 net/ipv4/ipcomp.c      |    7 +++++--
 net/ipv6/ah6.c         |   11 +++++++++--
 net/ipv6/esp6.c        |    5 ++++-
 net/xfrm/xfrm_policy.c |    2 +-
 net/xfrm/xfrm_replay.c |    4 +++-
 7 files changed, 45 insertions(+), 14 deletions(-)
author maintainer
#1 lore raw

PATCH 1/5 ah4/esp4: set transport header correctly for IPsec tunnel mode.

From: Li RongQing <[email protected]>

IPsec tunnel does not set ECN field to CE in inner header when the ECN field in the outer header is CE, and the ECN field in the inner header is ECT(0) or ECT(1).

The cause is ipip_hdr() does not return the correct address of inner header since skb->transport-header is not the inner header after esp_input_done2(), or ah_input().

Signed-off-by Li RongQing <[email protected]>
Signed-off-by Steffen Klassert <[email protected]>
---
 net/ipv4/ah4.c  |   11 +++++++++--
 net/ipv4/esp4.c |    5 ++++-
 2 files changed, 13 insertions(+), 3 deletions(-)
net/ipv4/ah4.c +9 -2
index a0d8392..a154d0a 100644
@@ -269,7 +269,11 @@ static void ah_input_done(struct crypto_async_request *base, int err)
 	skb->network_header += ah_hlen;
 	memcpy(skb_network_header(skb), work_iph, ihl);
 	__skb_pull(skb, ah_hlen + ihl);
-	skb_set_transport_header(skb, -ihl);
+
+	if (x->props.mode == XFRM_MODE_TUNNEL)
+		skb_reset_transport_header(skb);
+	else
+		skb_set_transport_header(skb, -ihl);
 out:
 	kfree(AH_SKB_CB(skb)->tmp);
 	xfrm_input_resume(skb, err);

@@ -381,7 +385,10 @@ static int ah_input(struct xfrm_state *x, struct sk_buff *skb)
 	skb->network_header += ah_hlen;
 	memcpy(skb_network_header(skb), work_iph, ihl);
 	__skb_pull(skb, ah_hlen + ihl);
-	skb_set_transport_header(skb, -ihl);
+	if (x->props.mode == XFRM_MODE_TUNNEL)
+		skb_reset_transport_header(skb);
+	else
+		skb_set_transport_header(skb, -ihl);
 
 	err = nexthdr;
net/ipv4/esp4.c +4 -1
index b61e9de..fd26ff4 100644
@@ -346,7 +346,10 @@ static int esp_input_done2(struct sk_buff *skb, int err)
 
 	pskb_trim(skb, skb->len - alen - padlen - 2);
 	__skb_pull(skb, hlen);
-	skb_set_transport_header(skb, -ihl);
+	if (x->props.mode == XFRM_MODE_TUNNEL)
+		skb_reset_transport_header(skb);
+	else
+		skb_set_transport_header(skb, -ihl);
 
 	err = nexthdr[1];
Signature · 1 line
1.7.9.5
author maintainer
#2 lore raw

PATCH 2/5 ah6/esp6: set transport header correctly for IPsec tunnel mode.

In reply to Steffen Klassert

From: Li RongQing <[email protected]>

IPsec tunnel does not set ECN field to CE in inner header when the ECN field in the outer header is CE, and the ECN field in the inner header is ECT(0) or ECT(1).

The cause is ipip6_hdr() does not return the correct address of inner header since skb->transport-header is not the inner header after esp6_input_done2(), or ah6_input().

Signed-off-by Li RongQing <[email protected]>
Signed-off-by Steffen Klassert <[email protected]>
---
 net/ipv6/ah6.c  |   11 +++++++++--
 net/ipv6/esp6.c |    5 ++++-
 2 files changed, 13 insertions(+), 3 deletions(-)
net/ipv6/ah6.c +9 -2
index ecc35b9..3842331 100644
@@ -472,7 +472,10 @@ static void ah6_input_done(struct crypto_async_request *base, int err)
 	skb->network_header += ah_hlen;
 	memcpy(skb_network_header(skb), work_iph, hdr_len);
 	__skb_pull(skb, ah_hlen + hdr_len);
-	skb_set_transport_header(skb, -hdr_len);
+	if (x->props.mode == XFRM_MODE_TUNNEL)
+		skb_reset_transport_header(skb);
+	else
+		skb_set_transport_header(skb, -hdr_len);
 out:
 	kfree(AH_SKB_CB(skb)->tmp);
 	xfrm_input_resume(skb, err);

@@ -593,9 +596,13 @@ static int ah6_input(struct xfrm_state *x, struct sk_buff *skb)
 
 	skb->network_header += ah_hlen;
 	memcpy(skb_network_header(skb), work_iph, hdr_len);
-	skb->transport_header = skb->network_header;
 	__skb_pull(skb, ah_hlen + hdr_len);
 
+	if (x->props.mode == XFRM_MODE_TUNNEL)
+		skb_reset_transport_header(skb);
+	else
+		skb_set_transport_header(skb, -hdr_len);
+
 	err = nexthdr;
 
 out_free:
net/ipv6/esp6.c +4 -1
index 282f372..40ffd72 100644
@@ -300,7 +300,10 @@ static int esp_input_done2(struct sk_buff *skb, int err)
 
 	pskb_trim(skb, skb->len - alen - padlen - 2);
 	__skb_pull(skb, hlen);
-	skb_set_transport_header(skb, -hdr_len);
+	if (x->props.mode == XFRM_MODE_TUNNEL)
+		skb_reset_transport_header(skb);
+	else
+		skb_set_transport_header(skb, -hdr_len);
 
 	err = nexthdr[1];
Signature · 1 line
1.7.9.5
author maintainer
#3 lore raw

PATCH 3/5 net/xfrm/xfrm_replay: avoid division by zero

In reply to Steffen Klassert

From: Nickolai Zeldovich <[email protected]>

All of the xfrm_replay->advance functions in xfrm_replay.c check if x->replay_esn->replay_window is zero (and return if so). However, one of them, xfrm_replay_advance_bmp(), divides by that value (in the '%' operator) before doing the check, which can potentially trigger a divide-by-zero exception. Some compilers will also assume that the earlier division means the value cannot be zero later, and thus will eliminate the subsequent zero check as dead code.

This patch moves the division to after the check.

Signed-off-by Nickolai Zeldovich <[email protected]>
Signed-off-by Steffen Klassert <[email protected]>
---
 net/xfrm/xfrm_replay.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
net/xfrm/xfrm_replay.c +3 -1
index 765f6fe..35754cc 100644
@@ -242,11 +242,13 @@ static void xfrm_replay_advance_bmp(struct xfrm_state *x, __be32 net_seq)
 	u32 diff;
 	struct xfrm_replay_state_esn *replay_esn = x->replay_esn;
 	u32 seq = ntohl(net_seq);
-	u32 pos = (replay_esn->seq - 1) % replay_esn->replay_window;
+	u32 pos;
 
 	if (!replay_esn->replay_window)
 		return;
 
+	pos = (replay_esn->seq - 1) % replay_esn->replay_window;
+
 	if (seq > replay_esn->seq) {
 		diff = seq - replay_esn->seq;
Signature · 1 line
1.7.9.5
author maintainer
#4 lore raw

PATCH 4/5 xfrm: fix freed block size calculation in xfrm_policy_fini()

In reply to Steffen Klassert

From: Michal Kubecek <[email protected]>

Missing multiplication of block size by sizeof(struct hlist_head) can cause xfrm_hash_free() to be called with wrong second argument so that kfree() is called on a block allocated with vzalloc() or __get_free_pages() or free_pages() is called with wrong order when a namespace with enough policies is removed.

Bug introduced by commit a35f6c5d, i.e. versions >= 2.6.29 are affected.

Signed-off-by Michal Kubecek <[email protected]>
Signed-off-by Steffen Klassert <[email protected]>
---
 net/xfrm/xfrm_policy.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
net/xfrm/xfrm_policy.c +1 -1
index 41eabc4..07c5857 100644
@@ -2656,7 +2656,7 @@ static void xfrm_policy_fini(struct net *net)
 		WARN_ON(!hlist_empty(&net->xfrm.policy_inexact[dir]));
 
 		htab = &net->xfrm.policy_bydst[dir];
-		sz = (htab->hmask + 1);
+		sz = (htab->hmask + 1) * sizeof(struct hlist_head);
 		WARN_ON(!hlist_empty(htab->table));
 		xfrm_hash_free(htab->table, sz);
 	}
Signature · 1 line
1.7.9.5
author maintainer
#5 lore raw

PATCH 5/5 xfrm4: Invalidate all ipv4 routes on IPsec pmtu events

In reply to Steffen Klassert

On IPsec pmtu events we can't access the transport headers of the original packet, so we can't find the socket that sent the packet. The only chance to notify the socket about the pmtu change is to force a relookup for all routes. This patch implenents this for the IPsec protocols.

Signed-off-by Steffen Klassert <[email protected]>
---
 net/ipv4/ah4.c    |    7 +++++--
 net/ipv4/esp4.c   |    7 +++++--
 net/ipv4/ipcomp.c |    7 +++++--
 3 files changed, 15 insertions(+), 6 deletions(-)
net/ipv4/ah4.c +5 -2
index a154d0a..a69b4e4 100644
@@ -420,9 +420,12 @@ static void ah4_err(struct sk_buff *skb, u32 info)
 	if (!x)
 		return;
 
-	if (icmp_hdr(skb)->type == ICMP_DEST_UNREACH)
+	if (icmp_hdr(skb)->type == ICMP_DEST_UNREACH) {
+		atomic_inc(&flow_cache_genid);
+		rt_genid_bump(net);
+
 		ipv4_update_pmtu(skb, net, info, 0, 0, IPPROTO_AH, 0);
-	else
+	} else
 		ipv4_redirect(skb, net, 0, 0, IPPROTO_AH, 0);
 	xfrm_state_put(x);
 }
net/ipv4/esp4.c +5 -2
index fd26ff4..3b4f0cd 100644
@@ -502,9 +502,12 @@ static void esp4_err(struct sk_buff *skb, u32 info)
 	if (!x)
 		return;
 
-	if (icmp_hdr(skb)->type == ICMP_DEST_UNREACH)
+	if (icmp_hdr(skb)->type == ICMP_DEST_UNREACH) {
+		atomic_inc(&flow_cache_genid);
+		rt_genid_bump(net);
+
 		ipv4_update_pmtu(skb, net, info, 0, 0, IPPROTO_ESP, 0);
-	else
+	} else
 		ipv4_redirect(skb, net, 0, 0, IPPROTO_ESP, 0);
 	xfrm_state_put(x);
 }
net/ipv4/ipcomp.c +5 -2
index d3ab47e..9a46dae 100644
@@ -47,9 +47,12 @@ static void ipcomp4_err(struct sk_buff *skb, u32 info)
 	if (!x)
 		return;
 
-	if (icmp_hdr(skb)->type == ICMP_DEST_UNREACH)
+	if (icmp_hdr(skb)->type == ICMP_DEST_UNREACH) {
+		atomic_inc(&flow_cache_genid);
+		rt_genid_bump(net);
+
 		ipv4_update_pmtu(skb, net, info, 0, 0, IPPROTO_COMP, 0);
-	else
+	} else
 		ipv4_redirect(skb, net, 0, 0, IPPROTO_COMP, 0);
 	xfrm_state_put(x);
 }
Signature · 1 line
1.7.9.5
maintainer
#6 lore raw
In reply to Steffen Klassert

From: Steffen Klassert <[email protected]>
Date: Tue, 22 Jan 2013 10:06:31 +0100

Quoted 13 lines

1) The transport header did not point to the right place after esp/ah processing on tunnel mode in the receive path. As a result, the ECN field of the inner header was not set correctly, fixes from Li RongQing.

2) We did a null check too late in one of the xfrm_replay advance functions. This can lead to a division by zero, fix from Nickolai Zeldovich.

3) The size calculation of the hash table missed the muiltplication with the actual struct size when the hash table is freed. We might call the wrong free function, fix from Michal Kubecek.

Quoted 5 lines

4) On IPsec pmtu events we can't access the transport headers of the original packet, so force a relookup for all routes to notify about the pmtu event.

Please pull or let me know if there are problems.

Pulled, thakns a lot.

Please be explicit in the future about what tree a set of changes are targetted at.

Thanks.