Re: possible deadlock in skb_queue_tail

2018-04-04 Thread Dmitry Vyukov 
On Wed, Apr 4, 2018 at 7:08 AM, Cong Wang  wrote:
> On Tue, Apr 3, 2018 at 4:42 AM, Kirill Tkhai  wrote:
>> On 03.04.2018 14:25, Dmitry Vyukov wrote:
>>> On Tue, Apr 3, 2018 at 11:50 AM, Kirill Tkhai  wrote:
 sk_diag_dump_icons() dumps only sockets in TCP_LISTEN state.
 TCP_LISTEN state may be assigned in only place in net/unix/af_unix.c:
 it's unix_listen(). The function is applied to stream and seqpacket
 socket types.

 It can't be stream because of the second stack, and seqpacket also can't,
 as I don't think it's possible for gcc to inline unix_seqpacket_sendmsg()
 in the way, we don't see it in the stack.

 So, this is looks like false positive result for me.

 Kirill
>>>
>>> Do you mean that these &(>lock)->rlock/1 referenced in 2 stacks are
>>> always different?
>>
>> In these 2 particular stacks they have to be different.
>
> So actually my patch could fix this false positive? I thought it couldn't.
> https://patchwork.ozlabs.org/patch/894342/

You know better!
If you suspect it can fix this report, and nobody has better
proposals, then we can just mark this as being fixed with your commit
and then see if it triggers again with your commit or not.

Re: possible deadlock in skb_queue_tail

2018-04-04 Thread Dmitry Vyukov 
On Wed, Apr 4, 2018 at 7:08 AM, Cong Wang  wrote:
> On Tue, Apr 3, 2018 at 4:42 AM, Kirill Tkhai  wrote:
>> On 03.04.2018 14:25, Dmitry Vyukov wrote:
>>> On Tue, Apr 3, 2018 at 11:50 AM, Kirill Tkhai  wrote:
 sk_diag_dump_icons() dumps only sockets in TCP_LISTEN state.
 TCP_LISTEN state may be assigned in only place in net/unix/af_unix.c:
 it's unix_listen(). The function is applied to stream and seqpacket
 socket types.

 It can't be stream because of the second stack, and seqpacket also can't,
 as I don't think it's possible for gcc to inline unix_seqpacket_sendmsg()
 in the way, we don't see it in the stack.

 So, this is looks like false positive result for me.

 Kirill
>>>
>>> Do you mean that these &(>lock)->rlock/1 referenced in 2 stacks are
>>> always different?
>>
>> In these 2 particular stacks they have to be different.
>
> So actually my patch could fix this false positive? I thought it couldn't.
> https://patchwork.ozlabs.org/patch/894342/

You know better!
If you suspect it can fix this report, and nobody has better
proposals, then we can just mark this as being fixed with your commit
and then see if it triggers again with your commit or not.

Re: possible deadlock in skb_queue_tail

2018-04-03 Thread Cong Wang 
On Tue, Apr 3, 2018 at 4:42 AM, Kirill Tkhai  wrote:
> On 03.04.2018 14:25, Dmitry Vyukov wrote:
>> On Tue, Apr 3, 2018 at 11:50 AM, Kirill Tkhai  wrote:
>>> sk_diag_dump_icons() dumps only sockets in TCP_LISTEN state.
>>> TCP_LISTEN state may be assigned in only place in net/unix/af_unix.c:
>>> it's unix_listen(). The function is applied to stream and seqpacket
>>> socket types.
>>>
>>> It can't be stream because of the second stack, and seqpacket also can't,
>>> as I don't think it's possible for gcc to inline unix_seqpacket_sendmsg()
>>> in the way, we don't see it in the stack.
>>>
>>> So, this is looks like false positive result for me.
>>>
>>> Kirill
>>
>> Do you mean that these &(>lock)->rlock/1 referenced in 2 stacks are
>> always different?
>
> In these 2 particular stacks they have to be different.

So actually my patch could fix this false positive? I thought it couldn't.
https://patchwork.ozlabs.org/patch/894342/

Re: possible deadlock in skb_queue_tail

2018-04-03 Thread Cong Wang 
On Tue, Apr 3, 2018 at 4:42 AM, Kirill Tkhai  wrote:
> On 03.04.2018 14:25, Dmitry Vyukov wrote:
>> On Tue, Apr 3, 2018 at 11:50 AM, Kirill Tkhai  wrote:
>>> sk_diag_dump_icons() dumps only sockets in TCP_LISTEN state.
>>> TCP_LISTEN state may be assigned in only place in net/unix/af_unix.c:
>>> it's unix_listen(). The function is applied to stream and seqpacket
>>> socket types.
>>>
>>> It can't be stream because of the second stack, and seqpacket also can't,
>>> as I don't think it's possible for gcc to inline unix_seqpacket_sendmsg()
>>> in the way, we don't see it in the stack.
>>>
>>> So, this is looks like false positive result for me.
>>>
>>> Kirill
>>
>> Do you mean that these &(>lock)->rlock/1 referenced in 2 stacks are
>> always different?
>
> In these 2 particular stacks they have to be different.

So actually my patch could fix this false positive? I thought it couldn't.
https://patchwork.ozlabs.org/patch/894342/

Re: possible deadlock in skb_queue_tail

2018-04-03 Thread Kirill Tkhai 
On 03.04.2018 14:25, Dmitry Vyukov wrote:
> On Tue, Apr 3, 2018 at 11:50 AM, Kirill Tkhai  wrote:
>> On 02.04.2018 12:20, syzbot wrote:
>>> Hello,
>>>
>>> syzbot hit the following crash on net-next commit
>>> 06b19fe9a6df7aaa423cd8404ebe5ac9ec4b2960 (Sun Apr 1 03:37:33 2018 +)
>>> Merge branch 'chelsio-inline-tls'
>>> syzbot dashboard link: 
>>> https://syzkaller.appspot.com/bug?extid=6b495100f17ca8554ab9
>>>
>>> Unfortunately, I don't have any reproducer for this crash yet.
>>> Raw console output: 
>>> https://syzkaller.appspot.com/x/log.txt?id=6218830443446272
>>> Kernel config: 
>>> https://syzkaller.appspot.com/x/.config?id=3327544840960562528
>>> compiler: gcc (GCC) 7.1.1 20170620
>>>
>>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>>> Reported-by: syzbot+6b495100f17ca8554...@syzkaller.appspotmail.com
>>> It will help syzbot understand when the bug is fixed. See footer for 
>>> details.
>>> If you forward the report, please keep this part and the footer.
>>>
>>>
>>> ==
>>> WARNING: possible circular locking dependency detected
>>> 4.16.0-rc6+ #290 Not tainted
>>> --
>>> syz-executor7/20971 is trying to acquire lock:
>>>  (_unix_sk_receive_queue_lock_key){+.+.}, at: [<271ef0d8>] 
>>> skb_queue_tail+0x26/0x150 net/core/skbuff.c:2899
>>>
>>> but task is already holding lock:
>>>  (&(>lock)->rlock/1){+.+.}, at: [<4e725e14>] 
>>> unix_state_double_lock+0x7b/0xb0 net/unix/af_unix.c:1088
>>>
>>> which lock already depends on the new lock.
>>>
>>>
>>> the existing dependency chain (in reverse order) is:
>>>
>>> -> #1 (&(>lock)->rlock/1){+.+.}:
>>>_raw_spin_lock_nested+0x28/0x40 kernel/locking/spinlock.c:354
>>>sk_diag_dump_icons net/unix/diag.c:82 [inline]
>>>sk_diag_fill.isra.4+0xa52/0xfe0 net/unix/diag.c:144
>>>sk_diag_dump net/unix/diag.c:178 [inline]
>>>unix_diag_dump+0x400/0x4f0 net/unix/diag.c:206
>>>netlink_dump+0x492/0xcf0 net/netlink/af_netlink.c:2221
>>>__netlink_dump_start+0x4ec/0x710 net/netlink/af_netlink.c:2318
>>>netlink_dump_start include/linux/netlink.h:214 [inline]
>>>unix_diag_handler_dump+0x3e7/0x750 net/unix/diag.c:307
>>>__sock_diag_cmd net/core/sock_diag.c:230 [inline]
>>>sock_diag_rcv_msg+0x204/0x360 net/core/sock_diag.c:261
>>>netlink_rcv_skb+0x14b/0x380 net/netlink/af_netlink.c:2443
>>>sock_diag_rcv+0x2a/0x40 net/core/sock_diag.c:272
>>>netlink_unicast_kernel net/netlink/af_netlink.c:1307 [inline]
>>>netlink_unicast+0x4c4/0x6b0 net/netlink/af_netlink.c:1333
>>>netlink_sendmsg+0xa4a/0xe80 net/netlink/af_netlink.c:1896
>>>sock_sendmsg_nosec net/socket.c:629 [inline]
>>>sock_sendmsg+0xca/0x110 net/socket.c:639
>>>sock_write_iter+0x31a/0x5d0 net/socket.c:908
>>>call_write_iter include/linux/fs.h:1782 [inline]
>>>new_sync_write fs/read_write.c:469 [inline]
>>>__vfs_write+0x684/0x970 fs/read_write.c:482
>>>vfs_write+0x189/0x510 fs/read_write.c:544
>>>SYSC_write fs/read_write.c:589 [inline]
>>>SyS_write+0xef/0x220 fs/read_write.c:581
>>>do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
>>>entry_SYSCALL_64_after_hwframe+0x42/0xb7
>>>
>>> -> #0 (_unix_sk_receive_queue_lock_key){+.+.}:
>>>lock_acquire+0x1d5/0x580 kernel/locking/lockdep.c:3920
>>>__raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
>>>_raw_spin_lock_irqsave+0x96/0xc0 kernel/locking/spinlock.c:152
>>>skb_queue_tail+0x26/0x150 net/core/skbuff.c:2899
>>>unix_dgram_sendmsg+0xa30/0x1610 net/unix/af_unix.c:1807
>>>sock_sendmsg_nosec net/socket.c:629 [inline]
>>>sock_sendmsg+0xca/0x110 net/socket.c:639
>>>___sys_sendmsg+0x320/0x8b0 net/socket.c:2047
>>>__sys_sendmmsg+0x1ee/0x620 net/socket.c:2137
>>>SYSC_sendmmsg net/socket.c:2168 [inline]
>>>SyS_sendmmsg+0x35/0x60 net/socket.c:2163
>>>do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
>>>entry_SYSCALL_64_after_hwframe+0x42/0xb7
>>
>> sk_diag_dump_icons() dumps only sockets in TCP_LISTEN state.
>> TCP_LISTEN state may be assigned in only place in net/unix/af_unix.c:
>> it's unix_listen(). The function is applied to stream and seqpacket
>> socket types.
>>
>> It can't be stream because of the second stack, and seqpacket also can't,
>> as I don't think it's possible for gcc to inline unix_seqpacket_sendmsg()
>> in the way, we don't see it in the stack.
>>
>> So, this is looks like false positive result for me.
>>
>> Kirill
> 
> Do you mean that these &(>lock)->rlock/1 referenced in 2 stacks are
> always different?

In these 2 particular stacks they have to be different.

But we may meet another stacks, where stream or seqpacket
functions are used instead of

Re: possible deadlock in skb_queue_tail

2018-04-03 Thread Kirill Tkhai 
On 03.04.2018 14:25, Dmitry Vyukov wrote:
> On Tue, Apr 3, 2018 at 11:50 AM, Kirill Tkhai  wrote:
>> On 02.04.2018 12:20, syzbot wrote:
>>> Hello,
>>>
>>> syzbot hit the following crash on net-next commit
>>> 06b19fe9a6df7aaa423cd8404ebe5ac9ec4b2960 (Sun Apr 1 03:37:33 2018 +)
>>> Merge branch 'chelsio-inline-tls'
>>> syzbot dashboard link: 
>>> https://syzkaller.appspot.com/bug?extid=6b495100f17ca8554ab9
>>>
>>> Unfortunately, I don't have any reproducer for this crash yet.
>>> Raw console output: 
>>> https://syzkaller.appspot.com/x/log.txt?id=6218830443446272
>>> Kernel config: 
>>> https://syzkaller.appspot.com/x/.config?id=3327544840960562528
>>> compiler: gcc (GCC) 7.1.1 20170620
>>>
>>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>>> Reported-by: syzbot+6b495100f17ca8554...@syzkaller.appspotmail.com
>>> It will help syzbot understand when the bug is fixed. See footer for 
>>> details.
>>> If you forward the report, please keep this part and the footer.
>>>
>>>
>>> ==
>>> WARNING: possible circular locking dependency detected
>>> 4.16.0-rc6+ #290 Not tainted
>>> --
>>> syz-executor7/20971 is trying to acquire lock:
>>>  (_unix_sk_receive_queue_lock_key){+.+.}, at: [<271ef0d8>] 
>>> skb_queue_tail+0x26/0x150 net/core/skbuff.c:2899
>>>
>>> but task is already holding lock:
>>>  (&(>lock)->rlock/1){+.+.}, at: [<4e725e14>] 
>>> unix_state_double_lock+0x7b/0xb0 net/unix/af_unix.c:1088
>>>
>>> which lock already depends on the new lock.
>>>
>>>
>>> the existing dependency chain (in reverse order) is:
>>>
>>> -> #1 (&(>lock)->rlock/1){+.+.}:
>>>_raw_spin_lock_nested+0x28/0x40 kernel/locking/spinlock.c:354
>>>sk_diag_dump_icons net/unix/diag.c:82 [inline]
>>>sk_diag_fill.isra.4+0xa52/0xfe0 net/unix/diag.c:144
>>>sk_diag_dump net/unix/diag.c:178 [inline]
>>>unix_diag_dump+0x400/0x4f0 net/unix/diag.c:206
>>>netlink_dump+0x492/0xcf0 net/netlink/af_netlink.c:2221
>>>__netlink_dump_start+0x4ec/0x710 net/netlink/af_netlink.c:2318
>>>netlink_dump_start include/linux/netlink.h:214 [inline]
>>>unix_diag_handler_dump+0x3e7/0x750 net/unix/diag.c:307
>>>__sock_diag_cmd net/core/sock_diag.c:230 [inline]
>>>sock_diag_rcv_msg+0x204/0x360 net/core/sock_diag.c:261
>>>netlink_rcv_skb+0x14b/0x380 net/netlink/af_netlink.c:2443
>>>sock_diag_rcv+0x2a/0x40 net/core/sock_diag.c:272
>>>netlink_unicast_kernel net/netlink/af_netlink.c:1307 [inline]
>>>netlink_unicast+0x4c4/0x6b0 net/netlink/af_netlink.c:1333
>>>netlink_sendmsg+0xa4a/0xe80 net/netlink/af_netlink.c:1896
>>>sock_sendmsg_nosec net/socket.c:629 [inline]
>>>sock_sendmsg+0xca/0x110 net/socket.c:639
>>>sock_write_iter+0x31a/0x5d0 net/socket.c:908
>>>call_write_iter include/linux/fs.h:1782 [inline]
>>>new_sync_write fs/read_write.c:469 [inline]
>>>__vfs_write+0x684/0x970 fs/read_write.c:482
>>>vfs_write+0x189/0x510 fs/read_write.c:544
>>>SYSC_write fs/read_write.c:589 [inline]
>>>SyS_write+0xef/0x220 fs/read_write.c:581
>>>do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
>>>entry_SYSCALL_64_after_hwframe+0x42/0xb7
>>>
>>> -> #0 (_unix_sk_receive_queue_lock_key){+.+.}:
>>>lock_acquire+0x1d5/0x580 kernel/locking/lockdep.c:3920
>>>__raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
>>>_raw_spin_lock_irqsave+0x96/0xc0 kernel/locking/spinlock.c:152
>>>skb_queue_tail+0x26/0x150 net/core/skbuff.c:2899
>>>unix_dgram_sendmsg+0xa30/0x1610 net/unix/af_unix.c:1807
>>>sock_sendmsg_nosec net/socket.c:629 [inline]
>>>sock_sendmsg+0xca/0x110 net/socket.c:639
>>>___sys_sendmsg+0x320/0x8b0 net/socket.c:2047
>>>__sys_sendmmsg+0x1ee/0x620 net/socket.c:2137
>>>SYSC_sendmmsg net/socket.c:2168 [inline]
>>>SyS_sendmmsg+0x35/0x60 net/socket.c:2163
>>>do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
>>>entry_SYSCALL_64_after_hwframe+0x42/0xb7
>>
>> sk_diag_dump_icons() dumps only sockets in TCP_LISTEN state.
>> TCP_LISTEN state may be assigned in only place in net/unix/af_unix.c:
>> it's unix_listen(). The function is applied to stream and seqpacket
>> socket types.
>>
>> It can't be stream because of the second stack, and seqpacket also can't,
>> as I don't think it's possible for gcc to inline unix_seqpacket_sendmsg()
>> in the way, we don't see it in the stack.
>>
>> So, this is looks like false positive result for me.
>>
>> Kirill
> 
> Do you mean that these &(>lock)->rlock/1 referenced in 2 stacks are
> always different?

In these 2 particular stacks they have to be different.

But we may meet another stacks, where stream or seqpacket
functions are used instead of unix_dgram_sendmsg(), and
they may be

Re: possible deadlock in skb_queue_tail

2018-04-03 Thread Dmitry Vyukov 
On Tue, Apr 3, 2018 at 11:50 AM, Kirill Tkhai  wrote:
> On 02.04.2018 12:20, syzbot wrote:
>> Hello,
>>
>> syzbot hit the following crash on net-next commit
>> 06b19fe9a6df7aaa423cd8404ebe5ac9ec4b2960 (Sun Apr 1 03:37:33 2018 +)
>> Merge branch 'chelsio-inline-tls'
>> syzbot dashboard link: 
>> https://syzkaller.appspot.com/bug?extid=6b495100f17ca8554ab9
>>
>> Unfortunately, I don't have any reproducer for this crash yet.
>> Raw console output: 
>> https://syzkaller.appspot.com/x/log.txt?id=6218830443446272
>> Kernel config: https://syzkaller.appspot.com/x/.config?id=3327544840960562528
>> compiler: gcc (GCC) 7.1.1 20170620
>>
>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>> Reported-by: syzbot+6b495100f17ca8554...@syzkaller.appspotmail.com
>> It will help syzbot understand when the bug is fixed. See footer for details.
>> If you forward the report, please keep this part and the footer.
>>
>>
>> ==
>> WARNING: possible circular locking dependency detected
>> 4.16.0-rc6+ #290 Not tainted
>> --
>> syz-executor7/20971 is trying to acquire lock:
>>  (_unix_sk_receive_queue_lock_key){+.+.}, at: [<271ef0d8>] 
>> skb_queue_tail+0x26/0x150 net/core/skbuff.c:2899
>>
>> but task is already holding lock:
>>  (&(>lock)->rlock/1){+.+.}, at: [<4e725e14>] 
>> unix_state_double_lock+0x7b/0xb0 net/unix/af_unix.c:1088
>>
>> which lock already depends on the new lock.
>>
>>
>> the existing dependency chain (in reverse order) is:
>>
>> -> #1 (&(>lock)->rlock/1){+.+.}:
>>_raw_spin_lock_nested+0x28/0x40 kernel/locking/spinlock.c:354
>>sk_diag_dump_icons net/unix/diag.c:82 [inline]
>>sk_diag_fill.isra.4+0xa52/0xfe0 net/unix/diag.c:144
>>sk_diag_dump net/unix/diag.c:178 [inline]
>>unix_diag_dump+0x400/0x4f0 net/unix/diag.c:206
>>netlink_dump+0x492/0xcf0 net/netlink/af_netlink.c:2221
>>__netlink_dump_start+0x4ec/0x710 net/netlink/af_netlink.c:2318
>>netlink_dump_start include/linux/netlink.h:214 [inline]
>>unix_diag_handler_dump+0x3e7/0x750 net/unix/diag.c:307
>>__sock_diag_cmd net/core/sock_diag.c:230 [inline]
>>sock_diag_rcv_msg+0x204/0x360 net/core/sock_diag.c:261
>>netlink_rcv_skb+0x14b/0x380 net/netlink/af_netlink.c:2443
>>sock_diag_rcv+0x2a/0x40 net/core/sock_diag.c:272
>>netlink_unicast_kernel net/netlink/af_netlink.c:1307 [inline]
>>netlink_unicast+0x4c4/0x6b0 net/netlink/af_netlink.c:1333
>>netlink_sendmsg+0xa4a/0xe80 net/netlink/af_netlink.c:1896
>>sock_sendmsg_nosec net/socket.c:629 [inline]
>>sock_sendmsg+0xca/0x110 net/socket.c:639
>>sock_write_iter+0x31a/0x5d0 net/socket.c:908
>>call_write_iter include/linux/fs.h:1782 [inline]
>>new_sync_write fs/read_write.c:469 [inline]
>>__vfs_write+0x684/0x970 fs/read_write.c:482
>>vfs_write+0x189/0x510 fs/read_write.c:544
>>SYSC_write fs/read_write.c:589 [inline]
>>SyS_write+0xef/0x220 fs/read_write.c:581
>>do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
>>entry_SYSCALL_64_after_hwframe+0x42/0xb7
>>
>> -> #0 (_unix_sk_receive_queue_lock_key){+.+.}:
>>lock_acquire+0x1d5/0x580 kernel/locking/lockdep.c:3920
>>__raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
>>_raw_spin_lock_irqsave+0x96/0xc0 kernel/locking/spinlock.c:152
>>skb_queue_tail+0x26/0x150 net/core/skbuff.c:2899
>>unix_dgram_sendmsg+0xa30/0x1610 net/unix/af_unix.c:1807
>>sock_sendmsg_nosec net/socket.c:629 [inline]
>>sock_sendmsg+0xca/0x110 net/socket.c:639
>>___sys_sendmsg+0x320/0x8b0 net/socket.c:2047
>>__sys_sendmmsg+0x1ee/0x620 net/socket.c:2137
>>SYSC_sendmmsg net/socket.c:2168 [inline]
>>SyS_sendmmsg+0x35/0x60 net/socket.c:2163
>>do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
>>entry_SYSCALL_64_after_hwframe+0x42/0xb7
>
> sk_diag_dump_icons() dumps only sockets in TCP_LISTEN state.
> TCP_LISTEN state may be assigned in only place in net/unix/af_unix.c:
> it's unix_listen(). The function is applied to stream and seqpacket
> socket types.
>
> It can't be stream because of the second stack, and seqpacket also can't,
> as I don't think it's possible for gcc to inline unix_seqpacket_sendmsg()
> in the way, we don't see it in the stack.
>
> So, this is looks like false positive result for me.
>
> Kirill

Do you mean that these &(>lock)->rlock/1 referenced in 2 stacks are
always different?

+Ingo for lockdep false positive
Do we need some kind of annotation here?


>> other info that might help us debug this:
>>
>>  Possible unsafe locking scenario:
>>
>>CPU0CPU1
>>
>>   lock(&(>lock)->rlock/1);
>>

Re: possible deadlock in skb_queue_tail

2018-04-03 Thread Dmitry Vyukov 
On Tue, Apr 3, 2018 at 11:50 AM, Kirill Tkhai  wrote:
> On 02.04.2018 12:20, syzbot wrote:
>> Hello,
>>
>> syzbot hit the following crash on net-next commit
>> 06b19fe9a6df7aaa423cd8404ebe5ac9ec4b2960 (Sun Apr 1 03:37:33 2018 +)
>> Merge branch 'chelsio-inline-tls'
>> syzbot dashboard link: 
>> https://syzkaller.appspot.com/bug?extid=6b495100f17ca8554ab9
>>
>> Unfortunately, I don't have any reproducer for this crash yet.
>> Raw console output: 
>> https://syzkaller.appspot.com/x/log.txt?id=6218830443446272
>> Kernel config: https://syzkaller.appspot.com/x/.config?id=3327544840960562528
>> compiler: gcc (GCC) 7.1.1 20170620
>>
>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>> Reported-by: syzbot+6b495100f17ca8554...@syzkaller.appspotmail.com
>> It will help syzbot understand when the bug is fixed. See footer for details.
>> If you forward the report, please keep this part and the footer.
>>
>>
>> ==
>> WARNING: possible circular locking dependency detected
>> 4.16.0-rc6+ #290 Not tainted
>> --
>> syz-executor7/20971 is trying to acquire lock:
>>  (_unix_sk_receive_queue_lock_key){+.+.}, at: [<271ef0d8>] 
>> skb_queue_tail+0x26/0x150 net/core/skbuff.c:2899
>>
>> but task is already holding lock:
>>  (&(>lock)->rlock/1){+.+.}, at: [<4e725e14>] 
>> unix_state_double_lock+0x7b/0xb0 net/unix/af_unix.c:1088
>>
>> which lock already depends on the new lock.
>>
>>
>> the existing dependency chain (in reverse order) is:
>>
>> -> #1 (&(>lock)->rlock/1){+.+.}:
>>_raw_spin_lock_nested+0x28/0x40 kernel/locking/spinlock.c:354
>>sk_diag_dump_icons net/unix/diag.c:82 [inline]
>>sk_diag_fill.isra.4+0xa52/0xfe0 net/unix/diag.c:144
>>sk_diag_dump net/unix/diag.c:178 [inline]
>>unix_diag_dump+0x400/0x4f0 net/unix/diag.c:206
>>netlink_dump+0x492/0xcf0 net/netlink/af_netlink.c:2221
>>__netlink_dump_start+0x4ec/0x710 net/netlink/af_netlink.c:2318
>>netlink_dump_start include/linux/netlink.h:214 [inline]
>>unix_diag_handler_dump+0x3e7/0x750 net/unix/diag.c:307
>>__sock_diag_cmd net/core/sock_diag.c:230 [inline]
>>sock_diag_rcv_msg+0x204/0x360 net/core/sock_diag.c:261
>>netlink_rcv_skb+0x14b/0x380 net/netlink/af_netlink.c:2443
>>sock_diag_rcv+0x2a/0x40 net/core/sock_diag.c:272
>>netlink_unicast_kernel net/netlink/af_netlink.c:1307 [inline]
>>netlink_unicast+0x4c4/0x6b0 net/netlink/af_netlink.c:1333
>>netlink_sendmsg+0xa4a/0xe80 net/netlink/af_netlink.c:1896
>>sock_sendmsg_nosec net/socket.c:629 [inline]
>>sock_sendmsg+0xca/0x110 net/socket.c:639
>>sock_write_iter+0x31a/0x5d0 net/socket.c:908
>>call_write_iter include/linux/fs.h:1782 [inline]
>>new_sync_write fs/read_write.c:469 [inline]
>>__vfs_write+0x684/0x970 fs/read_write.c:482
>>vfs_write+0x189/0x510 fs/read_write.c:544
>>SYSC_write fs/read_write.c:589 [inline]
>>SyS_write+0xef/0x220 fs/read_write.c:581
>>do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
>>entry_SYSCALL_64_after_hwframe+0x42/0xb7
>>
>> -> #0 (_unix_sk_receive_queue_lock_key){+.+.}:
>>lock_acquire+0x1d5/0x580 kernel/locking/lockdep.c:3920
>>__raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
>>_raw_spin_lock_irqsave+0x96/0xc0 kernel/locking/spinlock.c:152
>>skb_queue_tail+0x26/0x150 net/core/skbuff.c:2899
>>unix_dgram_sendmsg+0xa30/0x1610 net/unix/af_unix.c:1807
>>sock_sendmsg_nosec net/socket.c:629 [inline]
>>sock_sendmsg+0xca/0x110 net/socket.c:639
>>___sys_sendmsg+0x320/0x8b0 net/socket.c:2047
>>__sys_sendmmsg+0x1ee/0x620 net/socket.c:2137
>>SYSC_sendmmsg net/socket.c:2168 [inline]
>>SyS_sendmmsg+0x35/0x60 net/socket.c:2163
>>do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
>>entry_SYSCALL_64_after_hwframe+0x42/0xb7
>
> sk_diag_dump_icons() dumps only sockets in TCP_LISTEN state.
> TCP_LISTEN state may be assigned in only place in net/unix/af_unix.c:
> it's unix_listen(). The function is applied to stream and seqpacket
> socket types.
>
> It can't be stream because of the second stack, and seqpacket also can't,
> as I don't think it's possible for gcc to inline unix_seqpacket_sendmsg()
> in the way, we don't see it in the stack.
>
> So, this is looks like false positive result for me.
>
> Kirill

Do you mean that these &(>lock)->rlock/1 referenced in 2 stacks are
always different?

+Ingo for lockdep false positive
Do we need some kind of annotation here?


>> other info that might help us debug this:
>>
>>  Possible unsafe locking scenario:
>>
>>CPU0CPU1
>>
>>   lock(&(>lock)->rlock/1);
>>

Re: possible deadlock in skb_queue_tail

2018-04-03 Thread Kirill Tkhai 
On 02.04.2018 12:20, syzbot wrote:
> Hello,
> 
> syzbot hit the following crash on net-next commit
> 06b19fe9a6df7aaa423cd8404ebe5ac9ec4b2960 (Sun Apr 1 03:37:33 2018 +)
> Merge branch 'chelsio-inline-tls'
> syzbot dashboard link: 
> https://syzkaller.appspot.com/bug?extid=6b495100f17ca8554ab9
> 
> Unfortunately, I don't have any reproducer for this crash yet.
> Raw console output: 
> https://syzkaller.appspot.com/x/log.txt?id=6218830443446272
> Kernel config: https://syzkaller.appspot.com/x/.config?id=3327544840960562528
> compiler: gcc (GCC) 7.1.1 20170620
> 
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+6b495100f17ca8554...@syzkaller.appspotmail.com
> It will help syzbot understand when the bug is fixed. See footer for details.
> If you forward the report, please keep this part and the footer.
> 
> 
> ==
> WARNING: possible circular locking dependency detected
> 4.16.0-rc6+ #290 Not tainted
> --
> syz-executor7/20971 is trying to acquire lock:
>  (_unix_sk_receive_queue_lock_key){+.+.}, at: [<271ef0d8>] 
> skb_queue_tail+0x26/0x150 net/core/skbuff.c:2899
> 
> but task is already holding lock:
>  (&(>lock)->rlock/1){+.+.}, at: [<4e725e14>] 
> unix_state_double_lock+0x7b/0xb0 net/unix/af_unix.c:1088
> 
> which lock already depends on the new lock.
> 
> 
> the existing dependency chain (in reverse order) is:
> 
> -> #1 (&(>lock)->rlock/1){+.+.}:
>    _raw_spin_lock_nested+0x28/0x40 kernel/locking/spinlock.c:354
>    sk_diag_dump_icons net/unix/diag.c:82 [inline]
>    sk_diag_fill.isra.4+0xa52/0xfe0 net/unix/diag.c:144
>    sk_diag_dump net/unix/diag.c:178 [inline]
>    unix_diag_dump+0x400/0x4f0 net/unix/diag.c:206
>    netlink_dump+0x492/0xcf0 net/netlink/af_netlink.c:2221
>    __netlink_dump_start+0x4ec/0x710 net/netlink/af_netlink.c:2318
>    netlink_dump_start include/linux/netlink.h:214 [inline]
>    unix_diag_handler_dump+0x3e7/0x750 net/unix/diag.c:307
>    __sock_diag_cmd net/core/sock_diag.c:230 [inline]
>    sock_diag_rcv_msg+0x204/0x360 net/core/sock_diag.c:261
>    netlink_rcv_skb+0x14b/0x380 net/netlink/af_netlink.c:2443
>    sock_diag_rcv+0x2a/0x40 net/core/sock_diag.c:272
>    netlink_unicast_kernel net/netlink/af_netlink.c:1307 [inline]
>    netlink_unicast+0x4c4/0x6b0 net/netlink/af_netlink.c:1333
>    netlink_sendmsg+0xa4a/0xe80 net/netlink/af_netlink.c:1896
>    sock_sendmsg_nosec net/socket.c:629 [inline]
>    sock_sendmsg+0xca/0x110 net/socket.c:639
>    sock_write_iter+0x31a/0x5d0 net/socket.c:908
>    call_write_iter include/linux/fs.h:1782 [inline]
>    new_sync_write fs/read_write.c:469 [inline]
>    __vfs_write+0x684/0x970 fs/read_write.c:482
>    vfs_write+0x189/0x510 fs/read_write.c:544
>    SYSC_write fs/read_write.c:589 [inline]
>    SyS_write+0xef/0x220 fs/read_write.c:581
>    do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
>    entry_SYSCALL_64_after_hwframe+0x42/0xb7
> 
> -> #0 (_unix_sk_receive_queue_lock_key){+.+.}:
>    lock_acquire+0x1d5/0x580 kernel/locking/lockdep.c:3920
>    __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
>    _raw_spin_lock_irqsave+0x96/0xc0 kernel/locking/spinlock.c:152
>    skb_queue_tail+0x26/0x150 net/core/skbuff.c:2899
>    unix_dgram_sendmsg+0xa30/0x1610 net/unix/af_unix.c:1807
>    sock_sendmsg_nosec net/socket.c:629 [inline]
>    sock_sendmsg+0xca/0x110 net/socket.c:639
>    ___sys_sendmsg+0x320/0x8b0 net/socket.c:2047
>    __sys_sendmmsg+0x1ee/0x620 net/socket.c:2137
>    SYSC_sendmmsg net/socket.c:2168 [inline]
>    SyS_sendmmsg+0x35/0x60 net/socket.c:2163
>    do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
>    entry_SYSCALL_64_after_hwframe+0x42/0xb7

sk_diag_dump_icons() dumps only sockets in TCP_LISTEN state.
TCP_LISTEN state may be assigned in only place in net/unix/af_unix.c:
it's unix_listen(). The function is applied to stream and seqpacket
socket types.

It can't be stream because of the second stack, and seqpacket also can't,
as I don't think it's possible for gcc to inline unix_seqpacket_sendmsg()
in the way, we don't see it in the stack.

So, this is looks like false positive result for me.

Kirill

> 
> other info that might help us debug this:
> 
>  Possible unsafe locking scenario:
> 
>    CPU0    CPU1
>        
>   lock(&(>lock)->rlock/1);
>    lock(_unix_sk_receive_queue_lock_key);
>    lock(&(>lock)->rlock/1);
>   lock(_unix_sk_receive_queue_lock_key);
> 
>  *** DEADLOCK ***
> 
> 1 lock held by syz-executor7/20971:
>  #0:  (&(>lock)->rlock/1){+.+.}, at: [<4e725e14>] 
> unix_state_double_lock+0x7b/0xb0 net/unix/af_unix.c:1088
> 
> stack

Re: possible deadlock in skb_queue_tail

2018-04-03 Thread Kirill Tkhai 
On 02.04.2018 12:20, syzbot wrote:
> Hello,
> 
> syzbot hit the following crash on net-next commit
> 06b19fe9a6df7aaa423cd8404ebe5ac9ec4b2960 (Sun Apr 1 03:37:33 2018 +)
> Merge branch 'chelsio-inline-tls'
> syzbot dashboard link: 
> https://syzkaller.appspot.com/bug?extid=6b495100f17ca8554ab9
> 
> Unfortunately, I don't have any reproducer for this crash yet.
> Raw console output: 
> https://syzkaller.appspot.com/x/log.txt?id=6218830443446272
> Kernel config: https://syzkaller.appspot.com/x/.config?id=3327544840960562528
> compiler: gcc (GCC) 7.1.1 20170620
> 
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+6b495100f17ca8554...@syzkaller.appspotmail.com
> It will help syzbot understand when the bug is fixed. See footer for details.
> If you forward the report, please keep this part and the footer.
> 
> 
> ==
> WARNING: possible circular locking dependency detected
> 4.16.0-rc6+ #290 Not tainted
> --
> syz-executor7/20971 is trying to acquire lock:
>  (_unix_sk_receive_queue_lock_key){+.+.}, at: [<271ef0d8>] 
> skb_queue_tail+0x26/0x150 net/core/skbuff.c:2899
> 
> but task is already holding lock:
>  (&(>lock)->rlock/1){+.+.}, at: [<4e725e14>] 
> unix_state_double_lock+0x7b/0xb0 net/unix/af_unix.c:1088
> 
> which lock already depends on the new lock.
> 
> 
> the existing dependency chain (in reverse order) is:
> 
> -> #1 (&(>lock)->rlock/1){+.+.}:
>    _raw_spin_lock_nested+0x28/0x40 kernel/locking/spinlock.c:354
>    sk_diag_dump_icons net/unix/diag.c:82 [inline]
>    sk_diag_fill.isra.4+0xa52/0xfe0 net/unix/diag.c:144
>    sk_diag_dump net/unix/diag.c:178 [inline]
>    unix_diag_dump+0x400/0x4f0 net/unix/diag.c:206
>    netlink_dump+0x492/0xcf0 net/netlink/af_netlink.c:2221
>    __netlink_dump_start+0x4ec/0x710 net/netlink/af_netlink.c:2318
>    netlink_dump_start include/linux/netlink.h:214 [inline]
>    unix_diag_handler_dump+0x3e7/0x750 net/unix/diag.c:307
>    __sock_diag_cmd net/core/sock_diag.c:230 [inline]
>    sock_diag_rcv_msg+0x204/0x360 net/core/sock_diag.c:261
>    netlink_rcv_skb+0x14b/0x380 net/netlink/af_netlink.c:2443
>    sock_diag_rcv+0x2a/0x40 net/core/sock_diag.c:272
>    netlink_unicast_kernel net/netlink/af_netlink.c:1307 [inline]
>    netlink_unicast+0x4c4/0x6b0 net/netlink/af_netlink.c:1333
>    netlink_sendmsg+0xa4a/0xe80 net/netlink/af_netlink.c:1896
>    sock_sendmsg_nosec net/socket.c:629 [inline]
>    sock_sendmsg+0xca/0x110 net/socket.c:639
>    sock_write_iter+0x31a/0x5d0 net/socket.c:908
>    call_write_iter include/linux/fs.h:1782 [inline]
>    new_sync_write fs/read_write.c:469 [inline]
>    __vfs_write+0x684/0x970 fs/read_write.c:482
>    vfs_write+0x189/0x510 fs/read_write.c:544
>    SYSC_write fs/read_write.c:589 [inline]
>    SyS_write+0xef/0x220 fs/read_write.c:581
>    do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
>    entry_SYSCALL_64_after_hwframe+0x42/0xb7
> 
> -> #0 (_unix_sk_receive_queue_lock_key){+.+.}:
>    lock_acquire+0x1d5/0x580 kernel/locking/lockdep.c:3920
>    __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
>    _raw_spin_lock_irqsave+0x96/0xc0 kernel/locking/spinlock.c:152
>    skb_queue_tail+0x26/0x150 net/core/skbuff.c:2899
>    unix_dgram_sendmsg+0xa30/0x1610 net/unix/af_unix.c:1807
>    sock_sendmsg_nosec net/socket.c:629 [inline]
>    sock_sendmsg+0xca/0x110 net/socket.c:639
>    ___sys_sendmsg+0x320/0x8b0 net/socket.c:2047
>    __sys_sendmmsg+0x1ee/0x620 net/socket.c:2137
>    SYSC_sendmmsg net/socket.c:2168 [inline]
>    SyS_sendmmsg+0x35/0x60 net/socket.c:2163
>    do_syscall_64+0x281/0x940 arch/x86/entry/common.c:287
>    entry_SYSCALL_64_after_hwframe+0x42/0xb7

sk_diag_dump_icons() dumps only sockets in TCP_LISTEN state.
TCP_LISTEN state may be assigned in only place in net/unix/af_unix.c:
it's unix_listen(). The function is applied to stream and seqpacket
socket types.

It can't be stream because of the second stack, and seqpacket also can't,
as I don't think it's possible for gcc to inline unix_seqpacket_sendmsg()
in the way, we don't see it in the stack.

So, this is looks like false positive result for me.

Kirill

> 
> other info that might help us debug this:
> 
>  Possible unsafe locking scenario:
> 
>    CPU0    CPU1
>        
>   lock(&(>lock)->rlock/1);
>    lock(_unix_sk_receive_queue_lock_key);
>    lock(&(>lock)->rlock/1);
>   lock(_unix_sk_receive_queue_lock_key);
> 
>  *** DEADLOCK ***
> 
> 1 lock held by syz-executor7/20971:
>  #0:  (&(>lock)->rlock/1){+.+.}, at: [<4e725e14>] 
> unix_state_double_lock+0x7b/0xb0 net/unix/af_unix.c:1088
> 
> stack