lkml.org 
[lkml]   [2019]   [Feb]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    Subject[PATCH 3.16 109/305] llc: do not use sk_eat_skb()
    3.16.63-rc1 review patch.  If anyone has any objections, please let me know.

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

    From: Eric Dumazet <edumazet@google.com>

    commit 604d415e2bd642b7e02c80e719e0396b9d4a77a6 upstream.

    syzkaller triggered a use-after-free [1], caused by a combination of
    skb_get() in llc_conn_state_process() and usage of sk_eat_skb()

    sk_eat_skb() is assuming the skb about to be freed is only used by
    the current thread. TCP/DCCP stacks enforce this because current
    thread holds the socket lock.

    llc_conn_state_process() wants to make sure skb does not disappear,
    and holds a reference on the skb it manipulates. But as soon as this
    skb is added to socket receive queue, another thread can consume it.

    This means that llc must use regular skb_unlink() and kfree_skb()
    so that both producer and consumer can safely work on the same skb.

    [1]
    BUG: KASAN: use-after-free in atomic_read include/asm-generic/atomic-instrumented.h:21 [inline]
    BUG: KASAN: use-after-free in refcount_read include/linux/refcount.h:43 [inline]
    BUG: KASAN: use-after-free in skb_unref include/linux/skbuff.h:967 [inline]
    BUG: KASAN: use-after-free in kfree_skb+0xb7/0x580 net/core/skbuff.c:655
    Read of size 4 at addr ffff8801d1f6fba4 by task ksoftirqd/1/18

    CPU: 1 PID: 18 Comm: ksoftirqd/1 Not tainted 4.19.0-rc8+ #295
    Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
    Call Trace:
    __dump_stack lib/dump_stack.c:77 [inline]
    dump_stack+0x1c4/0x2b6 lib/dump_stack.c:113
    print_address_description.cold.8+0x9/0x1ff mm/kasan/report.c:256
    kasan_report_error mm/kasan/report.c:354 [inline]
    kasan_report.cold.9+0x242/0x309 mm/kasan/report.c:412
    check_memory_region_inline mm/kasan/kasan.c:260 [inline]
    check_memory_region+0x13e/0x1b0 mm/kasan/kasan.c:267
    kasan_check_read+0x11/0x20 mm/kasan/kasan.c:272
    atomic_read include/asm-generic/atomic-instrumented.h:21 [inline]
    refcount_read include/linux/refcount.h:43 [inline]
    skb_unref include/linux/skbuff.h:967 [inline]
    kfree_skb+0xb7/0x580 net/core/skbuff.c:655
    llc_sap_state_process+0x9b/0x550 net/llc/llc_sap.c:224
    llc_sap_rcv+0x156/0x1f0 net/llc/llc_sap.c:297
    llc_sap_handler+0x65e/0xf80 net/llc/llc_sap.c:438
    llc_rcv+0x79e/0xe20 net/llc/llc_input.c:208
    __netif_receive_skb_one_core+0x14d/0x200 net/core/dev.c:4913
    __netif_receive_skb+0x2c/0x1e0 net/core/dev.c:5023
    process_backlog+0x218/0x6f0 net/core/dev.c:5829
    napi_poll net/core/dev.c:6249 [inline]
    net_rx_action+0x7c5/0x1950 net/core/dev.c:6315
    __do_softirq+0x30c/0xb03 kernel/softirq.c:292
    run_ksoftirqd+0x94/0x100 kernel/softirq.c:653
    smpboot_thread_fn+0x68b/0xa00 kernel/smpboot.c:164
    kthread+0x35a/0x420 kernel/kthread.c:246
    ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:413

    Allocated by task 18:
    save_stack+0x43/0xd0 mm/kasan/kasan.c:448
    set_track mm/kasan/kasan.c:460 [inline]
    kasan_kmalloc+0xc7/0xe0 mm/kasan/kasan.c:553
    kasan_slab_alloc+0x12/0x20 mm/kasan/kasan.c:490
    kmem_cache_alloc_node+0x144/0x730 mm/slab.c:3644
    __alloc_skb+0x119/0x770 net/core/skbuff.c:193
    alloc_skb include/linux/skbuff.h:995 [inline]
    llc_alloc_frame+0xbc/0x370 net/llc/llc_sap.c:54
    llc_station_ac_send_xid_r net/llc/llc_station.c:52 [inline]
    llc_station_rcv+0x1dc/0x1420 net/llc/llc_station.c:111
    llc_rcv+0xc32/0xe20 net/llc/llc_input.c:220
    __netif_receive_skb_one_core+0x14d/0x200 net/core/dev.c:4913
    __netif_receive_skb+0x2c/0x1e0 net/core/dev.c:5023
    process_backlog+0x218/0x6f0 net/core/dev.c:5829
    napi_poll net/core/dev.c:6249 [inline]
    net_rx_action+0x7c5/0x1950 net/core/dev.c:6315
    __do_softirq+0x30c/0xb03 kernel/softirq.c:292

    Freed by task 16383:
    save_stack+0x43/0xd0 mm/kasan/kasan.c:448
    set_track mm/kasan/kasan.c:460 [inline]
    __kasan_slab_free+0x102/0x150 mm/kasan/kasan.c:521
    kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
    __cache_free mm/slab.c:3498 [inline]
    kmem_cache_free+0x83/0x290 mm/slab.c:3756
    kfree_skbmem+0x154/0x230 net/core/skbuff.c:582
    __kfree_skb+0x1d/0x20 net/core/skbuff.c:642
    sk_eat_skb include/net/sock.h:2366 [inline]
    llc_ui_recvmsg+0xec2/0x1610 net/llc/af_llc.c:882
    sock_recvmsg_nosec net/socket.c:794 [inline]
    sock_recvmsg+0xd0/0x110 net/socket.c:801
    ___sys_recvmsg+0x2b6/0x680 net/socket.c:2278
    __sys_recvmmsg+0x303/0xb90 net/socket.c:2390
    do_sys_recvmmsg+0x181/0x1a0 net/socket.c:2466
    __do_sys_recvmmsg net/socket.c:2484 [inline]
    __se_sys_recvmmsg net/socket.c:2480 [inline]
    __x64_sys_recvmmsg+0xbe/0x150 net/socket.c:2480
    do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
    entry_SYSCALL_64_after_hwframe+0x49/0xbe

    The buggy address belongs to the object at ffff8801d1f6fac0
    which belongs to the cache skbuff_head_cache of size 232
    The buggy address is located 228 bytes inside of
    232-byte region [ffff8801d1f6fac0, ffff8801d1f6fba8)
    The buggy address belongs to the page:
    page:ffffea000747dbc0 count:1 mapcount:0 mapping:ffff8801d9be7680 index:0xffff8801d1f6fe80
    flags: 0x2fffc0000000100(slab)
    raw: 02fffc0000000100 ffffea0007346e88 ffffea000705b108 ffff8801d9be7680
    raw: ffff8801d1f6fe80 ffff8801d1f6f0c0 000000010000000b 0000000000000000
    page dumped because: kasan: bad access detected

    Memory state around the buggy address:
    ffff8801d1f6fa80: fc fc fc fc fc fc fc fc fb fb fb fb fb fb fb fb
    ffff8801d1f6fb00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
    >ffff8801d1f6fb80: fb fb fb fb fb fc fc fc fc fc fc fc fc fc fc fc
    ^
    ffff8801d1f6fc00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
    ffff8801d1f6fc80: fb fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc

    Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
    Signed-off-by: Eric Dumazet <edumazet@google.com>
    Reported-by: syzbot <syzkaller@googlegroups.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>
    [bwh: Backported to 3.16:
    - sk_eat_skb() takes a third parameter here
    - Adjust indentation]
    Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
    ---
    net/llc/af_llc.c | 11 ++++-------
    1 file changed, 4 insertions(+), 7 deletions(-)

    --- a/net/llc/af_llc.c
    +++ b/net/llc/af_llc.c
    @@ -723,7 +723,6 @@ static int llc_ui_recvmsg(struct kiocb *
    struct sk_buff *skb = NULL;
    struct sock *sk = sock->sk;
    struct llc_sock *llc = llc_sk(sk);
    - unsigned long cpu_flags;
    size_t copied = 0;
    u32 peek_seq = 0;
    u32 *seq, skb_len;
    @@ -849,9 +848,8 @@ static int llc_ui_recvmsg(struct kiocb *
    goto copy_uaddr;

    if (!(flags & MSG_PEEK)) {
    - spin_lock_irqsave(&sk->sk_receive_queue.lock, cpu_flags);
    - sk_eat_skb(sk, skb, false);
    - spin_unlock_irqrestore(&sk->sk_receive_queue.lock, cpu_flags);
    + skb_unlink(skb, &sk->sk_receive_queue);
    + kfree_skb(skb);
    *seq = 0;
    }

    @@ -872,10 +870,9 @@ copy_uaddr:
    llc_cmsg_rcv(msg, skb);

    if (!(flags & MSG_PEEK)) {
    - spin_lock_irqsave(&sk->sk_receive_queue.lock, cpu_flags);
    - sk_eat_skb(sk, skb, false);
    - spin_unlock_irqrestore(&sk->sk_receive_queue.lock, cpu_flags);
    - *seq = 0;
    + skb_unlink(skb, &sk->sk_receive_queue);
    + kfree_skb(skb);
    + *seq = 0;
    }

    goto out;
    \
     
     \ /
      Last update: 2019-02-03 15:01    [W:4.394 / U:0.092 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site