lkml.org 
[lkml]   [2018]   [Jun]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    Subject[PATCH 3.16 294/410] l2tp: fix tunnel lookup use-after-free race
    3.16.57-rc1 review patch.  If anyone has any objections, please let me know.

    ------------------

    From: James Chapman <jchapman@katalix.com>

    commit 28f5bfb819195ad9c2eb9486babe7b0e4efe925f upstream.

    l2tp_tunnel_get walks the tunnel list to find a matching tunnel
    instance and if a match is found, its refcount is increased before
    returning the tunnel pointer. But when tunnel objects are destroyed,
    they are on the tunnel list after their refcount hits zero. Fix this
    by moving the code that removes the tunnel from the tunnel list from
    the tunnel socket destructor into in the l2tp_tunnel_delete path,
    before the tunnel refcount is decremented.

    refcount_t: increment on 0; use-after-free.
    WARNING: CPU: 3 PID: 13507 at lib/refcount.c:153 refcount_inc+0x47/0x50
    Modules linked in:
    CPU: 3 PID: 13507 Comm: syzbot_6e6a5ec8 Not tainted 4.16.0-rc2+ #36
    Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
    RIP: 0010:refcount_inc+0x47/0x50
    RSP: 0018:ffff8800136ffb20 EFLAGS: 00010286
    RAX: dffffc0000000008 RBX: ffff880017068e68 RCX: ffffffff814d3333
    RDX: 0000000000000000 RSI: ffff88001a59f6d8 RDI: ffff88001a59f6d8
    RBP: ffff8800136ffb28 R08: 0000000000000000 R09: 0000000000000000
    R10: ffff8800136ffab0 R11: 0000000000000000 R12: ffff880017068e50
    R13: 0000000000000000 R14: ffff8800174da800 R15: 0000000000000004
    FS: 00007f403ab1e700(0000) GS:ffff88001a580000(0000) knlGS:0000000000000000
    CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    CR2: 00000000205fafd2 CR3: 0000000016770000 CR4: 00000000000006e0
    Call Trace:
    l2tp_tunnel_get+0x2dd/0x4e0
    pppol2tp_connect+0x428/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:0x7f403a42f259
    RSP: 002b:00007f403ab1dee8 EFLAGS: 00000296 ORIG_RAX: 000000000000002a
    RAX: ffffffffffffffda RBX: 00000000205fafe4 RCX: 00007f403a42f259
    RDX: 000000000000002e RSI: 00000000205fafd2 RDI: 0000000000000004
    RBP: 00007f403ab1df20 R08: 00007f403ab1e700 R09: 0000000000000000
    R10: 00007f403ab1e700 R11: 0000000000000296 R12: 0000000000000000
    R13: 00007ffc81906cbf R14: 0000000000000000 R15: 00007f403ab2b040
    Code: 3b ff 5b 5d c3 e8 ca 5f 3b ff 80 3d 49 8e 66 04 00 75 ea e8 bc 5f 3b ff 48 c7 c7 60 69 64 85 c6 05 34 8e 66 04 01 e8 59 49 15 ff <0f> 0b eb ce 0f 1f 44 00 00 55 48 89 e5 41 56 41 55 41 54 53 49

    Fixes: f8ccac0e44934 ("l2tp: put tunnel socket release on a workqueue")
    Reported-and-tested-by: syzbot+19c09769f14b48810113@syzkaller.appspotmail.com
    Reported-and-tested-by: syzbot+347bd5acde002e353a36@syzkaller.appspotmail.com
    Reported-and-tested-by: syzbot+6e6a5ec8de31a94cd015@syzkaller.appspotmail.com
    Reported-and-tested-by: syzbot+9df43faf09bd400f2993@syzkaller.appspotmail.com
    Signed-off-by: James Chapman <jchapman@katalix.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>
    Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
    ---
    net/l2tp/l2tp_core.c | 14 +++++++-------
    1 file changed, 7 insertions(+), 7 deletions(-)

    --- a/net/l2tp/l2tp_core.c
    +++ b/net/l2tp/l2tp_core.c
    @@ -1249,7 +1249,6 @@ EXPORT_SYMBOL_GPL(l2tp_xmit_skb);
    static void l2tp_tunnel_destruct(struct sock *sk)
    {
    struct l2tp_tunnel *tunnel = l2tp_tunnel(sk);
    - struct l2tp_net *pn;

    if (tunnel == NULL)
    goto end;
    @@ -1272,12 +1271,6 @@ static void l2tp_tunnel_destruct(struct
    sk->sk_destruct = tunnel->old_sk_destruct;
    sk->sk_user_data = NULL;

    - /* Remove the tunnel struct from the tunnel list */
    - pn = l2tp_pernet(tunnel->l2tp_net);
    - spin_lock_bh(&pn->l2tp_tunnel_list_lock);
    - list_del_rcu(&tunnel->list);
    - spin_unlock_bh(&pn->l2tp_tunnel_list_lock);
    -
    /* Call the original destructor */
    if (sk->sk_destruct)
    (*sk->sk_destruct)(sk);
    @@ -1359,6 +1352,7 @@ static void l2tp_tunnel_del_work(struct
    del_work);
    struct sock *sk = tunnel->sock;
    struct socket *sock = sk->sk_socket;
    + struct l2tp_net *pn;

    l2tp_tunnel_closeall(tunnel);

    @@ -1371,6 +1365,12 @@ static void l2tp_tunnel_del_work(struct
    sk_release_kernel(sk);
    }

    + /* Remove the tunnel struct from the tunnel list */
    + pn = l2tp_pernet(tunnel->l2tp_net);
    + spin_lock_bh(&pn->l2tp_tunnel_list_lock);
    + list_del_rcu(&tunnel->list);
    + spin_unlock_bh(&pn->l2tp_tunnel_list_lock);
    +
    /* drop initial ref */
    l2tp_tunnel_dec_refcount(tunnel);

    \
     
     \ /
      Last update: 2018-06-07 16:21    [W:2.292 / U:0.440 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site