linux-kernel - ipv6: Plug sk_buff leak in ipv6_rcv (net/ipv6/ip6

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090326123853.GW31562@axis.com>
Date:	Thu, 26 Mar 2009 13:38:53 +0100
From:	Jesper Nilsson <jesper.nilsson@...s.com>
To:	YOSHIFUJI Hideaki <yoshfuji@...ux-ipv6.org>,
	netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:	Mikael Starvik <mikael.starvik@...s.com>
Subject: ipv6: Plug sk_buff leak in ipv6_rcv (net/ipv6/ip6_input.c)

Hi,

Commit 778d80be52699596bf70e0eb0761cf5e1e46088d
(ipv6: Add disable_ipv6 sysctl to disable IPv6 operaion on specific interface)
seems to have introduced a leak of sk_buff's for ipv6 traffic,
at least in some configurations where idev is NULL, or when ipv6
is disabled via sysctl.

The problem is that if the first condition of the if-statement
returns non-NULL, it returns an skb with only one reference,
and when the other conditions apply, execution jumps to the "out"
label, which does not call kfree_skb for it.

To plug this leak, change to use the "drop" label instead.
(this relies on it being ok to call kfree_skb on NULL)
This also allows us to avoid calling rcu_read_unlock here,
and removes the only user of the "out" label.

Signed-off-by: Jesper Nilsson <jesper.nilsson@...s.com>

---
 ip6_input.c |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index f171e8d..8f04bd9 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -75,8 +75,7 @@ int ipv6_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt
 	if ((skb = skb_share_check(skb, GFP_ATOMIC)) == NULL ||
 	    !idev || unlikely(idev->cnf.disable_ipv6)) {
 		IP6_INC_STATS_BH(net, idev, IPSTATS_MIB_INDISCARDS);
-		rcu_read_unlock();
-		goto out;
+		goto drop;
 	}

 	memset(IP6CB(skb), 0, sizeof(struct inet6_skb_parm));
@@ -147,7 +146,6 @@ err:
 drop:
 	rcu_read_unlock();
 	kfree_skb(skb);
-out:
 	return 0;
 }

/^JN - Jesper Nilsson
-- 
               Jesper Nilsson -- jesper.nilsson@...s.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/