l2tp: fix races with tunnel socket close
The tunnel socket tunnel->sock (struct sock) is accessed when
preparing a new ppp session on a tunnel at pppol2tp_session_init. If
the socket is closed by a thread while another is creating a new
session, the threads race. In pppol2tp_connect, the tunnel object may
be created if the pppol2tp socket is associated with the special
session_id 0 and the tunnel socket is looked up using the provided
fd. When handling this, pppol2tp_connect cannot sock_hold the tunnel
socket to prevent it being destroyed during pppol2tp_connect since
this may itself may race with the socket being destroyed. Doing
sockfd_lookup in pppol2tp_connect isn't sufficient to prevent
tunnel->sock going away either because a given tunnel socket fd may be
reused between calls to pppol2tp_connect. Instead, have
l2tp_tunnel_create sock_hold the tunnel socket before it does
sockfd_put. This ensures that the tunnel's socket is always extant
while the tunnel object exists. Hold a ref on the socket until the
tunnel is destroyed and ensure that all tunnel destroy paths go
through a common function (l2tp_tunnel_delete) since this will do the
final sock_put to release the tunnel socket.
Since the tunnel's socket is now guaranteed to exist if the tunnel
exists, we no longer need to use sockfd_lookup via l2tp_sock_to_tunnel
to derive the tunnel from the socket since this is always
sk_user_data.
Also, sessions no longer sock_hold the tunnel socket since sessions
already hold a tunnel ref and the tunnel sock will not be freed until
the tunnel is freed. Removing these sock_holds in
l2tp_session_register avoids a possible sock leak in the
pppol2tp_connect error path if l2tp_session_register succeeds but
attaching a ppp channel fails. The pppol2tp_connect error path could
have been fixed instead and have the sock ref dropped when the session
is freed, but doing a sock_put of the tunnel socket when the session
is freed would require a new session_free callback. It is simpler to
just remove the sock_hold of the tunnel socket in
l2tp_session_register, now that the tunnel socket lifetime is
guaranteed.
Finally, some init code in l2tp_tunnel_create is reordered to ensure
that the new tunnel object's refcount is set and the tunnel socket ref
is taken before the tunnel socket destructor callbacks are set.
kasan: CONFIG_KASAN_INLINE enabled
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] SMP KASAN
Modules linked in:
CPU: 0 PID: 4360 Comm: syzbot_19c09769 Not tainted 4.16.0-rc2+ #34
Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
RIP: 0010:pppol2tp_session_init+0x1d6/0x500
RSP: 0018:ffff88001377fb40 EFLAGS: 00010212
RAX: dffffc0000000000 RBX: ffff88001636a940 RCX: ffffffff84836c1d
RDX: 0000000000000045 RSI: 0000000055976744 RDI: 0000000000000228
RBP: ffff88001377fb60 R08: ffffffff84836bc8 R09: 0000000000000002
R10: ffff88001377fab8 R11: 0000000000000001 R12: 0000000000000000
R13: ffff88001636aac8 R14: ffff8800160f81c0 R15: 1ffff100026eff76
FS: 00007ffb3ea66700(0000) GS:ffff88001a400000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020e77000 CR3: 0000000016261000 CR4: 00000000000006f0
Call Trace:
pppol2tp_connect+0xd18/0x13c0
? pppol2tp_session_create+0x170/0x170
? __might_fault+0x115/0x1d0
? lock_downgrade+0x860/0x860
? __might_fault+0xe5/0x1d0
? security_socket_connect+0x8e/0xc0
SYSC_connect+0x1b6/0x310
? SYSC_bind+0x280/0x280
? __do_page_fault+0x5d1/0xca0
? up_read+0x1f/0x40
? __do_page_fault+0x3c8/0xca0
SyS_connect+0x29/0x30
? SyS_accept+0x40/0x40
do_syscall_64+0x1e0/0x730
? trace_hardirqs_off_thunk+0x1a/0x1c
entry_SYSCALL_64_after_hwframe+0x42/0xb7
RIP: 0033:0x7ffb3e376259
RSP: 002b:00007ffeda4f6508 EFLAGS: 00000202 ORIG_RAX: 000000000000002a
RAX: ffffffffffffffda RBX: 0000000020e77012 RCX: 00007ffb3e376259
RDX: 000000000000002e RSI: 0000000020e77000 RDI: 0000000000000004
RBP: 00007ffeda4f6540 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000400b60
R13: 00007ffeda4f6660 R14: 0000000000000000 R15: 0000000000000000
Code: 80 3d b0 ff 06 02 00 0f 84 07 02 00 00 e8 13 d6 db fc 49 8d bc 24 28 02 00 00 48 b8 00 00 00 00 00 fc ff df 48 89 f
a 48 c1 ea 03 <80> 3c 02 00 0f 85 ed 02 00 00 4d 8b a4 24 28 02 00 00 e8 13 16
Fixes: 80d84ef3ff
("l2tp: prevent l2tp_tunnel_delete racing with userspace close")
Signed-off-by: James Chapman <jchapman@katalix.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
parent
225eb26489
commit
d00fa9adc5
|
@ -136,51 +136,6 @@ l2tp_session_id_hash_2(struct l2tp_net *pn, u32 session_id)
|
|||
|
||||
}
|
||||
|
||||
/* Lookup the tunnel socket, possibly involving the fs code if the socket is
|
||||
* owned by userspace. A struct sock returned from this function must be
|
||||
* released using l2tp_tunnel_sock_put once you're done with it.
|
||||
*/
|
||||
static struct sock *l2tp_tunnel_sock_lookup(struct l2tp_tunnel *tunnel)
|
||||
{
|
||||
int err = 0;
|
||||
struct socket *sock = NULL;
|
||||
struct sock *sk = NULL;
|
||||
|
||||
if (!tunnel)
|
||||
goto out;
|
||||
|
||||
if (tunnel->fd >= 0) {
|
||||
/* Socket is owned by userspace, who might be in the process
|
||||
* of closing it. Look the socket up using the fd to ensure
|
||||
* consistency.
|
||||
*/
|
||||
sock = sockfd_lookup(tunnel->fd, &err);
|
||||
if (sock)
|
||||
sk = sock->sk;
|
||||
} else {
|
||||
/* Socket is owned by kernelspace */
|
||||
sk = tunnel->sock;
|
||||
sock_hold(sk);
|
||||
}
|
||||
|
||||
out:
|
||||
return sk;
|
||||
}
|
||||
|
||||
/* Drop a reference to a tunnel socket obtained via. l2tp_tunnel_sock_put */
|
||||
static void l2tp_tunnel_sock_put(struct sock *sk)
|
||||
{
|
||||
struct l2tp_tunnel *tunnel = l2tp_sock_to_tunnel(sk);
|
||||
if (tunnel) {
|
||||
if (tunnel->fd >= 0) {
|
||||
/* Socket is owned by userspace */
|
||||
sockfd_put(sk->sk_socket);
|
||||
}
|
||||
sock_put(sk);
|
||||
}
|
||||
sock_put(sk);
|
||||
}
|
||||
|
||||
/* Session hash list.
|
||||
* The session_id SHOULD be random according to RFC2661, but several
|
||||
* L2TP implementations (Cisco and Microsoft) use incrementing
|
||||
|
@ -193,6 +148,13 @@ l2tp_session_id_hash(struct l2tp_tunnel *tunnel, u32 session_id)
|
|||
return &tunnel->session_hlist[hash_32(session_id, L2TP_HASH_BITS)];
|
||||
}
|
||||
|
||||
void l2tp_tunnel_free(struct l2tp_tunnel *tunnel)
|
||||
{
|
||||
sock_put(tunnel->sock);
|
||||
/* the tunnel is freed in the socket destructor */
|
||||
}
|
||||
EXPORT_SYMBOL(l2tp_tunnel_free);
|
||||
|
||||
/* Lookup a tunnel. A new reference is held on the returned tunnel. */
|
||||
struct l2tp_tunnel *l2tp_tunnel_get(const struct net *net, u32 tunnel_id)
|
||||
{
|
||||
|
@ -345,13 +307,11 @@ int l2tp_session_register(struct l2tp_session *session,
|
|||
}
|
||||
|
||||
l2tp_tunnel_inc_refcount(tunnel);
|
||||
sock_hold(tunnel->sock);
|
||||
hlist_add_head_rcu(&session->global_hlist, g_head);
|
||||
|
||||
spin_unlock_bh(&pn->l2tp_session_hlist_lock);
|
||||
} else {
|
||||
l2tp_tunnel_inc_refcount(tunnel);
|
||||
sock_hold(tunnel->sock);
|
||||
}
|
||||
|
||||
hlist_add_head(&session->hlist, head);
|
||||
|
@ -969,7 +929,7 @@ int l2tp_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
|
|||
{
|
||||
struct l2tp_tunnel *tunnel;
|
||||
|
||||
tunnel = l2tp_sock_to_tunnel(sk);
|
||||
tunnel = l2tp_tunnel(sk);
|
||||
if (tunnel == NULL)
|
||||
goto pass_up;
|
||||
|
||||
|
@ -977,13 +937,10 @@ int l2tp_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
|
|||
tunnel->name, skb->len);
|
||||
|
||||
if (l2tp_udp_recv_core(tunnel, skb, tunnel->recv_payload_hook))
|
||||
goto pass_up_put;
|
||||
goto pass_up;
|
||||
|
||||
sock_put(sk);
|
||||
return 0;
|
||||
|
||||
pass_up_put:
|
||||
sock_put(sk);
|
||||
pass_up:
|
||||
return 1;
|
||||
}
|
||||
|
@ -1214,7 +1171,6 @@ static void l2tp_tunnel_destruct(struct sock *sk)
|
|||
|
||||
l2tp_info(tunnel, L2TP_MSG_CONTROL, "%s: closing...\n", tunnel->name);
|
||||
|
||||
|
||||
/* Disable udp encapsulation */
|
||||
switch (tunnel->encap) {
|
||||
case L2TP_ENCAPTYPE_UDP:
|
||||
|
@ -1237,12 +1193,11 @@ static void l2tp_tunnel_destruct(struct sock *sk)
|
|||
list_del_rcu(&tunnel->list);
|
||||
spin_unlock_bh(&pn->l2tp_tunnel_list_lock);
|
||||
|
||||
tunnel->sock = NULL;
|
||||
l2tp_tunnel_dec_refcount(tunnel);
|
||||
|
||||
/* Call the original destructor */
|
||||
if (sk->sk_destruct)
|
||||
(*sk->sk_destruct)(sk);
|
||||
|
||||
kfree_rcu(tunnel, rcu);
|
||||
end:
|
||||
return;
|
||||
}
|
||||
|
@ -1303,30 +1258,22 @@ EXPORT_SYMBOL_GPL(l2tp_tunnel_closeall);
|
|||
/* Tunnel socket destroy hook for UDP encapsulation */
|
||||
static void l2tp_udp_encap_destroy(struct sock *sk)
|
||||
{
|
||||
struct l2tp_tunnel *tunnel = l2tp_sock_to_tunnel(sk);
|
||||
if (tunnel) {
|
||||
l2tp_tunnel_closeall(tunnel);
|
||||
sock_put(sk);
|
||||
}
|
||||
struct l2tp_tunnel *tunnel = l2tp_tunnel(sk);
|
||||
|
||||
if (tunnel)
|
||||
l2tp_tunnel_delete(tunnel);
|
||||
}
|
||||
|
||||
/* Workqueue tunnel deletion function */
|
||||
static void l2tp_tunnel_del_work(struct work_struct *work)
|
||||
{
|
||||
struct l2tp_tunnel *tunnel = NULL;
|
||||
struct socket *sock = NULL;
|
||||
struct sock *sk = NULL;
|
||||
|
||||
tunnel = container_of(work, struct l2tp_tunnel, del_work);
|
||||
struct l2tp_tunnel *tunnel = container_of(work, struct l2tp_tunnel,
|
||||
del_work);
|
||||
struct sock *sk = tunnel->sock;
|
||||
struct socket *sock = sk->sk_socket;
|
||||
|
||||
l2tp_tunnel_closeall(tunnel);
|
||||
|
||||
sk = l2tp_tunnel_sock_lookup(tunnel);
|
||||
if (!sk)
|
||||
goto out;
|
||||
|
||||
sock = sk->sk_socket;
|
||||
|
||||
/* If the tunnel socket was created within the kernel, use
|
||||
* the sk API to release it here.
|
||||
*/
|
||||
|
@ -1337,8 +1284,10 @@ static void l2tp_tunnel_del_work(struct work_struct *work)
|
|||
}
|
||||
}
|
||||
|
||||
l2tp_tunnel_sock_put(sk);
|
||||
out:
|
||||
/* drop initial ref */
|
||||
l2tp_tunnel_dec_refcount(tunnel);
|
||||
|
||||
/* drop workqueue ref */
|
||||
l2tp_tunnel_dec_refcount(tunnel);
|
||||
}
|
||||
|
||||
|
@ -1591,13 +1540,22 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32
|
|||
sk->sk_user_data = tunnel;
|
||||
}
|
||||
|
||||
/* Bump the reference count. The tunnel context is deleted
|
||||
* only when this drops to zero. A reference is also held on
|
||||
* the tunnel socket to ensure that it is not released while
|
||||
* the tunnel is extant. Must be done before sk_destruct is
|
||||
* set.
|
||||
*/
|
||||
refcount_set(&tunnel->ref_count, 1);
|
||||
sock_hold(sk);
|
||||
tunnel->sock = sk;
|
||||
tunnel->fd = fd;
|
||||
|
||||
/* Hook on the tunnel socket destructor so that we can cleanup
|
||||
* if the tunnel socket goes away.
|
||||
*/
|
||||
tunnel->old_sk_destruct = sk->sk_destruct;
|
||||
sk->sk_destruct = &l2tp_tunnel_destruct;
|
||||
tunnel->sock = sk;
|
||||
tunnel->fd = fd;
|
||||
lockdep_set_class_and_name(&sk->sk_lock.slock, &l2tp_socket_class, "l2tp_sock");
|
||||
|
||||
sk->sk_allocation = GFP_ATOMIC;
|
||||
|
@ -1607,11 +1565,6 @@ int l2tp_tunnel_create(struct net *net, int fd, int version, u32 tunnel_id, u32
|
|||
|
||||
/* Add tunnel to our list */
|
||||
INIT_LIST_HEAD(&tunnel->list);
|
||||
|
||||
/* Bump the reference count. The tunnel context is deleted
|
||||
* only when this drops to zero. Must be done before list insertion
|
||||
*/
|
||||
refcount_set(&tunnel->ref_count, 1);
|
||||
spin_lock_bh(&pn->l2tp_tunnel_list_lock);
|
||||
list_add_rcu(&tunnel->list, &pn->l2tp_tunnel_list);
|
||||
spin_unlock_bh(&pn->l2tp_tunnel_list_lock);
|
||||
|
@ -1652,8 +1605,6 @@ void l2tp_session_free(struct l2tp_session *session)
|
|||
|
||||
if (tunnel) {
|
||||
BUG_ON(tunnel->magic != L2TP_TUNNEL_MAGIC);
|
||||
sock_put(tunnel->sock);
|
||||
session->tunnel = NULL;
|
||||
l2tp_tunnel_dec_refcount(tunnel);
|
||||
}
|
||||
|
||||
|
|
|
@ -214,27 +214,8 @@ static inline void *l2tp_session_priv(struct l2tp_session *session)
|
|||
return &session->priv[0];
|
||||
}
|
||||
|
||||
static inline struct l2tp_tunnel *l2tp_sock_to_tunnel(struct sock *sk)
|
||||
{
|
||||
struct l2tp_tunnel *tunnel;
|
||||
|
||||
if (sk == NULL)
|
||||
return NULL;
|
||||
|
||||
sock_hold(sk);
|
||||
tunnel = (struct l2tp_tunnel *)(sk->sk_user_data);
|
||||
if (tunnel == NULL) {
|
||||
sock_put(sk);
|
||||
goto out;
|
||||
}
|
||||
|
||||
BUG_ON(tunnel->magic != L2TP_TUNNEL_MAGIC);
|
||||
|
||||
out:
|
||||
return tunnel;
|
||||
}
|
||||
|
||||
struct l2tp_tunnel *l2tp_tunnel_get(const struct net *net, u32 tunnel_id);
|
||||
void l2tp_tunnel_free(struct l2tp_tunnel *tunnel);
|
||||
|
||||
struct l2tp_session *l2tp_session_get(const struct net *net,
|
||||
struct l2tp_tunnel *tunnel,
|
||||
|
@ -283,7 +264,7 @@ static inline void l2tp_tunnel_inc_refcount(struct l2tp_tunnel *tunnel)
|
|||
static inline void l2tp_tunnel_dec_refcount(struct l2tp_tunnel *tunnel)
|
||||
{
|
||||
if (refcount_dec_and_test(&tunnel->ref_count))
|
||||
kfree_rcu(tunnel, rcu);
|
||||
l2tp_tunnel_free(tunnel);
|
||||
}
|
||||
|
||||
/* Session reference counts. Incremented when code obtains a reference
|
||||
|
|
|
@ -234,17 +234,13 @@ static void l2tp_ip_close(struct sock *sk, long timeout)
|
|||
static void l2tp_ip_destroy_sock(struct sock *sk)
|
||||
{
|
||||
struct sk_buff *skb;
|
||||
struct l2tp_tunnel *tunnel = l2tp_sock_to_tunnel(sk);
|
||||
struct l2tp_tunnel *tunnel = sk->sk_user_data;
|
||||
|
||||
while ((skb = __skb_dequeue_tail(&sk->sk_write_queue)) != NULL)
|
||||
kfree_skb(skb);
|
||||
|
||||
if (tunnel) {
|
||||
l2tp_tunnel_closeall(tunnel);
|
||||
sock_put(sk);
|
||||
}
|
||||
|
||||
sk_refcnt_debug_dec(sk);
|
||||
if (tunnel)
|
||||
l2tp_tunnel_delete(tunnel);
|
||||
}
|
||||
|
||||
static int l2tp_ip_bind(struct sock *sk, struct sockaddr *uaddr, int addr_len)
|
||||
|
|
|
@ -248,16 +248,14 @@ static void l2tp_ip6_close(struct sock *sk, long timeout)
|
|||
|
||||
static void l2tp_ip6_destroy_sock(struct sock *sk)
|
||||
{
|
||||
struct l2tp_tunnel *tunnel = l2tp_sock_to_tunnel(sk);
|
||||
struct l2tp_tunnel *tunnel = sk->sk_user_data;
|
||||
|
||||
lock_sock(sk);
|
||||
ip6_flush_pending_frames(sk);
|
||||
release_sock(sk);
|
||||
|
||||
if (tunnel) {
|
||||
l2tp_tunnel_closeall(tunnel);
|
||||
sock_put(sk);
|
||||
}
|
||||
if (tunnel)
|
||||
l2tp_tunnel_delete(tunnel);
|
||||
|
||||
inet6_destroy_sock(sk);
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue