• Eric Dumazet's avatar
    tcp: tcp_release_cb() should release socket ownership · c3f9b018
    Eric Dumazet authored
    Lars Persson reported following deadlock :
    
    -000 |M:0x0:0x802B6AF8(asm) <-- arch_spin_lock
    -001 |tcp_v4_rcv(skb = 0x8BD527A0) <-- sk = 0x8BE6B2A0
    -002 |ip_local_deliver_finish(skb = 0x8BD527A0)
    -003 |__netif_receive_skb_core(skb = 0x8BD527A0, ?)
    -004 |netif_receive_skb(skb = 0x8BD527A0)
    -005 |elk_poll(napi = 0x8C770500, budget = 64)
    -006 |net_rx_action(?)
    -007 |__do_softirq()
    -008 |do_softirq()
    -009 |local_bh_enable()
    -010 |tcp_rcv_established(sk = 0x8BE6B2A0, skb = 0x87D3A9E0, th = 0x814EBE14, ?)
    -011 |tcp_v4_do_rcv(sk = 0x8BE6B2A0, skb = 0x87D3A9E0)
    -012 |tcp_delack_timer_handler(sk = 0x8BE6B2A0)
    -013 |tcp_release_cb(sk = 0x8BE6B2A0)
    -014 |release_sock(sk = 0x8BE6B2A0)
    -015 |tcp_sendmsg(?, sk = 0x8BE6B2A0, ?, ?)
    -016 |sock_sendmsg(sock = 0x8518C4C0, msg = 0x87D8DAA8, size = 4096)
    -017 |kernel_sendmsg(?, ?, ?, ?, size = 4096)
    -018 |smb_send_kvec()
    -019 |smb_send_rqst(server = 0x87C4D400, rqst = 0x87D8DBA0)
    -020 |cifs_call_async()
    -021 |cifs_async_writev(wdata = 0x87FD6580)
    -022 |cifs_writepages(mapping = 0x852096E4, wbc = 0x87D8DC88)
    -023 |__writeback_single_inode(inode = 0x852095D0, wbc = 0x87D8DC88)
    -024 |writeback_sb_inodes(sb = 0x87D6D800, wb = 0x87E4A9C0, work = 0x87D8DD88)
    -025 |__writeback_inodes_wb(wb = 0x87E4A9C0, work = 0x87D8DD88)
    -026 |wb_writeback(wb = 0x87E4A9C0, work = 0x87D8DD88)
    -027 |wb_do_writeback(wb = 0x87E4A9C0, force_wait = 0)
    -028 |bdi_writeback_workfn(work = 0x87E4A9CC)
    -029 |process_one_work(worker = 0x8B045880, work = 0x87E4A9CC)
    -030 |worker_thread(__worker = 0x8B045880)
    -031 |kthread(_create = 0x87CADD90)
    -032 |ret_from_kernel_thread(asm)
    
    Bug occurs because __tcp_checksum_complete_user() enables BH, assuming
    it is running from softirq context.
    
    Lars trace involved a NIC without RX checksum support but other points
    are problematic as well, like the prequeue stuff.
    
    Problem is triggered by a timer, that found socket being owned by user.
    
    tcp_release_cb() should call tcp_write_timer_handler() or
    tcp_delack_timer_handler() in the appropriate context :
    
    BH disabled and socket lock held, but 'owned' field cleared,
    as if they were running from timer handlers.
    
    Fixes: 6f458dfb ("tcp: improve latencies of timer triggered events")
    Reported-by: default avatarLars Persson <lars.persson@axis.com>
    Tested-by: default avatarLars Persson <lars.persson@axis.com>
    Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    c3f9b018
sock.h 63.8 KB