From 7c0ecc4c4f8fd90988aab8a95297b9c0038b6160 Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Wed, 26 Mar 2008 02:27:09 -0700 Subject: [ICMP]: Dst entry leak in icmp_send host re-lookup code (v2). Commit 8b7817f3a959ed99d7443afc12f78a7e1fcc2063 ([IPSEC]: Add ICMP host relookup support) introduced some dst leaks on error paths: the rt pointer can be forgotten to be put. Fix it bu going to a proper label. Found after net namespace's lo refused to unregister :) Many thanks to Den for valuable help during debugging. Herbert pointed out, that xfrm_lookup() will put the rtable in case of error itself, so the first goto fix is redundant. Signed-off-by: Pavel Emelyanov Signed-off-by: Denis V. Lunev Signed-off-by: David S. Miller --- net/ipv4/icmp.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'net/ipv4/icmp.c') diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c index a13c074dac09..a944e8053e28 100644 --- a/net/ipv4/icmp.c +++ b/net/ipv4/icmp.c @@ -591,7 +591,7 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info) } if (xfrm_decode_session_reverse(skb_in, &fl, AF_INET)) - goto out_unlock; + goto ende; if (inet_addr_type(net, fl.fl4_src) == RTN_LOCAL) err = __ip_route_output_key(net, &rt2, &fl); @@ -601,7 +601,7 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info) fl2.fl4_dst = fl.fl4_src; if (ip_route_output_key(net, &rt2, &fl2)) - goto out_unlock; + goto ende; /* Ugh! */ odst = skb_in->dst; @@ -614,7 +614,7 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info) } if (err) - goto out_unlock; + goto ende; err = xfrm_lookup((struct dst_entry **)&rt2, &fl, NULL, XFRM_LOOKUP_ICMP); -- cgit v1.2.2 From af2681828af5f2b42e12e8b16ba0cf113cf486c8 Mon Sep 17 00:00:00 2001 From: Herbert Xu Date: Thu, 3 Apr 2008 12:52:19 -0700 Subject: [ICMP]: Ensure that ICMP relookup maintains status quo The ICMP relookup path is only meant to modify behaviour when appropriate IPsec policies are in place and marked as requiring relookups. It is certainly not meant to modify behaviour when IPsec policies don't exist at all. However, due to an oversight on the error paths existing behaviour may in fact change should one of the relookup steps fail. This patch corrects this by redirecting all errors on relookup failures to the previous code path. That is, if the initial xfrm_lookup let the packet pass, we will stand by that decision should the relookup fail due to an error. This should be safe from a security point-of-view because compliant systems must install a default deny policy so the packet would'nt have passed in that case. Many thanks to Julian Anastasov for pointing out this error. Signed-off-by: Herbert Xu Signed-off-by: David S. Miller --- net/ipv4/icmp.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) (limited to 'net/ipv4/icmp.c') diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c index a944e8053e28..40508babad8c 100644 --- a/net/ipv4/icmp.c +++ b/net/ipv4/icmp.c @@ -591,7 +591,7 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info) } if (xfrm_decode_session_reverse(skb_in, &fl, AF_INET)) - goto ende; + goto relookup_failed; if (inet_addr_type(net, fl.fl4_src) == RTN_LOCAL) err = __ip_route_output_key(net, &rt2, &fl); @@ -601,7 +601,7 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info) fl2.fl4_dst = fl.fl4_src; if (ip_route_output_key(net, &rt2, &fl2)) - goto ende; + goto relookup_failed; /* Ugh! */ odst = skb_in->dst; @@ -614,21 +614,23 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info) } if (err) - goto ende; + goto relookup_failed; err = xfrm_lookup((struct dst_entry **)&rt2, &fl, NULL, XFRM_LOOKUP_ICMP); - if (err == -ENOENT) { + switch (err) { + case 0: + dst_release(&rt->u.dst); + rt = rt2; + break; + case -EPERM: + goto ende; + default: +relookup_failed: if (!rt) goto out_unlock; - goto route_done; + break; } - - dst_release(&rt->u.dst); - rt = rt2; - - if (err) - goto out_unlock; } route_done: -- cgit v1.2.2