David S. Miller wrote:
From: Patrick McHardy <[EMAIL PROTECTED]>
Date: Thu, 08 Sep 2005 23:00:39 +0200
When an error is returned in some cases xfrm_lookup releases
the dst_entry and sets the pointer to NULL, in some it doesn't.
There are multiple dst leaks in users of xfrm_lookup because
not all take care to release the dst_entry when an error was
returned. Is there a reason why xfrm_lookup doesn't always
release the dst_entry?
It's a known bug and even listed on the netdev TODO list
at:
http://vger.kernel.org/~davem/net_todo.html
:-)
In that case, here is a patch to always release the dst entry
on error and set the pointer to NULL. It also removes now
unnecessary dst_release calls in users of xfrm_lookup.
[XFRM]: Always release dst_entry on error in xfrm_lookup
Signed-off-by: Patrick McHardy <[EMAIL PROTECTED]>
---
commit 87c9b944bfcd3a536bc85760974a2eff5336ed28
tree dea19208ef2a38ba78cbe65684ddef07e7ead24b
parent 975f5749e0962c59b9b8f93abee8adf3e2bc292d
author Patrick McHardy <[EMAIL PROTECTED]> Thu, 08 Sep 2005 23:43:34 +0200
committer Patrick McHardy <[EMAIL PROTECTED]> Thu, 08 Sep 2005 23:43:34 +0200
net/ipv4/netfilter/ipt_REJECT.c | 5 +----
net/ipv6/datagram.c | 4 +---
net/ipv6/icmp.c | 5 ++---
net/ipv6/ndisc.c | 16 ++++------------
net/ipv6/netfilter/ip6t_REJECT.c | 5 +----
net/ipv6/raw.c | 4 +---
net/ipv6/tcp_ipv6.c | 15 +++------------
net/ipv6/udp.c | 4 +---
net/xfrm/xfrm_policy.c | 8 ++++----
9 files changed, 18 insertions(+), 48 deletions(-)
diff --git a/net/ipv4/netfilter/ipt_REJECT.c b/net/ipv4/netfilter/ipt_REJECT.c
--- a/net/ipv4/netfilter/ipt_REJECT.c
+++ b/net/ipv4/netfilter/ipt_REJECT.c
@@ -92,10 +92,7 @@ static inline struct rtable *route_rever
fl.fl_ip_sport = tcph->dest;
fl.fl_ip_dport = tcph->source;
- if (xfrm_lookup((struct dst_entry **)&rt, &fl, NULL, 0)) {
- dst_release(&rt->u.dst);
- rt = NULL;
- }
+ xfrm_lookup((struct dst_entry **)&rt, &fl, NULL, 0);
return rt;
}
diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -175,10 +175,8 @@ ipv4_connected:
if (final_p)
ipv6_addr_copy(&fl.fl6_dst, final_p);
- if ((err = xfrm_lookup(&dst, &fl, sk, 0)) < 0) {
- dst_release(dst);
+ if ((err = xfrm_lookup(&dst, &fl, sk, 0)) < 0)
goto out;
- }
/* source address lookup done in ip6_dst_lookup */
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -374,7 +374,7 @@ void icmpv6_send(struct sk_buff *skb, in
if (err)
goto out;
if ((err = xfrm_lookup(&dst, &fl, sk, 0)) < 0)
- goto out_dst_release;
+ goto out;
if (ipv6_addr_is_multicast(&fl.fl6_dst))
hlimit = np->mcast_hops;
@@ -464,7 +464,7 @@ static void icmpv6_echo_reply(struct sk_
if (err)
goto out;
if ((err = xfrm_lookup(&dst, &fl, sk, 0)) < 0)
- goto out_dst_release;
+ goto out;
if (ipv6_addr_is_multicast(&fl.fl6_dst))
hlimit = np->mcast_hops;
@@ -496,7 +496,6 @@ static void icmpv6_echo_reply(struct sk_
out_put:
if (likely(idev != NULL))
in6_dev_put(idev);
-out_dst_release:
dst_release(dst);
out:
icmpv6_xmit_unlock();
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -447,10 +447,8 @@ static void ndisc_send_na(struct net_dev
return;
err = xfrm_lookup(&dst, &fl, NULL, 0);
- if (err < 0) {
- dst_release(dst);
+ if (err < 0)
return;
- }
if (inc_opt) {
if (dev->addr_len)
@@ -539,10 +537,8 @@ void ndisc_send_ns(struct net_device *de
return;
err = xfrm_lookup(&dst, &fl, NULL, 0);
- if (err < 0) {
- dst_release(dst);
+ if (err < 0)
return;
- }
len = sizeof(struct icmp6hdr) + sizeof(struct in6_addr);
send_llinfo = dev->addr_len && !ipv6_addr_any(saddr);
@@ -616,10 +612,8 @@ void ndisc_send_rs(struct net_device *de
return;
err = xfrm_lookup(&dst, &fl, NULL, 0);
- if (err < 0) {
- dst_release(dst);
+ if (err < 0)
return;
- }
len = sizeof(struct icmp6hdr);
if (dev->addr_len)
@@ -1353,10 +1347,8 @@ void ndisc_send_redirect(struct sk_buff
return;
err = xfrm_lookup(&dst, &fl, NULL, 0);
- if (err) {
- dst_release(dst);
+ if (err)
return;
- }
rt = (struct rt6_info *) dst;
diff --git a/net/ipv6/netfilter/ip6t_REJECT.c b/net/ipv6/netfilter/ip6t_REJECT.c
--- a/net/ipv6/netfilter/ip6t_REJECT.c
+++ b/net/ipv6/netfilter/ip6t_REJECT.c
@@ -100,11 +100,8 @@ static void send_reset(struct sk_buff *o
dst = ip6_route_output(NULL, &fl);
if (dst == NULL)
return;
- if (dst->error ||
- xfrm_lookup(&dst, &fl, NULL, 0)) {
- dst_release(dst);
+ if (dst->error || xfrm_lookup(&dst, &fl, NULL, 0))
return;
- }
hh_len = (dst->dev->hard_header_len + 15)&~15;
nskb = alloc_skb(hh_len + 15 + dst->header_len + sizeof(struct ipv6hdr)
diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -782,10 +782,8 @@ static int rawv6_sendmsg(struct kiocb *i
if (final_p)
ipv6_addr_copy(&fl.fl6_dst, final_p);
- if ((err = xfrm_lookup(&dst, &fl, sk, 0)) < 0) {
- dst_release(dst);
+ if ((err = xfrm_lookup(&dst, &fl, sk, 0)) < 0)
goto out;
- }
if (hlimit < 0) {
if (ipv6_addr_is_multicast(&fl.fl6_dst))
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -632,10 +632,8 @@ static int tcp_v6_connect(struct sock *s
if (final_p)
ipv6_addr_copy(&fl.fl6_dst, final_p);
- if ((err = xfrm_lookup(&dst, &fl, sk, 0)) < 0) {
- dst_release(dst);
+ if ((err = xfrm_lookup(&dst, &fl, sk, 0)) < 0)
goto failure;
- }
if (saddr == NULL) {
saddr = &fl.fl6_src;
@@ -888,7 +886,6 @@ static int tcp_v6_send_synack(struct soc
}
done:
- dst_release(dst);
if (opt && opt != np->opt)
sock_kfree_s(sk, opt, opt->tot_len);
return err;
@@ -1001,10 +998,8 @@ static void tcp_v6_send_reset(struct sk_
/* sk = NULL, but it is safe for now. RST socket required. */
if (!ip6_dst_lookup(NULL, &buff->dst, &fl)) {
- if ((xfrm_lookup(&buff->dst, &fl, NULL, 0)) < 0) {
- dst_release(buff->dst);
+ if ((xfrm_lookup(&buff->dst, &fl, NULL, 0)) < 0)
return;
- }
ip6_xmit(NULL, buff, &fl, NULL, 0);
TCP_INC_STATS_BH(TCP_MIB_OUTSEGS);
@@ -1068,10 +1063,8 @@ static void tcp_v6_send_ack(struct sk_bu
fl.fl_ip_sport = t1->source;
if (!ip6_dst_lookup(NULL, &buff->dst, &fl)) {
- if ((xfrm_lookup(&buff->dst, &fl, NULL, 0)) < 0) {
- dst_release(buff->dst);
+ if ((xfrm_lookup(&buff->dst, &fl, NULL, 0)) < 0)
return;
- }
ip6_xmit(NULL, buff, &fl, NULL, 0);
TCP_INC_STATS_BH(TCP_MIB_OUTSEGS);
return;
@@ -1734,7 +1727,6 @@ static int tcp_v6_rebuild_header(struct
if ((err = xfrm_lookup(&dst, &fl, sk, 0)) < 0) {
sk->sk_err_soft = -err;
- dst_release(dst);
return err;
}
@@ -1787,7 +1779,6 @@ static int tcp_v6_xmit(struct sk_buff *s
if ((err = xfrm_lookup(&dst, &fl, sk, 0)) < 0) {
sk->sk_route_caps = 0;
- dst_release(dst);
return err;
}
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -799,10 +799,8 @@ do_udp_sendmsg:
if (final_p)
ipv6_addr_copy(&fl->fl6_dst, final_p);
- if ((err = xfrm_lookup(&dst, fl, sk, 0)) < 0) {
- dst_release(dst);
+ if ((err = xfrm_lookup(&dst, fl, sk, 0)) < 0)
goto out;
- }
if (hlimit < 0) {
if (ipv6_addr_is_multicast(&fl->fl6_dst))
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -765,8 +765,8 @@ restart:
switch (policy->action) {
case XFRM_POLICY_BLOCK:
/* Prohibit the flow */
- xfrm_pol_put(policy);
- return -EPERM;
+ err = -EPERM;
+ goto error;
case XFRM_POLICY_ALLOW:
if (policy->xfrm_nr == 0) {
@@ -782,8 +782,8 @@ restart:
*/
dst = xfrm_find_bundle(fl, policy, family);
if (IS_ERR(dst)) {
- xfrm_pol_put(policy);
- return PTR_ERR(dst);
+ err = PTR_ERR(dst);
+ goto error;
}
if (dst)
