Merge branch 'l2tp-fixes'

Guillaume Nault says:

====================
l2tp: fixes for l2tp_ip and l2tp_ip6 socket handling

This series addresses problems found while working on commit 32c231164b76
("l2tp: fix racy SOCK_ZAPPED flag check in l2tp_ip{,6}_bind()").

The first three patches fix races in socket's connect, recv and bind
operations. The last two ones fix scenarios where l2tp fails to
correctly lookup its userspace sockets.

Apart from the last patch, which is l2tp_ip6 specific, every patch
fixes the same problem in the L2TP IPv4 and IPv6 code.

All problems fixed by this series exist since the creation of the
l2tp_ip and l2tp_ip6 modules.

Changes since v1:
  * Patch #3: fix possible uninitialised use of 'ret' in l2tp_ip_bind().
====================

Acked-by: James Chapman <jchapman@katalix.com>

4 files changed, 81 insertions, 67 deletions

diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index 8fed1cd78658..f11ca837361b 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -970,6 +970,8 @@ int compat_ipv6_setsockopt(struct sock *sk, int level, int optname,
 int compat_ipv6_getsockopt(struct sock *sk, int level, int optname,
                            char __user *optval, int __user *optlen);
 
+int __ip6_datagram_connect(struct sock *sk, struct sockaddr *addr,
+                           int addr_len);
 int ip6_datagram_connect(struct sock *sk, struct sockaddr *addr, int addr_len);
 int ip6_datagram_connect_v6_only(struct sock *sk, struct sockaddr *addr,
                                  int addr_len);
diff --git a/net/ipv6/datagram.c b/net/ipv6/datagram.c
index 37874e2f30ed..ccf40550c475 100644
--- a/net/ipv6/datagram.c
+++ b/net/ipv6/datagram.c
@@ -139,7 +139,8 @@ void ip6_datagram_release_cb(struct sock *sk)
 }
 EXPORT_SYMBOL_GPL(ip6_datagram_release_cb);
 
-static int __ip6_datagram_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
+int __ip6_datagram_connect(struct sock *sk, struct sockaddr *uaddr,
+                           int addr_len)
 {
         struct sockaddr_in6     *usin = (struct sockaddr_in6 *) uaddr;
         struct inet_sock        *inet = inet_sk(sk);
@@ -252,6 +253,7 @@ ipv4_connected:
 out:
         return err;
 }
+EXPORT_SYMBOL_GPL(__ip6_datagram_connect);
 
 int ip6_datagram_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
 {
diff --git a/net/l2tp/l2tp_ip.c b/net/l2tp/l2tp_ip.c
index 982f6c44ea01..8938b6ba57a0 100644
--- a/net/l2tp/l2tp_ip.c
+++ b/net/l2tp/l2tp_ip.c
@@ -61,7 +61,8 @@ static struct sock *__l2tp_ip_bind_lookup(struct net *net, __be32 laddr, int dif
                 if ((l2tp->conn_id == tunnel_id) &&
                     net_eq(sock_net(sk), net) &&
                     !(inet->inet_rcv_saddr && inet->inet_rcv_saddr != laddr) &&
-                    !(sk->sk_bound_dev_if && sk->sk_bound_dev_if != dif))
+                    (!sk->sk_bound_dev_if || !dif ||
+                     sk->sk_bound_dev_if == dif))
                         goto found;
         }
 
@@ -182,15 +183,17 @@ pass_up:
                 struct iphdr *iph = (struct iphdr *) skb_network_header(skb);
 
                 read_lock_bh(&l2tp_ip_lock);
-                sk = __l2tp_ip_bind_lookup(net, iph->daddr, 0, tunnel_id);
+                sk = __l2tp_ip_bind_lookup(net, iph->daddr, inet_iif(skb),
+                                           tunnel_id);
+                if (!sk) {
+                        read_unlock_bh(&l2tp_ip_lock);
+                        goto discard;
+                }
+
+                sock_hold(sk);
                 read_unlock_bh(&l2tp_ip_lock);
         }
 
-        if (sk == NULL)
-                goto discard;
-
-        sock_hold(sk);
-
         if (!xfrm4_policy_check(sk, XFRM_POLICY_IN, skb))
                 goto discard_put;
 
@@ -256,15 +259,9 @@ static int l2tp_ip_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len)
         if (addr->l2tp_family != AF_INET)
                 return -EINVAL;
 
-        ret = -EADDRINUSE;
-        read_lock_bh(&l2tp_ip_lock);
-        if (__l2tp_ip_bind_lookup(net, addr->l2tp_addr.s_addr,
-                                  sk->sk_bound_dev_if, addr->l2tp_conn_id))
-                goto out_in_use;
-
-        read_unlock_bh(&l2tp_ip_lock);
-
         lock_sock(sk);
+
+        ret = -EINVAL;
         if (!sock_flag(sk, SOCK_ZAPPED))
                 goto out;
 
@@ -281,14 +278,22 @@ static int l2tp_ip_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len)
                 inet->inet_rcv_saddr = inet->inet_saddr = addr->l2tp_addr.s_addr;
         if (chk_addr_ret == RTN_MULTICAST || chk_addr_ret == RTN_BROADCAST)
                 inet->inet_saddr = 0;  /* Use device */
-        sk_dst_reset(sk);
 
+        write_lock_bh(&l2tp_ip_lock);
+        if (__l2tp_ip_bind_lookup(net, addr->l2tp_addr.s_addr,
+                                  sk->sk_bound_dev_if, addr->l2tp_conn_id)) {
+                write_unlock_bh(&l2tp_ip_lock);
+                ret = -EADDRINUSE;
+                goto out;
+        }
+
+        sk_dst_reset(sk);
         l2tp_ip_sk(sk)->conn_id = addr->l2tp_conn_id;
 
-        write_lock_bh(&l2tp_ip_lock);
         sk_add_bind_node(sk, &l2tp_ip_bind_table);
         sk_del_node_init(sk);
         write_unlock_bh(&l2tp_ip_lock);
+
         ret = 0;
         sock_reset_flag(sk, SOCK_ZAPPED);
 
@@ -296,11 +301,6 @@ out:
         release_sock(sk);
 
         return ret;
-
-out_in_use:
-        read_unlock_bh(&l2tp_ip_lock);
-
-        return ret;
 }
 
 static int l2tp_ip_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
@@ -308,21 +308,24 @@ static int l2tp_ip_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len
         struct sockaddr_l2tpip *lsa = (struct sockaddr_l2tpip *) uaddr;
         int rc;
 
-        if (sock_flag(sk, SOCK_ZAPPED)) /* Must bind first - autobinding does not work */
-                return -EINVAL;
-
         if (addr_len < sizeof(*lsa))
                 return -EINVAL;
 
         if (ipv4_is_multicast(lsa->l2tp_addr.s_addr))
                 return -EINVAL;
 
-        rc = ip4_datagram_connect(sk, uaddr, addr_len);
-        if (rc < 0)
-                return rc;
-
         lock_sock(sk);
 
+        /* Must bind first - autobinding does not work */
+        if (sock_flag(sk, SOCK_ZAPPED)) {
+                rc = -EINVAL;
+                goto out_sk;
+        }
+
+        rc = __ip4_datagram_connect(sk, uaddr, addr_len);
+        if (rc < 0)
+                goto out_sk;
+
         l2tp_ip_sk(sk)->peer_conn_id = lsa->l2tp_conn_id;
 
         write_lock_bh(&l2tp_ip_lock);
@@ -330,7 +333,9 @@ static int l2tp_ip_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len
         sk_add_bind_node(sk, &l2tp_ip_bind_table);
         write_unlock_bh(&l2tp_ip_lock);
 
+out_sk:
         release_sock(sk);
+
         return rc;
 }
 
diff --git a/net/l2tp/l2tp_ip6.c b/net/l2tp/l2tp_ip6.c
index 9978d01ba0ba..aa821cb639e5 100644
--- a/net/l2tp/l2tp_ip6.c
+++ b/net/l2tp/l2tp_ip6.c
@@ -72,8 +72,9 @@ static struct sock *__l2tp_ip6_bind_lookup(struct net *net,
 
                 if ((l2tp->conn_id == tunnel_id) &&
                     net_eq(sock_net(sk), net) &&
-                    !(addr && ipv6_addr_equal(addr, laddr)) &&
-                    !(sk->sk_bound_dev_if && sk->sk_bound_dev_if != dif))
+                    (!addr || ipv6_addr_equal(addr, laddr)) &&
+                    (!sk->sk_bound_dev_if || !dif ||
+                     sk->sk_bound_dev_if == dif))
                         goto found;
         }
 
@@ -196,16 +197,17 @@ pass_up:
                 struct ipv6hdr *iph = ipv6_hdr(skb);
 
                 read_lock_bh(&l2tp_ip6_lock);
-                sk = __l2tp_ip6_bind_lookup(net, &iph->daddr,
-                                            0, tunnel_id);
+                sk = __l2tp_ip6_bind_lookup(net, &iph->daddr, inet6_iif(skb),
+                                            tunnel_id);
+                if (!sk) {
+                        read_unlock_bh(&l2tp_ip6_lock);
+                        goto discard;
+                }
+
+                sock_hold(sk);
                 read_unlock_bh(&l2tp_ip6_lock);
         }
 
-        if (sk == NULL)
-                goto discard;
-
-        sock_hold(sk);
-
         if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
                 goto discard_put;
 
@@ -266,6 +268,7 @@ static int l2tp_ip6_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len)
         struct sockaddr_l2tpip6 *addr = (struct sockaddr_l2tpip6 *) uaddr;
         struct net *net = sock_net(sk);
         __be32 v4addr = 0;
+        int bound_dev_if;
         int addr_type;
         int err;
 
@@ -284,13 +287,6 @@ static int l2tp_ip6_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len)
         if (addr_type & IPV6_ADDR_MULTICAST)
                 return -EADDRNOTAVAIL;
 
-        err = -EADDRINUSE;
-        read_lock_bh(&l2tp_ip6_lock);
-        if (__l2tp_ip6_bind_lookup(net, &addr->l2tp_addr,
-                                   sk->sk_bound_dev_if, addr->l2tp_conn_id))
-                goto out_in_use;
-        read_unlock_bh(&l2tp_ip6_lock);
-
         lock_sock(sk);
 
         err = -EINVAL;
@@ -300,28 +296,25 @@ static int l2tp_ip6_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len)
         if (sk->sk_state != TCP_CLOSE)
                 goto out_unlock;
 
+        bound_dev_if = sk->sk_bound_dev_if;
+
         /* Check if the address belongs to the host. */
         rcu_read_lock();
         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->l2tp_scope_id) {
-                                /* Override any existing binding, if another
-                                 * one is supplied by user.
-                                 */
-                                sk->sk_bound_dev_if = addr->l2tp_scope_id;
-                        }
+                        if (addr->l2tp_scope_id)
+                                bound_dev_if = addr->l2tp_scope_id;
 
                         /* Binding to link-local address requires an
-                           interface */
-                        if (!sk->sk_bound_dev_if)
+                         * interface.
+                         */
+                        if (!bound_dev_if)
                                 goto out_unlock_rcu;
 
                         err = -ENODEV;
-                        dev = dev_get_by_index_rcu(sock_net(sk),
-                                                   sk->sk_bound_dev_if);
+                        dev = dev_get_by_index_rcu(sock_net(sk), bound_dev_if);
                         if (!dev)
                                 goto out_unlock_rcu;
                 }
@@ -336,13 +329,22 @@ static int l2tp_ip6_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len)
         }
         rcu_read_unlock();
 
-        inet->inet_rcv_saddr = inet->inet_saddr = v4addr;
+        write_lock_bh(&l2tp_ip6_lock);
+        if (__l2tp_ip6_bind_lookup(net, &addr->l2tp_addr, bound_dev_if,
+                                   addr->l2tp_conn_id)) {
+                write_unlock_bh(&l2tp_ip6_lock);
+                err = -EADDRINUSE;
+                goto out_unlock;
+        }
+
+        inet->inet_saddr = v4addr;
+        inet->inet_rcv_saddr = v4addr;
+        sk->sk_bound_dev_if = bound_dev_if;
         sk->sk_v6_rcv_saddr = addr->l2tp_addr;
         np->saddr = addr->l2tp_addr;
 
         l2tp_ip6_sk(sk)->conn_id = addr->l2tp_conn_id;
 
-        write_lock_bh(&l2tp_ip6_lock);
         sk_add_bind_node(sk, &l2tp_ip6_bind_table);
         sk_del_node_init(sk);
         write_unlock_bh(&l2tp_ip6_lock);
@@ -355,10 +357,7 @@ out_unlock_rcu:
         rcu_read_unlock();
 out_unlock:
         release_sock(sk);
-        return err;
 
-out_in_use:
-        read_unlock_bh(&l2tp_ip6_lock);
         return err;
 }
 
@@ -371,9 +370,6 @@ static int l2tp_ip6_connect(struct sock *sk, struct sockaddr *uaddr,
         int     addr_type;
         int rc;
 
-        if (sock_flag(sk, SOCK_ZAPPED)) /* Must bind first - autobinding does not work */
-                return -EINVAL;
-
         if (addr_len < sizeof(*lsa))
                 return -EINVAL;
 
@@ -390,10 +386,18 @@ static int l2tp_ip6_connect(struct sock *sk, struct sockaddr *uaddr,
                         return -EINVAL;
         }
 
-        rc = ip6_datagram_connect(sk, uaddr, addr_len);
-
         lock_sock(sk);
 
+         /* Must bind first - autobinding does not work */
+        if (sock_flag(sk, SOCK_ZAPPED)) {
+                rc = -EINVAL;
+                goto out_sk;
+        }
+
+        rc = __ip6_datagram_connect(sk, uaddr, addr_len);
+        if (rc < 0)
+                goto out_sk;
+
         l2tp_ip6_sk(sk)->peer_conn_id = lsa->l2tp_conn_id;
 
         write_lock_bh(&l2tp_ip6_lock);
@@ -401,6 +405,7 @@ static int l2tp_ip6_connect(struct sock *sk, struct sockaddr *uaddr,
         sk_add_bind_node(sk, &l2tp_ip6_bind_table);
         write_unlock_bh(&l2tp_ip6_lock);
 
+out_sk:
         release_sock(sk);
 
         return rc;