lkml.org 
[lkml]   [2020]   [Dec]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: WARNING in sk_stream_kill_queues (5)
    On Thu, 3 Dec 2020 at 17:27, Eric Dumazet <edumazet@google.com> wrote:
    > On Thu, Dec 3, 2020 at 4:58 PM Marco Elver <elver@google.com> wrote:
    > >
    > > On Mon, Nov 30, 2020 at 12:40AM -0800, syzbot wrote:
    > > > Hello,
    > > >
    > > > syzbot found the following issue on:
    > > >
    > > > HEAD commit: 6147c83f Add linux-next specific files for 20201126
    > > > git tree: linux-next
    > > > console output: https://syzkaller.appspot.com/x/log.txt?x=117c9679500000
    > > > kernel config: https://syzkaller.appspot.com/x/.config?x=9b91566da897c24f
    > > > dashboard link: https://syzkaller.appspot.com/bug?extid=7b99aafdcc2eedea6178
    > > > compiler: gcc (GCC) 10.1.0-syz 20200507
    > > > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=103bf743500000
    > > > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=167c60c9500000
    > > >
    > > > The issue was bisected to:
    > > >
    > > > commit 145cd60fb481328faafba76842aa0fd242e2b163
    > > > Author: Alexander Potapenko <glider@google.com>
    > > > Date: Tue Nov 24 05:38:44 2020 +0000
    > > >
    > > > mm, kfence: insert KFENCE hooks for SLUB
    > > >
    > > > bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=13abe5b3500000
    > > > final oops: https://syzkaller.appspot.com/x/report.txt?x=106be5b3500000
    > > > console output: https://syzkaller.appspot.com/x/log.txt?x=17abe5b3500000
    > > >
    > > > IMPORTANT: if you fix the issue, please add the following tag to the commit:
    > > > Reported-by: syzbot+7b99aafdcc2eedea6178@syzkaller.appspotmail.com
    > > > Fixes: 145cd60fb481 ("mm, kfence: insert KFENCE hooks for SLUB")
    > > >
    > > > ------------[ cut here ]------------
    > > > WARNING: CPU: 0 PID: 11307 at net/core/stream.c:207 sk_stream_kill_queues+0x3c3/0x530 net/core/stream.c:207
    > > [...]
    > > > Call Trace:
    > > > inet_csk_destroy_sock+0x1a5/0x490 net/ipv4/inet_connection_sock.c:885
    > > > __tcp_close+0xd3e/0x1170 net/ipv4/tcp.c:2585
    > > > tcp_close+0x29/0xc0 net/ipv4/tcp.c:2597
    > > > inet_release+0x12e/0x280 net/ipv4/af_inet.c:431
    > > > __sock_release+0xcd/0x280 net/socket.c:596
    > > > sock_close+0x18/0x20 net/socket.c:1255
    > > > __fput+0x283/0x920 fs/file_table.c:280
    > > > task_work_run+0xdd/0x190 kernel/task_work.c:140
    > > > exit_task_work include/linux/task_work.h:30 [inline]
    > > > do_exit+0xb89/0x29e0 kernel/exit.c:823
    > > > do_group_exit+0x125/0x310 kernel/exit.c:920
    > > > get_signal+0x3ec/0x2010 kernel/signal.c:2770
    > > > arch_do_signal_or_restart+0x2a8/0x1eb0 arch/x86/kernel/signal.c:811
    > > > handle_signal_work kernel/entry/common.c:144 [inline]
    > > > exit_to_user_mode_loop kernel/entry/common.c:168 [inline]
    > > > exit_to_user_mode_prepare+0x124/0x200 kernel/entry/common.c:198
    > > > syscall_exit_to_user_mode+0x36/0x260 kernel/entry/common.c:275
    > > > entry_SYSCALL_64_after_hwframe+0x44/0xa9
    > >
    > > I've been debugging this and I think enabling KFENCE uncovered that some
    > > code is assuming that the following is always true:
    > >
    > > ksize(kmalloc(S)) == ksize(kmalloc(S))
    > >
    >
    >
    > I do not think we make this assumption.
    >
    > Each skb tracks the 'truesize' which is populated from __alloc_skb()
    > using ksize(allocated head) .
    >
    > So if ksize() decides to give us random data, it should be still fine,
    > because we use ksize(buff) only once at alloc skb time, and record the
    > value in skb->truesize
    > (only the socket buffer accounting would be off)

    Good, thanks for clarifying. So something else must be off then.

    > > but I don't think this assumption can be made (with or without KFENCE).
    > >
    > > With KFENCE, we actually end up testing no code assumes this, because
    > > KFENCE's ksize() always returns the exact size S.
    > >
    > > I have narrowed it down to sk_wmem_queued becoming <0 in
    > > sk_wmem_free_skb().
    > >
    > > The skb passed to sk_wmem_free_skb() and whose truesize causes
    > > sk_wmem_queued to become negative is always allocated in:
    > >
    > > | kmem_cache_alloc_node+0x140/0x400 mm/slub.c:2939
    > > | __alloc_skb+0x6d/0x710 net/core/skbuff.c:198
    > > | alloc_skb_fclone include/linux/skbuff.h:1144 [inline]
    > > | sk_stream_alloc_skb+0x109/0xc30 net/ipv4/tcp.c:888
    > > | tso_fragment net/ipv4/tcp_output.c:2124 [inline]
    > > | tcp_write_xmit+0x1dbf/0x5ce0 net/ipv4/tcp_output.c:2674
    > > | __tcp_push_pending_frames+0xaa/0x390 net/ipv4/tcp_output.c:2866
    > > | tcp_push_pending_frames include/net/tcp.h:1864 [inline]
    > > | tcp_data_snd_check net/ipv4/tcp_input.c:5374 [inline]
    > > | tcp_rcv_established+0x8c9/0x1eb0 net/ipv4/tcp_input.c:5869
    > > | tcp_v4_do_rcv+0x5d1/0x870 net/ipv4/tcp_ipv4.c:1668
    > > | sk_backlog_rcv include/net/sock.h:1011 [inline]
    > > | __release_sock+0x134/0x3a0 net/core/sock.c:2523
    > > | release_sock+0x54/0x1b0 net/core/sock.c:3053
    > > | sk_wait_data+0x177/0x450 net/core/sock.c:2565
    > > | tcp_recvmsg+0x17ea/0x2aa0 net/ipv4/tcp.c:2181
    > > | inet_recvmsg+0x11b/0x5d0 net/ipv4/af_inet.c:848
    > > | sock_recvmsg_nosec net/socket.c:885 [inline]
    > > | sock_recvmsg net/socket.c:903 [inline]
    > > | sock_recvmsg net/socket.c:899 [inline]
    > > | ____sys_recvmsg+0x2c4/0x600 net/socket.c:2563
    > > | ___sys_recvmsg+0x127/0x200 net/socket.c:2605
    > > | __sys_recvmsg+0xe2/0x1a0 net/socket.c:2641
    > > | do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
    > > | entry_SYSCALL_64_after_hwframe+0x44/0xa9
    > >
    > > I used the code below to add some warnings that helped narrow it down.
    > >
    > > Does any of this help explain the problem?
    >
    > Not yet :)

    Damn...

    > tso_fragment() transfers some payload from one skb to another, and
    > properly shifts this amount from src skb to dst skb (@buff) :
    >
    > sk_wmem_queued_add(sk, buff->truesize);
    > buff->truesize += nlen;
    > skb->truesize -= nlen;

    Any other stacktraces that might help?

    Thanks,
    -- Marco

    \
     
     \ /
      Last update: 2020-12-03 17:39    [W:3.514 / U:0.000 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site