Skip to content

Commit

Permalink
Merge branch 'l2tp-fixes'
Browse files Browse the repository at this point in the history 
Guillaume Nault says:

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

This series addresses problems found while working on commit 32c2311
("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>
  • Loading branch information
@davem330
davem330 committed Nov 30, 2016
2 parents bb83d62 + 31e2f21 commit 7752f72
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 67 deletions.
2 changes: 2 additions & 0 deletions include/net/ipv6.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 3 additions & 1 deletion net/ipv6/datagram.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -252,6 +253,7 @@ static int __ip6_datagram_connect(struct sock *sk, struct sockaddr *uaddr, int a
out:
return err;
}
EXPORT_SYMBOL_GPL(__ip6_datagram_connect);

int ip6_datagram_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
{
Expand Down
63 changes: 34 additions & 29 deletions net/l2tp/l2tp_ip.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -182,15 +183,17 @@ static int l2tp_ip_recv(struct sk_buff *skb)
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;

Expand Down Expand Up @@ -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;

Expand All @@ -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;
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);

out:
release_sock(sk);

return ret;

out_in_use:
read_unlock_bh(&l2tp_ip_lock);

return ret;
}

Expand All @@ -308,29 +308,34 @@ 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);
hlist_del_init(&sk->sk_bind_node);
sk_add_bind_node(sk, &l2tp_ip_bind_table);
write_unlock_bh(&l2tp_ip_lock);

out_sk:
release_sock(sk);

return rc;
}

Expand Down
79 changes: 42 additions & 37 deletions net/l2tp/l2tp_ip6.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -196,16 +197,17 @@ static int l2tp_ip6_recv(struct sk_buff *skb)
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;

Expand Down Expand Up @@ -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;

Expand All @@ -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;
Expand All @@ -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;
}
Expand All @@ -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);
Expand All @@ -355,10 +357,7 @@ static int l2tp_ip6_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len)
rcu_read_unlock();
out_unlock:
release_sock(sk);
return err;

out_in_use:
read_unlock_bh(&l2tp_ip6_lock);
return err;
}

Expand All @@ -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;

Expand All @@ -390,17 +386,26 @@ 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);
hlist_del_init(&sk->sk_bind_node);
sk_add_bind_node(sk, &l2tp_ip6_bind_table);
write_unlock_bh(&l2tp_ip6_lock);

out_sk:
release_sock(sk);

return rc;
Expand Down

0 comments on commit 7752f72

Please sign in to comment.