netdev - Re: [PATCH 1/5] inet: Hide route peer accesses behind helpers.

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-prev] [day] [month] [year] [list]

Message-Id: <20120611.041910.1087693152444897262.davem@davemloft.net>
Date:	Mon, 11 Jun 2012 04:19:10 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	eric.dumazet@...il.com
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCH 1/5] inet: Hide route peer accesses behind helpers.

From: Eric Dumazet <eric.dumazet@...il.com>
Date: Mon, 11 Jun 2012 12:51:02 +0200

> From: Eric Dumazet <edumazet@...gle.com>
> 
> On Mon, 2012-06-11 at 02:29 -0700, David Miller wrote:
>> +static inline bool inetpeer_ptr_set_peer(unsigned long *ptr, struct inet_peer *peer)
>> +{
>> +	unsigned long val = (unsigned long) peer;
>> +	unsigned long orig = *ptr;
>> +
>> +	if (!(orig & INETPEER_BASE_BIT) || !val ||
>> +	    cmpxchg(ptr, orig, val) != orig)
>> +		return false;
>> +	return true;
>> +}
> 
> If peer is NULL here, we return false;

Good catch.

--------------------
inet: Avoid potential NULL peer dereference.

We handle NULL in rt{,6}_set_peer but then our caller will try to pass
that NULL pointer into inet_putpeer() which isn't ready for it.

Fix this by moving the NULL check one level up, and then remove the
now unnecessary NULL check from inetpeer_ptr_set_peer().

Reported-by: Eric Dumazet <eric.dumazet@...il.com>
Signed-off-by: David S. Miller <davem@...emloft.net>
---
 include/net/inetpeer.h |    2 +-
 net/ipv4/route.c       |   11 ++++++-----
 net/ipv6/route.c       |   10 ++++++----
 3 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/include/net/inetpeer.h b/include/net/inetpeer.h
index e15c086..c27c8f1 100644
--- a/include/net/inetpeer.h
+++ b/include/net/inetpeer.h
@@ -104,7 +104,7 @@ static inline bool inetpeer_ptr_set_peer(unsigned long *ptr, struct inet_peer *p
 	unsigned long val = (unsigned long) peer;
 	unsigned long orig = *ptr;
 
-	if (!(orig & INETPEER_BASE_BIT) || !val ||
+	if (!(orig & INETPEER_BASE_BIT) ||
 	    cmpxchg(ptr, orig, val) != orig)
 		return false;
 	return true;
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 4c33ce3..842510d 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1333,11 +1333,12 @@ void rt_bind_peer(struct rtable *rt, __be32 daddr, int create)
 		return;
 
 	peer = inet_getpeer_v4(base, daddr, create);
-
-	if (!rt_set_peer(rt, peer))
-		inet_putpeer(peer);
-	else
-		rt->rt_peer_genid = rt_peer_genid();
+	if (peer) {
+		if (!rt_set_peer(rt, peer))
+			inet_putpeer(peer);
+		else
+			rt->rt_peer_genid = rt_peer_genid();
+	}
 }
 
 /*
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index d9ba480..58a3ec2 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -313,10 +313,12 @@ void rt6_bind_peer(struct rt6_info *rt, int create)
 		return;
 
 	peer = inet_getpeer_v6(base, &rt->rt6i_dst.addr, create);
-	if (!rt6_set_peer(rt, peer))
-		inet_putpeer(peer);
-	else
-		rt->rt6i_peer_genid = rt6_peer_genid();
+	if (peer) {
+		if (!rt6_set_peer(rt, peer))
+			inet_putpeer(peer);
+		else
+			rt->rt6i_peer_genid = rt6_peer_genid();
+	}
 }
 
 static void ip6_dst_ifdown(struct dst_entry *dst, struct net_device *dev,
-- 
1.7.10

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html