[PATCH 1/2] IPv6: stricter checks on link-locals in bind and sendmsg

[PATCH 1/2] IPv6: stricter checks on link-locals in bind and sendmsg

[Ville Nuorvala]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 695 bytes --]

Hi Dave,

I found a couple of suspected bugs related to (source) address
handling while rummaging around the code...

When binding to a link-local address, inet6_bind() and raw6_bind() only
check that an interface is specified and that the address exists, but
they don't check if it actually exists on the specified interface.

Similarly, in datagram_sent_ctl() we don't check for the possibility of a
link-local address when we receive the source address from userspace.

The attached patch should fix this behavior.

Thanks,
Ville
--
Ville Nuorvala
Research Assistant, Institute of Digital Communications,
Helsinki University of Technology
email: vnuorval@tcs.hut.fi, phone: +358 (0)9 451 5257

[-- Attachment #2: Type: TEXT/PLAIN, Size: 7507 bytes --]

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.1511  -> 1.1513 
#	 net/ipv6/datagram.c	1.11    -> 1.12   
#	      net/ipv6/raw.c	1.45    -> 1.46   
#	 net/ipv6/af_inet6.c	1.57    -> 1.58   
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 04/01/13	vnuorval@cs78227090.pp.htv.fi	1.1512
# When binding to a link-local address, check that it actually exists on the specified interface.
# --------------------------------------------
# 04/01/13	vnuorval@amber.hut.mediapoli.com	1.1513
# When sending a datagram using a link-local source address, check that it actually exists on the
# specified interface.
# --------------------------------------------
#
diff -Nru a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
--- a/net/ipv6/af_inet6.c	Wed Jan 14 00:14:51 2004
+++ b/net/ipv6/af_inet6.c	Wed Jan 14 00:14:51 2004
@@ -294,6 +294,7 @@
 	__u32 v4addr = 0;
 	unsigned short snum;
 	int addr_type = 0;
+	int err = 0;
 
 	/* If the socket has its own bind function then use it. */
 	if (sk->sk_prot->bind)
@@ -305,24 +306,6 @@
 	if ((addr_type & IPV6_ADDR_MULTICAST) && sock->type == SOCK_STREAM)
 		return -EINVAL;
 
-	/* Check if the address belongs to the host. */
-	if (addr_type == IPV6_ADDR_MAPPED) {
-		v4addr = addr->sin6_addr.s6_addr32[3];
-		if (inet_addr_type(v4addr) != RTN_LOCAL)
-			return -EADDRNOTAVAIL;
-	} else {
-		if (addr_type != IPV6_ADDR_ANY) {
-			/* ipv4 addr of the socket is invalid.  Only the
-			 * unspecified and mapped address have a v4 equivalent.
-			 */
-			v4addr = LOOPBACK4_IPV6;
-			if (!(addr_type & IPV6_ADDR_MULTICAST))	{
-				if (!ipv6_chk_addr(&addr->sin6_addr, NULL))
-					return -EADDRNOTAVAIL;
-			}
-		}
-	}
-
 	snum = ntohs(addr->sin6_port);
 	if (snum && snum < PROT_SOCK && !capable(CAP_NET_BIND_SERVICE))
 		return -EACCES;
@@ -331,23 +314,56 @@
 
 	/* Check these errors (active socket, double bind). */
 	if (sk->sk_state != TCP_CLOSE || inet->num) {
-		release_sock(sk);
-		return -EINVAL;
+		err = -EINVAL;
+		goto out;
 	}
 
-	if (addr_type & IPV6_ADDR_LINKLOCAL) {
-		if (addr_len >= sizeof(struct sockaddr_in6) &&
-		    addr->sin6_scope_id) {
-			/* Override any existing binding, if another one
-			 * is supplied by user.
-			 */
-			sk->sk_bound_dev_if = addr->sin6_scope_id;
+	/* Check if the address belongs to the host. */
+	if (addr_type == IPV6_ADDR_MAPPED) {
+		v4addr = addr->sin6_addr.s6_addr32[3];
+		if (inet_addr_type(v4addr) != RTN_LOCAL) {
+			err = -EADDRNOTAVAIL;
+			goto out;
 		}
+	} else {
+		if (addr_type != IPV6_ADDR_ANY) {
+			struct net_device *dev = NULL;
+
+			if (addr_type & IPV6_ADDR_LINKLOCAL) {
+				if (addr_len >= sizeof(struct sockaddr_in6) &&
+				    addr->sin6_scope_id) {
+					/* Override any existing binding, if another one
+					 * is supplied by user.
+					 */
+					sk->sk_bound_dev_if = addr->sin6_scope_id;
+				}
+				
+				/* Binding to link-local address requires an interface */
+				if (!sk->sk_bound_dev_if) {
+					err = -EINVAL;
+					goto out;
+				}
+				dev = dev_get_by_index(sk->sk_bound_dev_if);
+				if (!dev) {
+					err = -ENODEV;
+					goto out;
+				}
+			}
 
-		/* Binding to link-local address requires an interface */
-		if (!sk->sk_bound_dev_if) {
-			release_sock(sk);
-			return -EINVAL;
+			/* ipv4 addr of the socket is invalid.  Only the
+			 * unspecified and mapped address have a v4 equivalent.
+			 */
+			v4addr = LOOPBACK4_IPV6;
+			if (!(addr_type & IPV6_ADDR_MULTICAST))	{
+				if (!ipv6_chk_addr(&addr->sin6_addr, dev)) {
+					if (dev)
+						dev_put(dev);
+					err = -EADDRNOTAVAIL;
+					goto out;
+				}
+			}
+			if (dev)
+				dev_put(dev);
 		}
 	}
 
@@ -362,8 +378,8 @@
 	/* Make sure we are allowed to bind here. */
 	if (sk->sk_prot->get_port(sk, snum)) {
 		inet_reset_saddr(sk);
-		release_sock(sk);
-		return -EADDRINUSE;
+		err = -EADDRINUSE;
+		goto out;
 	}
 
 	if (addr_type != IPV6_ADDR_ANY)
@@ -373,9 +389,9 @@
 	inet->sport = ntohs(inet->num);
 	inet->dport = 0;
 	inet->daddr = 0;
+out:
 	release_sock(sk);
-
-	return 0;
+	return err;
 }
 
 int inet6_release(struct socket *sock)
diff -Nru a/net/ipv6/datagram.c b/net/ipv6/datagram.c
--- a/net/ipv6/datagram.c	Wed Jan 14 00:14:51 2004
+++ b/net/ipv6/datagram.c	Wed Jan 14 00:14:51 2004
@@ -265,6 +265,8 @@
 	int err = 0;
 
 	for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
+		int addr_type;
+		struct net_device *dev = NULL;
 
 		if (cmsg->cmsg_len < sizeof(struct cmsghdr) ||
 		    (unsigned long)(((char*)cmsg - (char*)msg->msg_control)
@@ -291,16 +293,30 @@
 				fl->oif = src_info->ipi6_ifindex;
 			}
 
-			if (!ipv6_addr_any(&src_info->ipi6_addr)) {
-				if (!ipv6_chk_addr(&src_info->ipi6_addr, NULL)) {
-					err = -EINVAL;
-					goto exit_f;
-				}
+			addr_type = ipv6_addr_type(&src_info->ipi6_addr);
 
-				ipv6_addr_copy(&fl->fl6_src,
-					       &src_info->ipi6_addr);
+			if (ipv6_addr_type == IPV6_ADDR_ANY)
+				break;
+			
+			if (addr_type & IPV6_ADDR_LINKLOCAL) {
+				if (!src_info->ipi6_ifindex)
+					return -EINVAL;
+				else {
+					dev = dev_get_by_index(src_info->ipi6_ifindex);
+					if (!dev)
+						return -ENODEV;
+				}
+			}
+			if (!ipv6_chk_addr(&src_info->ipi6_addr, dev)) {
+				if (dev)
+					dev_put(dev);
+				err = -EINVAL;
+				goto exit_f;
 			}
+			if (dev)
+				dev_put(dev);
 
+			ipv6_addr_copy(&fl->fl6_src, &src_info->ipi6_addr);
 			break;
 
 		case IPV6_FLOWINFO:
diff -Nru a/net/ipv6/raw.c b/net/ipv6/raw.c
--- a/net/ipv6/raw.c	Wed Jan 14 00:14:51 2004
+++ b/net/ipv6/raw.c	Wed Jan 14 00:14:51 2004
@@ -197,31 +197,44 @@
 	if (sk->sk_state != TCP_CLOSE)
 		goto out;
 
-	if (addr_type & IPV6_ADDR_LINKLOCAL) {
-		if (addr_len >= sizeof(struct sockaddr_in6) &&
-		    addr->sin6_scope_id) {
-			/* Override any existing binding, if another one
-			 * is supplied by user.
-			 */
-			sk->sk_bound_dev_if = addr->sin6_scope_id;
-		}
-
-		/* Binding to link-local address requires an interface */
-		if (!sk->sk_bound_dev_if)
-			goto out;
-	}
-
 	/* Check if the address belongs to the host. */
 	if (addr_type != IPV6_ADDR_ANY) {
+		struct net_device *dev = NULL;
+
+		if (addr_type & IPV6_ADDR_LINKLOCAL) {
+			if (addr_len >= sizeof(struct sockaddr_in6) &&
+			    addr->sin6_scope_id) {
+				/* Override any existing binding, if another
+				 * one is supplied by user.
+				 */
+				sk->sk_bound_dev_if = addr->sin6_scope_id;
+			}
+			
+			/* Binding to link-local address requires an interface */
+			if (!sk->sk_bound_dev_if)
+				goto out;
+
+			dev = dev_get_by_index(sk->sk_bound_dev_if);
+			if (!dev) {
+				err = -ENODEV;
+				goto out;
+			}
+		}
+		
 		/* ipv4 addr of the socket is invalid.  Only the
 		 * unpecified and mapped address have a v4 equivalent.
 		 */
 		v4addr = LOOPBACK4_IPV6;
 		if (!(addr_type & IPV6_ADDR_MULTICAST))	{
 			err = -EADDRNOTAVAIL;
-			if (!ipv6_chk_addr(&addr->sin6_addr, NULL))
+			if (!ipv6_chk_addr(&addr->sin6_addr, dev)) {
+				if (dev)
+					dev_put(dev);
 				goto out;
+			}
 		}
+		if (dev)
+			dev_put(dev);
 	}
 
 	inet->rcv_saddr = inet->saddr = v4addr;

Re: [PATCH 1/2] IPv6: stricter checks on link-locals in bind and sendmsg

[David S. Miller]

On Wed, 14 Jan 2004 10:35:11 +0200 (EET)
Ville Nuorvala <vnuorval@tcs.hut.fi> wrote:

> I found a couple of suspected bugs related to (source) address
> handling while rummaging around the code...
> 
> When binding to a link-local address, inet6_bind() and raw6_bind() only
> check that an interface is specified and that the address exists, but
> they don't check if it actually exists on the specified interface.
> 
> Similarly, in datagram_sent_ctl() we don't check for the possibility of a
> link-local address when we receive the source address from userspace.

Applied, thanks Ville.