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] [thread-next>] [day] [month] [year] [list]
Date:   Thu,  8 Mar 2018 15:37:27 +0100
From:   Paolo Abeni <pabeni@...hat.com>
To:     netdev@...r.kernel.org
Cc:     "David S. Miller" <davem@...emloft.net>,
        Guillaume Nault <g.nault@...halink.fr>,
        James Chapman <jchapman@...alix.com>,
        Wei Wang <weiwan@...gle.com>, David Ahern <dsahern@...il.com>
Subject: [PATCH net 2/2] l2tp: fix races with ipv4-mapped ipv6 addresses

When creating a new socket, l2tp_tunnel_create() ensures that
such socket is connected, but when using a socket provided by
the user space, no check is done on the socket state.

This may foul the later check for ipv6 sockets that are
ipv4-mapped, e.g. in case of unconnected ipv6 socket bound to
ipv4 address.

Moreover the connection status and/or peer of a user-space
controlled socket may change at runtime.

This change addresses the issues:
* explicitly checking for TCP_ESTABLISHED for user space provided sockets
* dropping the v4mapped flag usage - it can become outdated - and
  explicitly invoking ipv6_addr_v4mapped() instead
* refreshing the inet_sk copy of ipv4-mapped ipv6 address at xmit time.

The issue is apparently there since ancient times.

Reported-and-tested-by: syzbot+92fa328176eb07e4ac1a@...kaller.appspotmail.com
Fixes: 3557baabf280 ("[L2TP]: PPP over L2TP driver core")
Signed-off-by: Paolo Abeni <pabeni@...hat.com>
---
 net/l2tp/l2tp_core.c | 58 +++++++++++++++++++++++++++++++++-------------------
 net/l2tp/l2tp_core.h | 13 +++++++++---
 2 files changed, 47 insertions(+), 24 deletions(-)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 83421c6f0bef..ad6aa9b64415 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -1049,7 +1049,8 @@ static int l2tp_xmit_core(struct l2tp_session *session, struct sk_buff *skb,
 	/* Queue the packet to IP for output */
 	skb->ignore_df = 1;
 #if IS_ENABLED(CONFIG_IPV6)
-	if (tunnel->sock->sk_family == PF_INET6 && !tunnel->v4mapped)
+	if (tunnel->sock->sk_family == PF_INET6 &&
+	    !ipv6_addr_v4mapped(&tunnel->sock->sk_v6_daddr))
 		error = inet6_csk_xmit(tunnel->sock, skb, NULL);
 	else
 #endif
@@ -1112,11 +1113,30 @@ int l2tp_xmit_skb(struct l2tp_session *session, struct sk_buff *skb, int hdr_len
 		goto out_unlock;
 	}
 
+	/* User-space may change the connection status for the user-space
+	 * provided socket at run time: we must check it under the socket lock
+	 */
+	inet = inet_sk(sk);
+	if (tunnel->fd >= 0) {
+		if (sk->sk_state != TCP_ESTABLISHED) {
+			ret = NET_XMIT_DROP;
+			goto out_unlock;
+		}
+
+		/* if the uses space changes the ipv4-mapped ipv6 address,
+		 * the kernel copy of the ipv4 address is not updated.
+		 * Refresh it only if needed, to avoid dirtying the socket
+		 * on each packet.
+		 */
+		if (l2tp_sk_is_v4mapped(sk) &&
+		    inet->inet_daddr != sk->sk_v6_daddr.s6_addr32[3])
+			inet->inet_daddr = sk->sk_v6_daddr.s6_addr32[3];
+	}
+
 	/* Get routing info from the tunnel socket */
 	skb_dst_drop(skb);
 	skb_dst_set(skb, dst_clone(__sk_dst_check(sk, 0)));
 
-	inet = inet_sk(sk);
 	fl = &inet->cork.fl;
 	switch (tunnel->encap) {
 	case L2TP_ENCAPTYPE_UDP:
@@ -1130,15 +1150,13 @@ int l2tp_xmit_skb(struct l2tp_session *session, struct sk_buff *skb, int hdr_len
 		uh->len = htons(udp_len);
 
 		/* Calculate UDP checksum if configured to do so */
-#if IS_ENABLED(CONFIG_IPV6)
-		if (sk->sk_family == PF_INET6 && !tunnel->v4mapped)
+		if (l2tp_sk_is_v4mapped(sk))
 			udp6_set_csum(udp_get_no_check6_tx(sk),
 				      skb, &inet6_sk(sk)->saddr,
 				      &sk->sk_v6_daddr, udp_len);
 		else
-#endif
-		udp_set_csum(sk->sk_no_check_tx, skb, inet->inet_saddr,
-			     inet->inet_daddr, udp_len);
+			udp_set_csum(sk->sk_no_check_tx, skb, inet->inet_saddr,
+				     inet->inet_daddr, udp_len);
 		break;
 
 	case L2TP_ENCAPTYPE_IP:
@@ -1449,6 +1467,13 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32
 			err = -EINVAL;
 			goto err;
 		}
+
+		/* Reject unconnected sockets */
+		if (sock->sk->sk_state != TCP_ESTABLISHED) {
+			pr_err("tunl %u: sock fd=%d is unconnected\n",
+			       tunnel_id, fd);
+			goto err;
+		}
 	}
 
 	sk = sock->sk;
@@ -1507,23 +1532,14 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32
 	if (cfg != NULL)
 		tunnel->debug = cfg->debug;
 
-#if IS_ENABLED(CONFIG_IPV6)
-	if (sk->sk_family == PF_INET6) {
+	if (l2tp_sk_is_v4mapped(sk)) {
 		struct ipv6_pinfo *np = inet6_sk(sk);
+		struct inet_sock *inet = inet_sk(sk);
 
-		if (ipv6_addr_v4mapped(&np->saddr) &&
-		    ipv6_addr_v4mapped(&sk->sk_v6_daddr)) {
-			struct inet_sock *inet = inet_sk(sk);
-
-			tunnel->v4mapped = true;
-			inet->inet_saddr = np->saddr.s6_addr32[3];
-			inet->inet_rcv_saddr = sk->sk_v6_rcv_saddr.s6_addr32[3];
-			inet->inet_daddr = sk->sk_v6_daddr.s6_addr32[3];
-		} else {
-			tunnel->v4mapped = false;
-		}
+		inet->inet_saddr = np->saddr.s6_addr32[3];
+		inet->inet_rcv_saddr = sk->sk_v6_rcv_saddr.s6_addr32[3];
+		inet->inet_daddr = sk->sk_v6_daddr.s6_addr32[3];
 	}
-#endif
 
 	/* Mark socket as an encapsulation socket. See net/ipv4/udp.c */
 	tunnel->encap = encap;
diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index a1aa9550f04e..c042aaeb074b 100644
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -188,9 +188,6 @@ struct l2tp_tunnel {
 	struct sock		*sock;		/* Parent socket */
 	int			fd;		/* Parent fd, if tunnel socket
 						 * was created by userspace */
-#if IS_ENABLED(CONFIG_IPV6)
-	bool			v4mapped;
-#endif
 
 	struct work_struct	del_work;
 
@@ -214,6 +211,16 @@ static inline void *l2tp_session_priv(struct l2tp_session *session)
 	return &session->priv[0];
 }
 
+static bool l2tp_sk_is_v4mapped(struct sock *sk)
+{
+#if IS_ENABLED(CONFIG_IPV6)
+	return sk->sk_family == PF_INET6 &&
+	       ipv6_addr_v4mapped(&sk->sk_v6_daddr);
+#else
+	return 0;
+#endif
+}
+
 struct l2tp_tunnel *l2tp_tunnel_get(const struct net *net, u32 tunnel_id);
 void l2tp_tunnel_free(struct l2tp_tunnel *tunnel);
 
-- 
2.14.3

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ