l2tp: fix race in pppol2tp_release with session object destroy
pppol2tp_release uses call_rcu to put the final ref on its socket. But
the session object doesn't hold a ref on the session socket so may be
freed while the pppol2tp_put_sk RCU callback is scheduled. Fix this by
having the session hold a ref on its socket until the session is
destroyed. It is this ref that is dropped via call_rcu.
Sessions are also deleted via l2tp_tunnel_closeall. This must now also put
the final ref via call_rcu. So move the call_rcu call site into
pppol2tp_session_close so that this happens in both destroy paths. A
common destroy path should really be implemented, perhaps with
l2tp_tunnel_closeall calling l2tp_session_delete like pppol2tp_release
does, but this will be looked at later.
ODEBUG: activate active (active state 1) object type: rcu_head hint: (null)
WARNING: CPU: 3 PID: 13407 at lib/debugobjects.c:291 debug_print_object+0x166/0x220
Modules linked in:
CPU: 3 PID: 13407 Comm: syzbot_19c09769 Not tainted 4.16.0-rc2+ #38
Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
RIP: 0010:debug_print_object+0x166/0x220
RSP: 0018:ffff880013647a00 EFLAGS: 00010082
RAX: dffffc0000000008 RBX: 0000000000000003 RCX: ffffffff814d3333
RDX: 0000000000000000 RSI: 0000000000000001 RDI: ffff88001a59f6d0
RBP: ffff880013647a40 R08: 0000000000000000 R09: 0000000000000001
R10: ffff8800136479a8 R11: 0000000000000000 R12: 0000000000000001
R13: ffffffff86161420 R14: ffffffff85648b60 R15: 0000000000000000
FS: 0000000000000000(0000) GS:ffff88001a580000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020e77000 CR3: 0000000006022000 CR4: 00000000000006e0
Call Trace:
debug_object_activate+0x38b/0x530
? debug_object_assert_init+0x3b0/0x3b0
? __mutex_unlock_slowpath+0x85/0x8b0
? pppol2tp_session_destruct+0x110/0x110
__call_rcu.constprop.66+0x39/0x890
? __call_rcu.constprop.66+0x39/0x890
call_rcu_sched+0x17/0x20
pppol2tp_release+0x2c7/0x440
? fcntl_setlk+0xca0/0xca0
? sock_alloc_file+0x340/0x340
sock_release+0x92/0x1e0
sock_close+0x1b/0x20
__fput+0x296/0x6e0
____fput+0x1a/0x20
task_work_run+0x127/0x1a0
do_exit+0x7f9/0x2ce0
? SYSC_connect+0x212/0x310
? mm_update_next_owner+0x690/0x690
? up_read+0x1f/0x40
? __do_page_fault+0x3c8/0xca0
do_group_exit+0x10d/0x330
? do_group_exit+0x330/0x330
SyS_exit_group+0x22/0x30
do_syscall_64+0x1e0/0x730
? trace_hardirqs_off_thunk+0x1a/0x1c
entry_SYSCALL_64_after_hwframe+0x42/0xb7
RIP: 0033:0x7f362e471259
RSP: 002b:00007ffe389abe08 EFLAGS: 00000202 ORIG_RAX: 00000000000000e7
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f362e471259
RDX: 00007f362e471259 RSI: 000000000000002e RDI: 0000000000000000
RBP: 00007ffe389abe30 R08: 0000000000000000 R09: 00007f362e944270
R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000400b60
R13: 00007ffe389abf50 R14: 0000000000000000 R15: 0000000000000000
Code: 8d 3c dd a0 8f 64 85 48 89 fa 48 c1 ea 03 80 3c 02 00 75 7b 48 8b 14 dd a0 8f 64 85 4c 89 f6 48 c7 c7 20 85 64 85 e
8 2a 55 14 ff <0f> 0b 83 05 ad 2a 68 04 01 48 83 c4 18 5b 41 5c 41 5d 41 5e 41
Fixes: ee40fb2e1e
("l2tp: protect sock pointer of struct pppol2tp_session with RCU")
Signed-off-by: James Chapman <jchapman@katalix.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
parent
d00fa9adc5
commit
d02ba2a611
|
@ -416,10 +416,28 @@ abort:
|
|||
* Session (and tunnel control) socket create/destroy.
|
||||
*****************************************************************************/
|
||||
|
||||
static void pppol2tp_put_sk(struct rcu_head *head)
|
||||
{
|
||||
struct pppol2tp_session *ps;
|
||||
|
||||
ps = container_of(head, typeof(*ps), rcu);
|
||||
sock_put(ps->__sk);
|
||||
}
|
||||
|
||||
/* Called by l2tp_core when a session socket is being closed.
|
||||
*/
|
||||
static void pppol2tp_session_close(struct l2tp_session *session)
|
||||
{
|
||||
struct pppol2tp_session *ps;
|
||||
|
||||
ps = l2tp_session_priv(session);
|
||||
mutex_lock(&ps->sk_lock);
|
||||
ps->__sk = rcu_dereference_protected(ps->sk,
|
||||
lockdep_is_held(&ps->sk_lock));
|
||||
RCU_INIT_POINTER(ps->sk, NULL);
|
||||
if (ps->__sk)
|
||||
call_rcu(&ps->rcu, pppol2tp_put_sk);
|
||||
mutex_unlock(&ps->sk_lock);
|
||||
}
|
||||
|
||||
/* Really kill the session socket. (Called from sock_put() if
|
||||
|
@ -439,14 +457,6 @@ static void pppol2tp_session_destruct(struct sock *sk)
|
|||
}
|
||||
}
|
||||
|
||||
static void pppol2tp_put_sk(struct rcu_head *head)
|
||||
{
|
||||
struct pppol2tp_session *ps;
|
||||
|
||||
ps = container_of(head, typeof(*ps), rcu);
|
||||
sock_put(ps->__sk);
|
||||
}
|
||||
|
||||
/* Called when the PPPoX socket (session) is closed.
|
||||
*/
|
||||
static int pppol2tp_release(struct socket *sock)
|
||||
|
@ -470,26 +480,17 @@ static int pppol2tp_release(struct socket *sock)
|
|||
sock_orphan(sk);
|
||||
sock->sk = NULL;
|
||||
|
||||
/* If the socket is associated with a session,
|
||||
* l2tp_session_delete will call pppol2tp_session_close which
|
||||
* will drop the session's ref on the socket.
|
||||
*/
|
||||
session = pppol2tp_sock_to_session(sk);
|
||||
|
||||
if (session != NULL) {
|
||||
struct pppol2tp_session *ps;
|
||||
|
||||
if (session) {
|
||||
l2tp_session_delete(session);
|
||||
|
||||
ps = l2tp_session_priv(session);
|
||||
mutex_lock(&ps->sk_lock);
|
||||
ps->__sk = rcu_dereference_protected(ps->sk,
|
||||
lockdep_is_held(&ps->sk_lock));
|
||||
RCU_INIT_POINTER(ps->sk, NULL);
|
||||
mutex_unlock(&ps->sk_lock);
|
||||
call_rcu(&ps->rcu, pppol2tp_put_sk);
|
||||
|
||||
/* Rely on the sock_put() call at the end of the function for
|
||||
* dropping the reference held by pppol2tp_sock_to_session().
|
||||
* The last reference will be dropped by pppol2tp_put_sk().
|
||||
*/
|
||||
/* drop the ref obtained by pppol2tp_sock_to_session */
|
||||
sock_put(sk);
|
||||
}
|
||||
|
||||
release_sock(sk);
|
||||
|
||||
/* This will delete the session context via
|
||||
|
@ -786,6 +787,7 @@ static int pppol2tp_connect(struct socket *sock, struct sockaddr *uservaddr,
|
|||
|
||||
out_no_ppp:
|
||||
/* This is how we get the session context from the socket. */
|
||||
sock_hold(sk);
|
||||
sk->sk_user_data = session;
|
||||
rcu_assign_pointer(ps->sk, sk);
|
||||
mutex_unlock(&ps->sk_lock);
|
||||
|
|
Loading…
Reference in New Issue