Commit 7752f727 authored by David S. Miller's avatar David S. Miller

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 32c23116
("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: default avatarJames Chapman <jchapman@katalix.com>
parents bb83d62f 31e2f21f
...@@ -970,6 +970,8 @@ int compat_ipv6_setsockopt(struct sock *sk, int level, int optname, ...@@ -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, int compat_ipv6_getsockopt(struct sock *sk, int level, int optname,
char __user *optval, int __user *optlen); 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(struct sock *sk, struct sockaddr *addr, int addr_len);
int ip6_datagram_connect_v6_only(struct sock *sk, struct sockaddr *addr, int ip6_datagram_connect_v6_only(struct sock *sk, struct sockaddr *addr,
int addr_len); int addr_len);
......
...@@ -139,7 +139,8 @@ void ip6_datagram_release_cb(struct sock *sk) ...@@ -139,7 +139,8 @@ void ip6_datagram_release_cb(struct sock *sk)
} }
EXPORT_SYMBOL_GPL(ip6_datagram_release_cb); 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 sockaddr_in6 *usin = (struct sockaddr_in6 *) uaddr;
struct inet_sock *inet = inet_sk(sk); struct inet_sock *inet = inet_sk(sk);
...@@ -252,6 +253,7 @@ static int __ip6_datagram_connect(struct sock *sk, struct sockaddr *uaddr, int a ...@@ -252,6 +253,7 @@ static int __ip6_datagram_connect(struct sock *sk, struct sockaddr *uaddr, int a
out: out:
return err; return err;
} }
EXPORT_SYMBOL_GPL(__ip6_datagram_connect);
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)
{ {
......
...@@ -61,7 +61,8 @@ static struct sock *__l2tp_ip_bind_lookup(struct net *net, __be32 laddr, int dif ...@@ -61,7 +61,8 @@ static struct sock *__l2tp_ip_bind_lookup(struct net *net, __be32 laddr, int dif
if ((l2tp->conn_id == tunnel_id) && if ((l2tp->conn_id == tunnel_id) &&
net_eq(sock_net(sk), net) && net_eq(sock_net(sk), net) &&
!(inet->inet_rcv_saddr && inet->inet_rcv_saddr != laddr) && !(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; goto found;
} }
...@@ -182,14 +183,16 @@ static int l2tp_ip_recv(struct sk_buff *skb) ...@@ -182,14 +183,16 @@ static int l2tp_ip_recv(struct sk_buff *skb)
struct iphdr *iph = (struct iphdr *) skb_network_header(skb); struct iphdr *iph = (struct iphdr *) skb_network_header(skb);
read_lock_bh(&l2tp_ip_lock); 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); read_unlock_bh(&l2tp_ip_lock);
}
if (sk == NULL)
goto discard; goto discard;
}
sock_hold(sk); sock_hold(sk);
read_unlock_bh(&l2tp_ip_lock);
}
if (!xfrm4_policy_check(sk, XFRM_POLICY_IN, skb)) if (!xfrm4_policy_check(sk, XFRM_POLICY_IN, skb))
goto discard_put; goto discard_put;
...@@ -256,15 +259,9 @@ static int l2tp_ip_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len) ...@@ -256,15 +259,9 @@ static int l2tp_ip_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len)
if (addr->l2tp_family != AF_INET) if (addr->l2tp_family != AF_INET)
return -EINVAL; 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); lock_sock(sk);
ret = -EINVAL;
if (!sock_flag(sk, SOCK_ZAPPED)) if (!sock_flag(sk, SOCK_ZAPPED))
goto out; goto out;
...@@ -281,25 +278,28 @@ static int l2tp_ip_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len) ...@@ -281,25 +278,28 @@ 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; inet->inet_rcv_saddr = inet->inet_saddr = addr->l2tp_addr.s_addr;
if (chk_addr_ret == RTN_MULTICAST || chk_addr_ret == RTN_BROADCAST) if (chk_addr_ret == RTN_MULTICAST || chk_addr_ret == RTN_BROADCAST)
inet->inet_saddr = 0; /* Use device */ 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; 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_add_bind_node(sk, &l2tp_ip_bind_table);
sk_del_node_init(sk); sk_del_node_init(sk);
write_unlock_bh(&l2tp_ip_lock); write_unlock_bh(&l2tp_ip_lock);
ret = 0; ret = 0;
sock_reset_flag(sk, SOCK_ZAPPED); sock_reset_flag(sk, SOCK_ZAPPED);
out: out:
release_sock(sk); release_sock(sk);
return ret;
out_in_use:
read_unlock_bh(&l2tp_ip_lock);
return ret; return ret;
} }
...@@ -308,21 +308,24 @@ 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; struct sockaddr_l2tpip *lsa = (struct sockaddr_l2tpip *) uaddr;
int rc; int rc;
if (sock_flag(sk, SOCK_ZAPPED)) /* Must bind first - autobinding does not work */
return -EINVAL;
if (addr_len < sizeof(*lsa)) if (addr_len < sizeof(*lsa))
return -EINVAL; return -EINVAL;
if (ipv4_is_multicast(lsa->l2tp_addr.s_addr)) if (ipv4_is_multicast(lsa->l2tp_addr.s_addr))
return -EINVAL; return -EINVAL;
rc = ip4_datagram_connect(sk, uaddr, addr_len);
if (rc < 0)
return rc;
lock_sock(sk); 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; l2tp_ip_sk(sk)->peer_conn_id = lsa->l2tp_conn_id;
write_lock_bh(&l2tp_ip_lock); write_lock_bh(&l2tp_ip_lock);
...@@ -330,7 +333,9 @@ static int l2tp_ip_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len ...@@ -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); sk_add_bind_node(sk, &l2tp_ip_bind_table);
write_unlock_bh(&l2tp_ip_lock); write_unlock_bh(&l2tp_ip_lock);
out_sk:
release_sock(sk); release_sock(sk);
return rc; return rc;
} }
......
...@@ -72,8 +72,9 @@ static struct sock *__l2tp_ip6_bind_lookup(struct net *net, ...@@ -72,8 +72,9 @@ static struct sock *__l2tp_ip6_bind_lookup(struct net *net,
if ((l2tp->conn_id == tunnel_id) && if ((l2tp->conn_id == tunnel_id) &&
net_eq(sock_net(sk), net) && net_eq(sock_net(sk), net) &&
!(addr && ipv6_addr_equal(addr, laddr)) && (!addr || ipv6_addr_equal(addr, 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; goto found;
} }
...@@ -196,15 +197,16 @@ static int l2tp_ip6_recv(struct sk_buff *skb) ...@@ -196,15 +197,16 @@ static int l2tp_ip6_recv(struct sk_buff *skb)
struct ipv6hdr *iph = ipv6_hdr(skb); struct ipv6hdr *iph = ipv6_hdr(skb);
read_lock_bh(&l2tp_ip6_lock); read_lock_bh(&l2tp_ip6_lock);
sk = __l2tp_ip6_bind_lookup(net, &iph->daddr, sk = __l2tp_ip6_bind_lookup(net, &iph->daddr, inet6_iif(skb),
0, tunnel_id); tunnel_id);
if (!sk) {
read_unlock_bh(&l2tp_ip6_lock); read_unlock_bh(&l2tp_ip6_lock);
}
if (sk == NULL)
goto discard; goto discard;
}
sock_hold(sk); sock_hold(sk);
read_unlock_bh(&l2tp_ip6_lock);
}
if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb)) if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
goto discard_put; goto discard_put;
...@@ -266,6 +268,7 @@ static int l2tp_ip6_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len) ...@@ -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 sockaddr_l2tpip6 *addr = (struct sockaddr_l2tpip6 *) uaddr;
struct net *net = sock_net(sk); struct net *net = sock_net(sk);
__be32 v4addr = 0; __be32 v4addr = 0;
int bound_dev_if;
int addr_type; int addr_type;
int err; int err;
...@@ -284,13 +287,6 @@ static int l2tp_ip6_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len) ...@@ -284,13 +287,6 @@ static int l2tp_ip6_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len)
if (addr_type & IPV6_ADDR_MULTICAST) if (addr_type & IPV6_ADDR_MULTICAST)
return -EADDRNOTAVAIL; 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); lock_sock(sk);
err = -EINVAL; err = -EINVAL;
...@@ -300,28 +296,25 @@ static int l2tp_ip6_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len) ...@@ -300,28 +296,25 @@ static int l2tp_ip6_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len)
if (sk->sk_state != TCP_CLOSE) if (sk->sk_state != TCP_CLOSE)
goto out_unlock; goto out_unlock;
bound_dev_if = sk->sk_bound_dev_if;
/* Check if the address belongs to the host. */ /* Check if the address belongs to the host. */
rcu_read_lock(); rcu_read_lock();
if (addr_type != IPV6_ADDR_ANY) { if (addr_type != IPV6_ADDR_ANY) {
struct net_device *dev = NULL; struct net_device *dev = NULL;
if (addr_type & IPV6_ADDR_LINKLOCAL) { if (addr_type & IPV6_ADDR_LINKLOCAL) {
if (addr_len >= sizeof(struct sockaddr_in6) && if (addr->l2tp_scope_id)
addr->l2tp_scope_id) { bound_dev_if = addr->l2tp_scope_id;
/* Override any existing binding, if another
* one is supplied by user.
*/
sk->sk_bound_dev_if = addr->l2tp_scope_id;
}
/* Binding to link-local address requires an /* Binding to link-local address requires an
interface */ * interface.
if (!sk->sk_bound_dev_if) */
if (!bound_dev_if)
goto out_unlock_rcu; goto out_unlock_rcu;
err = -ENODEV; err = -ENODEV;
dev = dev_get_by_index_rcu(sock_net(sk), dev = dev_get_by_index_rcu(sock_net(sk), bound_dev_if);
sk->sk_bound_dev_if);
if (!dev) if (!dev)
goto out_unlock_rcu; goto out_unlock_rcu;
} }
...@@ -336,13 +329,22 @@ static int l2tp_ip6_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len) ...@@ -336,13 +329,22 @@ static int l2tp_ip6_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len)
} }
rcu_read_unlock(); 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; sk->sk_v6_rcv_saddr = addr->l2tp_addr;
np->saddr = addr->l2tp_addr; np->saddr = addr->l2tp_addr;
l2tp_ip6_sk(sk)->conn_id = addr->l2tp_conn_id; 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_add_bind_node(sk, &l2tp_ip6_bind_table);
sk_del_node_init(sk); sk_del_node_init(sk);
write_unlock_bh(&l2tp_ip6_lock); write_unlock_bh(&l2tp_ip6_lock);
...@@ -355,10 +357,7 @@ static int l2tp_ip6_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len) ...@@ -355,10 +357,7 @@ static int l2tp_ip6_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len)
rcu_read_unlock(); rcu_read_unlock();
out_unlock: out_unlock:
release_sock(sk); release_sock(sk);
return err;
out_in_use:
read_unlock_bh(&l2tp_ip6_lock);
return err; return err;
} }
...@@ -371,9 +370,6 @@ static int l2tp_ip6_connect(struct sock *sk, struct sockaddr *uaddr, ...@@ -371,9 +370,6 @@ static int l2tp_ip6_connect(struct sock *sk, struct sockaddr *uaddr,
int addr_type; int addr_type;
int rc; int rc;
if (sock_flag(sk, SOCK_ZAPPED)) /* Must bind first - autobinding does not work */
return -EINVAL;
if (addr_len < sizeof(*lsa)) if (addr_len < sizeof(*lsa))
return -EINVAL; return -EINVAL;
...@@ -390,10 +386,18 @@ static int l2tp_ip6_connect(struct sock *sk, struct sockaddr *uaddr, ...@@ -390,10 +386,18 @@ static int l2tp_ip6_connect(struct sock *sk, struct sockaddr *uaddr,
return -EINVAL; return -EINVAL;
} }
rc = ip6_datagram_connect(sk, uaddr, addr_len);
lock_sock(sk); 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; l2tp_ip6_sk(sk)->peer_conn_id = lsa->l2tp_conn_id;
write_lock_bh(&l2tp_ip6_lock); write_lock_bh(&l2tp_ip6_lock);
...@@ -401,6 +405,7 @@ static int l2tp_ip6_connect(struct sock *sk, struct sockaddr *uaddr, ...@@ -401,6 +405,7 @@ static int l2tp_ip6_connect(struct sock *sk, struct sockaddr *uaddr,
sk_add_bind_node(sk, &l2tp_ip6_bind_table); sk_add_bind_node(sk, &l2tp_ip6_bind_table);
write_unlock_bh(&l2tp_ip6_lock); write_unlock_bh(&l2tp_ip6_lock);
out_sk:
release_sock(sk); release_sock(sk);
return rc; return rc;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment