Re: fs, net: deadlock between bind/splice on af_unix

2017-06-23 Thread Cong Wang 
Hi,

On Thu, Jun 22, 2017 at 10:49 AM,   wrote:
> I was getting below crash while running mp4.

Are you sure your 3.14 kernel has my patch in this thread?
commit 0fb44559ffd67de8517098 is merged in 4.10.

Also, your crash is on unix_dgram_sendmsg() path, not
unix_bind().

Re: fs, net: deadlock between bind/splice on af_unix

2017-02-09 Thread Cong Wang 
On Tue, Feb 7, 2017 at 6:20 AM, Mateusz Guzik  wrote:
>
> Yes, but unix_release_sock is expected to leave the file behind.
> Note I'm not claiming there is a leak, but that racing threads will be
> able to trigger a condition where you create a file and fail to bind it.
>

Which is expected, right? No one guarantees the success of file
creation is the success of bind, the previous code does but it is not
part of API AFAIK. Should a sane user-space application check
the file creation for a successful bind() or just check its return value?

> What to do with the file now?
>

We just do what unix_release_sock() does, so why do you keep
asking the same question?

If you still complain about the race with user-space, think about the
same race in-between a successful bind() and close(), nothing is new.

Re: fs, net: deadlock between bind/splice on af_unix

2017-02-07 Thread Mateusz Guzik 
On Sun, Feb 05, 2017 at 11:22:12PM -0800, Cong Wang wrote:
> On Tue, Jan 31, 2017 at 10:14 AM, Mateusz Guzik  wrote:
> > On Mon, Jan 30, 2017 at 10:44:03PM -0800, Cong Wang wrote:
> >> Mind being more specific?
> >
> > Consider 2 threads which bind the same socket, but with different paths.
> >
> > Currently exactly one file will get created, the one used to bind.
> >
> > With your patch both threads can succeed creating their respective
> > files, but only one will manage to bind. The other one must error out,
> > but it already created a file it is unclear what to do with.
> 
> In this case, it simply puts the path back:
> 
> err = -EINVAL;
> if (u->addr)
> goto out_up;
> [...]
> 
> out_up:
> mutex_unlock(>bindlock);
> out_put:
> if (err)
> path_put();
> out:
> return err;
> 
> 
> Which is what unix_release_sock() does too:
> 
> if (path.dentry)
> path_put();

Yes, but unix_release_sock is expected to leave the file behind.
Note I'm not claiming there is a leak, but that racing threads will be
able to trigger a condition where you create a file and fail to bind it.

What to do with the file now?

Untested, but likely a working solution would rework the code so that
e.g. a flag is set and the lock can be dropped.

Re: fs, net: deadlock between bind/splice on af_unix

2017-02-05 Thread Cong Wang 
On Tue, Jan 31, 2017 at 10:14 AM, Mateusz Guzik  wrote:
> On Mon, Jan 30, 2017 at 10:44:03PM -0800, Cong Wang wrote:
>> Mind being more specific?
>
> Consider 2 threads which bind the same socket, but with different paths.
>
> Currently exactly one file will get created, the one used to bind.
>
> With your patch both threads can succeed creating their respective
> files, but only one will manage to bind. The other one must error out,
> but it already created a file it is unclear what to do with.

In this case, it simply puts the path back:

err = -EINVAL;
if (u->addr)
goto out_up;
[...]

out_up:
mutex_unlock(>bindlock);
out_put:
if (err)
path_put();
out:
return err;


Which is what unix_release_sock() does too:

if (path.dentry)
path_put();

Re: fs, net: deadlock between bind/splice on af_unix

2017-01-31 Thread Mateusz Guzik 
On Mon, Jan 30, 2017 at 10:44:03PM -0800, Cong Wang wrote:
> On Thu, Jan 26, 2017 at 10:41 PM, Mateusz Guzik  wrote:
> > On Thu, Jan 26, 2017 at 09:11:07PM -0800, Cong Wang wrote:
> >> On Thu, Jan 26, 2017 at 3:29 PM, Mateusz Guzik  wrote:
> >> > Currently the file creation is potponed until unix_bind can no longer
> >> > fail otherwise. With it reordered, it may be someone races you with a
> >> > different path and now you are left with a file to clean up. Except it
> >> > is quite unclear for me if you can unlink it.
> >>
> >> What races do you mean here? If you mean someone could get a
> >> refcount of that file, it could happen no matter we have bindlock or not
> >> since it is visible once created. The filesystem layer should take care of
> >> the file refcount so all we need to do here is calling path_put() as in my
> >> patch. Or if you mean two threads calling unix_bind() could race without
> >> binlock, only one of them should succeed the other one just fails out.
> >
> > Two threads can race and one fails with EINVAL.
> >
> > With your patch there is a new file created and it is unclear what to
> > do with it - leaving it as it is sounds like the last resort and
> > unlinking it sounds extremely fishy as it opens you to games played by
> > the user.
> 
> But the file is created and visible to users too even without my patch,
> the file is also put when the unix sock is released. So the only difference
> my patch makes is bindlock is no longer taken during file creation, which
> does not seem to be the cause of the problem you complain here.
> 
> Mind being more specific?

Consider 2 threads which bind the same socket, but with different paths.

Currently exactly one file will get created, the one used to bind.

With your patch both threads can succeed creating their respective
files, but only one will manage to bind. The other one must error out,
but it already created a file it is unclear what to do with.

Re: fs, net: deadlock between bind/splice on af_unix

2017-01-30 Thread Cong Wang 
On Thu, Jan 26, 2017 at 10:41 PM, Mateusz Guzik  wrote:
> On Thu, Jan 26, 2017 at 09:11:07PM -0800, Cong Wang wrote:
>> On Thu, Jan 26, 2017 at 3:29 PM, Mateusz Guzik  wrote:
>> > Currently the file creation is potponed until unix_bind can no longer
>> > fail otherwise. With it reordered, it may be someone races you with a
>> > different path and now you are left with a file to clean up. Except it
>> > is quite unclear for me if you can unlink it.
>>
>> What races do you mean here? If you mean someone could get a
>> refcount of that file, it could happen no matter we have bindlock or not
>> since it is visible once created. The filesystem layer should take care of
>> the file refcount so all we need to do here is calling path_put() as in my
>> patch. Or if you mean two threads calling unix_bind() could race without
>> binlock, only one of them should succeed the other one just fails out.
>
> Two threads can race and one fails with EINVAL.
>
> With your patch there is a new file created and it is unclear what to
> do with it - leaving it as it is sounds like the last resort and
> unlinking it sounds extremely fishy as it opens you to games played by
> the user.

But the file is created and visible to users too even without my patch,
the file is also put when the unix sock is released. So the only difference
my patch makes is bindlock is no longer taken during file creation, which
does not seem to be the cause of the problem you complain here.

Mind being more specific?

Re: fs, net: deadlock between bind/splice on af_unix

2017-01-26 Thread Mateusz Guzik 
On Thu, Jan 26, 2017 at 09:11:07PM -0800, Cong Wang wrote:
> On Thu, Jan 26, 2017 at 3:29 PM, Mateusz Guzik  wrote:
> > On Tue, Jan 17, 2017 at 01:21:48PM -0800, Cong Wang wrote:
> >> On Mon, Jan 16, 2017 at 1:32 AM, Dmitry Vyukov  wrote:
> >> > On Fri, Dec 9, 2016 at 7:41 AM, Al Viro  wrote:
> >> >> On Thu, Dec 08, 2016 at 10:32:00PM -0800, Cong Wang wrote:
> >> >>
> >> >>> > Why do we do autobind there, anyway, and why is it conditional on
> >> >>> > SOCK_PASSCRED?  Note that e.g. for SOCK_STREAM we can bloody well get
> >> >>> > to sending stuff without autobind ever done - just use socketpair()
> >> >>> > to create that sucker and we won't be going through the connect()
> >> >>> > at all.
> >> >>>
> >> >>> In the case Dmitry reported, unix_dgram_sendmsg() calls 
> >> >>> unix_autobind(),
> >> >>> not SOCK_STREAM.
> >> >>
> >> >> Yes, I've noticed.  What I'm asking is what in there needs autobind 
> >> >> triggered
> >> >> on sendmsg and why doesn't the same need affect the SOCK_STREAM case?
> >> >>
> >> >>> I guess some lock, perhaps the u->bindlock could be dropped before
> >> >>> acquiring the next one (sb_writer), but I need to double check.
> >> >>
> >> >> Bad idea, IMO - do you *want* autobind being able to come through while
> >> >> bind(2) is busy with mknod?
> >> >
> >> >
> >> > Ping. This is still happening on HEAD.
> >> >
> >>
> >> Thanks for your reminder. Mind to give the attached patch (compile only)
> >> a try? I take another approach to fix this deadlock, which moves the
> >> unix_mknod() out of unix->bindlock. Not sure if there is any unexpected
> >> impact with this way.
> >>
> >
> > I don't think this is the right approach.
> >
> > Currently the file creation is potponed until unix_bind can no longer
> > fail otherwise. With it reordered, it may be someone races you with a
> > different path and now you are left with a file to clean up. Except it
> > is quite unclear for me if you can unlink it.
> 
> What races do you mean here? If you mean someone could get a
> refcount of that file, it could happen no matter we have bindlock or not
> since it is visible once created. The filesystem layer should take care of
> the file refcount so all we need to do here is calling path_put() as in my
> patch. Or if you mean two threads calling unix_bind() could race without
> binlock, only one of them should succeed the other one just fails out.

Two threads can race and one fails with EINVAL.

With your patch there is a new file created and it is unclear what to
do with it - leaving it as it is sounds like the last resort and
unlinking it sounds extremely fishy as it opens you to games played by
the user.

Re: fs, net: deadlock between bind/splice on af_unix

2017-01-26 Thread Cong Wang 
On Thu, Jan 26, 2017 at 3:29 PM, Mateusz Guzik  wrote:
> On Tue, Jan 17, 2017 at 01:21:48PM -0800, Cong Wang wrote:
>> On Mon, Jan 16, 2017 at 1:32 AM, Dmitry Vyukov  wrote:
>> > On Fri, Dec 9, 2016 at 7:41 AM, Al Viro  wrote:
>> >> On Thu, Dec 08, 2016 at 10:32:00PM -0800, Cong Wang wrote:
>> >>
>> >>> > Why do we do autobind there, anyway, and why is it conditional on
>> >>> > SOCK_PASSCRED?  Note that e.g. for SOCK_STREAM we can bloody well get
>> >>> > to sending stuff without autobind ever done - just use socketpair()
>> >>> > to create that sucker and we won't be going through the connect()
>> >>> > at all.
>> >>>
>> >>> In the case Dmitry reported, unix_dgram_sendmsg() calls unix_autobind(),
>> >>> not SOCK_STREAM.
>> >>
>> >> Yes, I've noticed.  What I'm asking is what in there needs autobind 
>> >> triggered
>> >> on sendmsg and why doesn't the same need affect the SOCK_STREAM case?
>> >>
>> >>> I guess some lock, perhaps the u->bindlock could be dropped before
>> >>> acquiring the next one (sb_writer), but I need to double check.
>> >>
>> >> Bad idea, IMO - do you *want* autobind being able to come through while
>> >> bind(2) is busy with mknod?
>> >
>> >
>> > Ping. This is still happening on HEAD.
>> >
>>
>> Thanks for your reminder. Mind to give the attached patch (compile only)
>> a try? I take another approach to fix this deadlock, which moves the
>> unix_mknod() out of unix->bindlock. Not sure if there is any unexpected
>> impact with this way.
>>
>
> I don't think this is the right approach.
>
> Currently the file creation is potponed until unix_bind can no longer
> fail otherwise. With it reordered, it may be someone races you with a
> different path and now you are left with a file to clean up. Except it
> is quite unclear for me if you can unlink it.

What races do you mean here? If you mean someone could get a
refcount of that file, it could happen no matter we have bindlock or not
since it is visible once created. The filesystem layer should take care of
the file refcount so all we need to do here is calling path_put() as in my
patch. Or if you mean two threads calling unix_bind() could race without
binlock, only one of them should succeed the other one just fails out.

Re: fs, net: deadlock between bind/splice on af_unix

2017-01-26 Thread Mateusz Guzik 
On Tue, Jan 17, 2017 at 01:21:48PM -0800, Cong Wang wrote:
> On Mon, Jan 16, 2017 at 1:32 AM, Dmitry Vyukov  wrote:
> > On Fri, Dec 9, 2016 at 7:41 AM, Al Viro  wrote:
> >> On Thu, Dec 08, 2016 at 10:32:00PM -0800, Cong Wang wrote:
> >>
> >>> > Why do we do autobind there, anyway, and why is it conditional on
> >>> > SOCK_PASSCRED?  Note that e.g. for SOCK_STREAM we can bloody well get
> >>> > to sending stuff without autobind ever done - just use socketpair()
> >>> > to create that sucker and we won't be going through the connect()
> >>> > at all.
> >>>
> >>> In the case Dmitry reported, unix_dgram_sendmsg() calls unix_autobind(),
> >>> not SOCK_STREAM.
> >>
> >> Yes, I've noticed.  What I'm asking is what in there needs autobind 
> >> triggered
> >> on sendmsg and why doesn't the same need affect the SOCK_STREAM case?
> >>
> >>> I guess some lock, perhaps the u->bindlock could be dropped before
> >>> acquiring the next one (sb_writer), but I need to double check.
> >>
> >> Bad idea, IMO - do you *want* autobind being able to come through while
> >> bind(2) is busy with mknod?
> >
> >
> > Ping. This is still happening on HEAD.
> >
> 
> Thanks for your reminder. Mind to give the attached patch (compile only)
> a try? I take another approach to fix this deadlock, which moves the
> unix_mknod() out of unix->bindlock. Not sure if there is any unexpected
> impact with this way.
> 

I don't think this is the right approach.

Currently the file creation is potponed until unix_bind can no longer
fail otherwise. With it reordered, it may be someone races you with a
different path and now you are left with a file to clean up. Except it
is quite unclear for me if you can unlink it.

I don't have a good idea how to fix it. A somewhat typical approach
would introduce an intermediate state ("under construction") and drop
the lock between calling into unix_mknod.

In this particular case, perhaps you could repurpose gc_flags as a
general flags carrier and add a 'binding in process' flag to test.

Re: fs, net: deadlock between bind/splice on af_unix

2017-01-23 Thread Cong Wang 
On Fri, Jan 20, 2017 at 2:52 PM, Dmitry Vyukov  wrote:
>
> This works! I did not see the deadlock warning, nor any other related crashes.
>
> Tested-by: Dmitry Vyukov 

Thanks for verifying it. I will send it out formally soon.

Re: fs, net: deadlock between bind/splice on af_unix

2017-01-20 Thread Dmitry Vyukov 
On Fri, Jan 20, 2017 at 5:57 AM, Cong Wang  wrote:
>> > Why do we do autobind there, anyway, and why is it conditional on
>> > SOCK_PASSCRED?  Note that e.g. for SOCK_STREAM we can bloody well get
>> > to sending stuff without autobind ever done - just use socketpair()
>> > to create that sucker and we won't be going through the connect()
>> > at all.
>>
>> In the case Dmitry reported, unix_dgram_sendmsg() calls unix_autobind(),
>> not SOCK_STREAM.
>
> Yes, I've noticed.  What I'm asking is what in there needs autobind 
> triggered
> on sendmsg and why doesn't the same need affect the SOCK_STREAM case?
>
>> I guess some lock, perhaps the u->bindlock could be dropped before
>> acquiring the next one (sb_writer), but I need to double check.
>
> Bad idea, IMO - do you *want* autobind being able to come through while
> bind(2) is busy with mknod?


 Ping. This is still happening on HEAD.

>>>
>>> Thanks for your reminder. Mind to give the attached patch (compile only)
>>> a try? I take another approach to fix this deadlock, which moves the
>>> unix_mknod() out of unix->bindlock. Not sure if there is any unexpected
>>> impact with this way.
>>
>>
>> I instantly hit:
>>
>
> Oh, sorry about it, I forgot to initialize struct path...
>
> Attached is the updated version, I just did a boot test, no crash at least. ;)
>
> Thanks!

This works! I did not see the deadlock warning, nor any other related crashes.

Tested-by: Dmitry Vyukov

Re: fs, net: deadlock between bind/splice on af_unix

2017-01-19 Thread Cong Wang 
On Wed, Jan 18, 2017 at 1:17 AM, Dmitry Vyukov  wrote:
> On Tue, Jan 17, 2017 at 10:21 PM, Cong Wang  wrote:
>> On Mon, Jan 16, 2017 at 1:32 AM, Dmitry Vyukov  wrote:
>>> On Fri, Dec 9, 2016 at 7:41 AM, Al Viro  wrote:
 On Thu, Dec 08, 2016 at 10:32:00PM -0800, Cong Wang wrote:

> > Why do we do autobind there, anyway, and why is it conditional on
> > SOCK_PASSCRED?  Note that e.g. for SOCK_STREAM we can bloody well get
> > to sending stuff without autobind ever done - just use socketpair()
> > to create that sucker and we won't be going through the connect()
> > at all.
>
> In the case Dmitry reported, unix_dgram_sendmsg() calls unix_autobind(),
> not SOCK_STREAM.

 Yes, I've noticed.  What I'm asking is what in there needs autobind 
 triggered
 on sendmsg and why doesn't the same need affect the SOCK_STREAM case?

> I guess some lock, perhaps the u->bindlock could be dropped before
> acquiring the next one (sb_writer), but I need to double check.

 Bad idea, IMO - do you *want* autobind being able to come through while
 bind(2) is busy with mknod?
>>>
>>>
>>> Ping. This is still happening on HEAD.
>>>
>>
>> Thanks for your reminder. Mind to give the attached patch (compile only)
>> a try? I take another approach to fix this deadlock, which moves the
>> unix_mknod() out of unix->bindlock. Not sure if there is any unexpected
>> impact with this way.
>
>
> I instantly hit:
>

Oh, sorry about it, I forgot to initialize struct path...

Attached is the updated version, I just did a boot test, no crash at least. ;)

Thanks!
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 127656e..cef7987 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -995,6 +995,7 @@ static int unix_bind(struct socket *sock, struct sockaddr 
*uaddr, int addr_len)
unsigned int hash;
struct unix_address *addr;
struct hlist_head *list;
+   struct path path = { NULL, NULL };
 
err = -EINVAL;
if (sunaddr->sun_family != AF_UNIX)
@@ -1010,9 +1011,20 @@ static int unix_bind(struct socket *sock, struct 
sockaddr *uaddr, int addr_len)
goto out;
addr_len = err;
 
+   if (sun_path[0]) {
+   umode_t mode = S_IFSOCK |
+  (SOCK_INODE(sock)->i_mode & ~current_umask());
+   err = unix_mknod(sun_path, mode, );
+   if (err) {
+   if (err == -EEXIST)
+   err = -EADDRINUSE;
+   goto out;
+   }
+   }
+
err = mutex_lock_interruptible(>bindlock);
if (err)
-   goto out;
+   goto out_put;
 
err = -EINVAL;
if (u->addr)
@@ -1029,16 +1041,6 @@ static int unix_bind(struct socket *sock, struct 
sockaddr *uaddr, int addr_len)
atomic_set(>refcnt, 1);
 
if (sun_path[0]) {
-   struct path path;
-   umode_t mode = S_IFSOCK |
-  (SOCK_INODE(sock)->i_mode & ~current_umask());
-   err = unix_mknod(sun_path, mode, );
-   if (err) {
-   if (err == -EEXIST)
-   err = -EADDRINUSE;
-   unix_release_addr(addr);
-   goto out_up;
-   }
addr->hash = UNIX_HASH_SIZE;
hash = d_backing_inode(path.dentry)->i_ino & (UNIX_HASH_SIZE - 
1);
spin_lock(_table_lock);
@@ -1065,6 +1067,9 @@ static int unix_bind(struct socket *sock, struct sockaddr 
*uaddr, int addr_len)
spin_unlock(_table_lock);
 out_up:
mutex_unlock(>bindlock);
+out_put:
+   if (err)
+   path_put();
 out:
return err;
 }

Re: fs, net: deadlock between bind/splice on af_unix

2017-01-18 Thread Dmitry Vyukov 
On Tue, Jan 17, 2017 at 10:21 PM, Cong Wang  wrote:
> On Mon, Jan 16, 2017 at 1:32 AM, Dmitry Vyukov  wrote:
>> On Fri, Dec 9, 2016 at 7:41 AM, Al Viro  wrote:
>>> On Thu, Dec 08, 2016 at 10:32:00PM -0800, Cong Wang wrote:
>>>
 > Why do we do autobind there, anyway, and why is it conditional on
 > SOCK_PASSCRED?  Note that e.g. for SOCK_STREAM we can bloody well get
 > to sending stuff without autobind ever done - just use socketpair()
 > to create that sucker and we won't be going through the connect()
 > at all.

 In the case Dmitry reported, unix_dgram_sendmsg() calls unix_autobind(),
 not SOCK_STREAM.
>>>
>>> Yes, I've noticed.  What I'm asking is what in there needs autobind 
>>> triggered
>>> on sendmsg and why doesn't the same need affect the SOCK_STREAM case?
>>>
 I guess some lock, perhaps the u->bindlock could be dropped before
 acquiring the next one (sb_writer), but I need to double check.
>>>
>>> Bad idea, IMO - do you *want* autobind being able to come through while
>>> bind(2) is busy with mknod?
>>
>>
>> Ping. This is still happening on HEAD.
>>
>
> Thanks for your reminder. Mind to give the attached patch (compile only)
> a try? I take another approach to fix this deadlock, which moves the
> unix_mknod() out of unix->bindlock. Not sure if there is any unexpected
> impact with this way.


I instantly hit:

general protection fault:  [#1] SMP KASAN
Dumping ftrace buffer:
   (ftrace buffer empty)
Modules linked in:
CPU: 0 PID: 8930 Comm: syz-executor1 Not tainted 4.10.0-rc4+ #177
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
task: 88003c908840 task.stack: 88003a9a
RIP: 0010:__lock_acquire+0xb3a/0x3430 kernel/locking/lockdep.c:3224
RSP: 0018:88003a9a7218 EFLAGS: 00010006
RAX: dc00 RBX: dc00 RCX: 
RDX: 0003 RSI:  RDI: 110007534e9d
RBP: 88003a9a7750 R08: 0001 R09: 
R10: 0018 R11:  R12: 88003c908840
R13: 0001 R14: 863504a0 R15: 0001
FS:  7f4f8eb5d700() GS:88003fc0() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2: 20b1d000 CR3: 3bde9000 CR4: 06f0
Call Trace:
 lock_acquire+0x2a1/0x630 kernel/locking/lockdep.c:3753
 __raw_spin_lock include/linux/spinlock_api_smp.h:144 [inline]
 _raw_spin_lock+0x33/0x50 kernel/locking/spinlock.c:151
 spin_lock include/linux/spinlock.h:302 [inline]
 list_lru_add+0x10b/0x340 mm/list_lru.c:115
 d_lru_add fs/dcache.c:366 [inline]
 dentry_lru_add fs/dcache.c:421 [inline]
 dput.part.27+0x659/0x7c0 fs/dcache.c:784
 dput+0x1f/0x30 fs/dcache.c:753
 path_put+0x31/0x70 fs/namei.c:500
 unix_bind+0x424/0xea0 net/unix/af_unix.c:1072
 SYSC_bind+0x20e/0x4a0 net/socket.c:1413
 SyS_bind+0x24/0x30 net/socket.c:1399
 entry_SYSCALL_64_fastpath+0x1f/0xc2
RIP: 0033:0x4454b9
RSP: 002b:7f4f8eb5cb58 EFLAGS: 0292 ORIG_RAX: 0031
RAX: ffda RBX: 001d RCX: 004454b9
RDX: 0008 RSI: 2002cff8 RDI: 001d
RBP: 006dd230 R08:  R09: 
R10:  R11: 0292 R12: 0070
R13: 7f4f8f2de9d0 R14: 7f4f8f2dfc40 R15: 
Code: e9 03 f3 48 ab 48 81 c4 10 05 00 00 44 89 e8 5b 41 5c 41 5d 41
5e 41 5f 5d c3 4c 89 d2 48 b8 00 00 00 00 00 fc ff df 48 c1 ea 03 <80>
3c 02 00 0f 85 9e 26 00 00 49 81 3a e0 be 6b 85 41 bf 00 00
RIP: __lock_acquire+0xb3a/0x3430 kernel/locking/lockdep.c:3224 RSP:
88003a9a7218
---[ end trace 78951d69744a2fe1 ]---
Kernel panic - not syncing: Fatal exception
Dumping ftrace buffer:
   (ftrace buffer empty)
Kernel Offset: disabled


and:


BUG: KASAN: use-after-free in list_lru_add+0x2fd/0x340
mm/list_lru.c:112 at addr 88006b301340
Read of size 8 by task syz-executor0/7116
CPU: 2 PID: 7116 Comm: syz-executor0 Not tainted 4.10.0-rc4+ #177
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Call Trace:
 __dump_stack lib/dump_stack.c:15 [inline]
 dump_stack+0x2ee/0x3ef lib/dump_stack.c:51
 kasan_object_err+0x1c/0x70 mm/kasan/report.c:165
 print_address_description mm/kasan/report.c:203 [inline]
 kasan_report_error mm/kasan/report.c:287 [inline]
 kasan_report+0x1b6/0x460 mm/kasan/report.c:307
 __asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:333
 list_lru_add+0x2fd/0x340 mm/list_lru.c:112
 d_lru_add fs/dcache.c:366 [inline]
 dentry_lru_add fs/dcache.c:421 [inline]
 dput.part.27+0x659/0x7c0 fs/dcache.c:784
 dput+0x1f/0x30 fs/dcache.c:753
 path_put+0x31/0x70 fs/namei.c:500
 unix_bind+0x424/0xea0 net/unix/af_unix.c:1072
 SYSC_bind+0x20e/0x4a0 net/socket.c:1413
 SyS_bind+0x24/0x30 net/socket.c:1399
 entry_SYSCALL_64_fastpath+0x1f/0xc2
RIP: 0033:0x4454b9
RSP: 002b:7f1b034ebb58 EFLAGS:

Re: fs, net: deadlock between bind/splice on af_unix

2017-01-17 Thread Cong Wang 
On Mon, Jan 16, 2017 at 1:32 AM, Dmitry Vyukov  wrote:
> On Fri, Dec 9, 2016 at 7:41 AM, Al Viro  wrote:
>> On Thu, Dec 08, 2016 at 10:32:00PM -0800, Cong Wang wrote:
>>
>>> > Why do we do autobind there, anyway, and why is it conditional on
>>> > SOCK_PASSCRED?  Note that e.g. for SOCK_STREAM we can bloody well get
>>> > to sending stuff without autobind ever done - just use socketpair()
>>> > to create that sucker and we won't be going through the connect()
>>> > at all.
>>>
>>> In the case Dmitry reported, unix_dgram_sendmsg() calls unix_autobind(),
>>> not SOCK_STREAM.
>>
>> Yes, I've noticed.  What I'm asking is what in there needs autobind triggered
>> on sendmsg and why doesn't the same need affect the SOCK_STREAM case?
>>
>>> I guess some lock, perhaps the u->bindlock could be dropped before
>>> acquiring the next one (sb_writer), but I need to double check.
>>
>> Bad idea, IMO - do you *want* autobind being able to come through while
>> bind(2) is busy with mknod?
>
>
> Ping. This is still happening on HEAD.
>

Thanks for your reminder. Mind to give the attached patch (compile only)
a try? I take another approach to fix this deadlock, which moves the
unix_mknod() out of unix->bindlock. Not sure if there is any unexpected
impact with this way.

Thanks.
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 127656e..5d4b4d1 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -995,6 +995,7 @@ static int unix_bind(struct socket *sock, struct sockaddr 
*uaddr, int addr_len)
unsigned int hash;
struct unix_address *addr;
struct hlist_head *list;
+   struct path path;
 
err = -EINVAL;
if (sunaddr->sun_family != AF_UNIX)
@@ -1010,9 +1011,20 @@ static int unix_bind(struct socket *sock, struct 
sockaddr *uaddr, int addr_len)
goto out;
addr_len = err;
 
+   if (sun_path[0]) {
+   umode_t mode = S_IFSOCK |
+  (SOCK_INODE(sock)->i_mode & ~current_umask());
+   err = unix_mknod(sun_path, mode, );
+   if (err) {
+   if (err == -EEXIST)
+   err = -EADDRINUSE;
+   goto out;
+   }
+   }
+
err = mutex_lock_interruptible(>bindlock);
if (err)
-   goto out;
+   goto out_put;
 
err = -EINVAL;
if (u->addr)
@@ -1029,16 +1041,6 @@ static int unix_bind(struct socket *sock, struct 
sockaddr *uaddr, int addr_len)
atomic_set(>refcnt, 1);
 
if (sun_path[0]) {
-   struct path path;
-   umode_t mode = S_IFSOCK |
-  (SOCK_INODE(sock)->i_mode & ~current_umask());
-   err = unix_mknod(sun_path, mode, );
-   if (err) {
-   if (err == -EEXIST)
-   err = -EADDRINUSE;
-   unix_release_addr(addr);
-   goto out_up;
-   }
addr->hash = UNIX_HASH_SIZE;
hash = d_backing_inode(path.dentry)->i_ino & (UNIX_HASH_SIZE - 
1);
spin_lock(_table_lock);
@@ -1065,6 +1067,9 @@ static int unix_bind(struct socket *sock, struct sockaddr 
*uaddr, int addr_len)
spin_unlock(_table_lock);
 out_up:
mutex_unlock(>bindlock);
+out_put:
+   if (err)
+   path_put();
 out:
return err;
 }

Re: fs, net: deadlock between bind/splice on af_unix

2017-01-17 Thread Eric W. Biederman 
Al Viro  writes:

> On Thu, Dec 08, 2016 at 10:32:00PM -0800, Cong Wang wrote:
>
>> > Why do we do autobind there, anyway, and why is it conditional on
>> > SOCK_PASSCRED?  Note that e.g. for SOCK_STREAM we can bloody well get
>> > to sending stuff without autobind ever done - just use socketpair()
>> > to create that sucker and we won't be going through the connect()
>> > at all.
>> 
>> In the case Dmitry reported, unix_dgram_sendmsg() calls unix_autobind(),
>> not SOCK_STREAM.
>
> Yes, I've noticed.  What I'm asking is what in there needs autobind triggered
> on sendmsg and why doesn't the same need affect the SOCK_STREAM case?

With respect to the conditionality on SOCK_PASSCRED those are the linux
semantics.  Semantically that is the way the code has behaved since
2.1.15 when support for passing credentials was added to the code.
So I presume someone thought it was a good idea to have a name for
a socket that is sending credentials to another socket.  It certainly
seems reasonable at first glance.

With socketpair the only path that doesn't enforce this with
SOCK_STREAM and SOCK_PASSCRED that is either an oversight or a don't
care because we already know who is at the other end.

I can imagine two possible fixes:
1) Declare that splice is non-sense in the presence of SOCK_PASSCRED.
2) Someone adds a preparation operation that can be called on
   af_unix sockets that will ensure the autobind happens before
   any problematic locks are taken.

Eric

Re: fs, net: deadlock between bind/splice on af_unix

2017-01-16 Thread Dmitry Vyukov 
On Fri, Dec 9, 2016 at 7:41 AM, Al Viro  wrote:
> On Thu, Dec 08, 2016 at 10:32:00PM -0800, Cong Wang wrote:
>
>> > Why do we do autobind there, anyway, and why is it conditional on
>> > SOCK_PASSCRED?  Note that e.g. for SOCK_STREAM we can bloody well get
>> > to sending stuff without autobind ever done - just use socketpair()
>> > to create that sucker and we won't be going through the connect()
>> > at all.
>>
>> In the case Dmitry reported, unix_dgram_sendmsg() calls unix_autobind(),
>> not SOCK_STREAM.
>
> Yes, I've noticed.  What I'm asking is what in there needs autobind triggered
> on sendmsg and why doesn't the same need affect the SOCK_STREAM case?
>
>> I guess some lock, perhaps the u->bindlock could be dropped before
>> acquiring the next one (sb_writer), but I need to double check.
>
> Bad idea, IMO - do you *want* autobind being able to come through while
> bind(2) is busy with mknod?


Ping. This is still happening on HEAD.


[ INFO: possible circular locking dependency detected ]
4.9.0 #1 Not tainted
---
syz-executor6/25491 is trying to acquire lock:
 (>bindlock){+.+.+.}, at: []
unix_autobind.isra.28+0xc5/0x880 net/unix/af_unix.c:852
but task is already holding lock:
 (>mutex/1){+.+.+.}, at: [] pipe_lock_nested
fs/pipe.c:66 [inline]
 (>mutex/1){+.+.+.}, at: []
pipe_lock+0x56/0x70 fs/pipe.c:74
which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

   [  836.500536] [] validate_chain
kernel/locking/lockdep.c:2265 [inline]
   [  836.500536] []
__lock_acquire+0x2149/0x3430 kernel/locking/lockdep.c:3338
   [  836.508456] [] lock_acquire+0x2a1/0x630
kernel/locking/lockdep.c:3753
   [  836.516117] [] __mutex_lock_common
kernel/locking/mutex.c:521 [inline]
   [  836.516117] []
mutex_lock_nested+0x24e/0xff0 kernel/locking/mutex.c:621
   [  836.524139] [] pipe_lock_nested
fs/pipe.c:66 [inline]
   [  836.524139] [] pipe_lock+0x56/0x70 fs/pipe.c:74
   [  836.531287] []
iter_file_splice_write+0x262/0xf80 fs/splice.c:717
   [  836.539720] [] do_splice_from
fs/splice.c:869 [inline]
   [  836.539720] [] do_splice fs/splice.c:1160 [inline]
   [  836.539720] [] SYSC_splice fs/splice.c:1410 [inline]
   [  836.539720] [] SyS_splice+0x7c0/0x1690
fs/splice.c:1393
   [  836.547273] [] entry_SYSCALL_64_fastpath+0x1f/0xc2

   [  836.560730] [] validate_chain
kernel/locking/lockdep.c:2265 [inline]
   [  836.560730] []
__lock_acquire+0x2149/0x3430 kernel/locking/lockdep.c:3338
   [  836.568655] [] lock_acquire+0x2a1/0x630
kernel/locking/lockdep.c:3753
   [  836.576230] []
percpu_down_read_preempt_disable include/linux/percpu-rwsem.h:35
[inline]
   [  836.576230] [] percpu_down_read
include/linux/percpu-rwsem.h:58 [inline]
   [  836.576230] []
__sb_start_write+0x19a/0x2b0 fs/super.c:1252
   [  836.584168] [] sb_start_write
include/linux/fs.h:1554 [inline]
   [  836.584168] [] mnt_want_write+0x3f/0xb0
fs/namespace.c:389
   [  836.591744] [] filename_create+0x151/0x610
fs/namei.c:3598
   [  836.599574] [] kern_path_create+0x33/0x40
fs/namei.c:3644
   [  836.607328] [] unix_mknod
net/unix/af_unix.c:967 [inline]
   [  836.607328] [] unix_bind+0x4c3/0xe00
net/unix/af_unix.c:1035
   [  836.614634] [] SYSC_bind+0x20e/0x4a0
net/socket.c:1382
   [  836.621950] [] SyS_bind+0x24/0x30 net/socket.c:1368
   [  836.629015] [] entry_SYSCALL_64_fastpath+0x1f/0xc2

   [  836.642405] [] check_prev_add
kernel/locking/lockdep.c:1828 [inline]
   [  836.642405] []
check_prevs_add+0xa8d/0x1c00 kernel/locking/lockdep.c:1938
   [  836.650348] [] validate_chain
kernel/locking/lockdep.c:2265 [inline]
   [  836.650348] []
__lock_acquire+0x2149/0x3430 kernel/locking/lockdep.c:3338
   [  836.658315] [] lock_acquire+0x2a1/0x630
kernel/locking/lockdep.c:3753
   [  836.665928] [] __mutex_lock_common
kernel/locking/mutex.c:521 [inline]
   [  836.665928] []
mutex_lock_interruptible_nested+0x2e1/0x12a0
kernel/locking/mutex.c:650
   [  836.675287] []
unix_autobind.isra.28+0xc5/0x880 net/unix/af_unix.c:852
   [  836.683571] []
unix_dgram_sendmsg+0x104c/0x1720 net/unix/af_unix.c:1667
   [  836.691870] []
unix_seqpacket_sendmsg+0xf3/0x160 net/unix/af_unix.c:2071
   [  836.700261] [] sock_sendmsg_nosec
net/socket.c:621 [inline]
   [  836.700261] [] sock_sendmsg+0xca/0x110
net/socket.c:631
   [  836.707758] [] kernel_sendmsg+0x47/0x60
net/socket.c:639
   [  836.715327] []
sock_no_sendpage+0x216/0x300 net/core/sock.c:2321
   [  836.723278] [] kernel_sendpage+0x90/0xe0
net/socket.c:3289
   [  836.730944] [] sock_sendpage+0x8c/0xc0
net/socket.c:775
   [  836.738421] []
pipe_to_sendpage+0x29d/0x3e0 fs/splice.c:469
   [  836.746374] [] splice_from_pipe_feed
fs/splice.c:520 [inline]
   [  836.746374] []
__splice_from_pipe+0x328/0x760 fs/splice.c:644
   [

Re: fs, net: deadlock between bind/splice on af_unix

2016-12-08 Thread Al Viro 
On Thu, Dec 08, 2016 at 10:32:00PM -0800, Cong Wang wrote:

> > Why do we do autobind there, anyway, and why is it conditional on
> > SOCK_PASSCRED?  Note that e.g. for SOCK_STREAM we can bloody well get
> > to sending stuff without autobind ever done - just use socketpair()
> > to create that sucker and we won't be going through the connect()
> > at all.
> 
> In the case Dmitry reported, unix_dgram_sendmsg() calls unix_autobind(),
> not SOCK_STREAM.

Yes, I've noticed.  What I'm asking is what in there needs autobind triggered
on sendmsg and why doesn't the same need affect the SOCK_STREAM case?

> I guess some lock, perhaps the u->bindlock could be dropped before
> acquiring the next one (sb_writer), but I need to double check.

Bad idea, IMO - do you *want* autobind being able to come through while
bind(2) is busy with mknod?

Re: fs, net: deadlock between bind/splice on af_unix

2016-12-08 Thread Cong Wang 
On Thu, Dec 8, 2016 at 5:32 PM, Al Viro  wrote:
> On Thu, Dec 08, 2016 at 04:08:27PM -0800, Cong Wang wrote:
>> On Thu, Dec 8, 2016 at 8:30 AM, Dmitry Vyukov  wrote:
>> > Chain exists of:
>> >  Possible unsafe locking scenario:
>> >
>> >CPU0CPU1
>> >
>> >   lock(sb_writers#5);
>> >lock(>bindlock);
>> >lock(sb_writers#5);
>> >   lock(>mutex/1);
>>
>> This looks false positive, probably just needs lockdep_set_class()
>> to set keys for pipe->mutex and unix->bindlock.
>
> I'm afraid that it's not a false positive at all.

Right, I was totally misled by the scenario output of lockdep, the stack
traces actually are much more reasonable.

The deadlock scenario is easy actually, comparing with the netlink one
which has 4 locks involved, it is:

unix_bind() path:
u->bindlock ==> sb_writer

do_splice() path:
sb_writer ==> pipe->mutex ==> u->bindlock

 *** DEADLOCK ***

>
> Why do we do autobind there, anyway, and why is it conditional on
> SOCK_PASSCRED?  Note that e.g. for SOCK_STREAM we can bloody well get
> to sending stuff without autobind ever done - just use socketpair()
> to create that sucker and we won't be going through the connect()
> at all.

In the case Dmitry reported, unix_dgram_sendmsg() calls unix_autobind(),
not SOCK_STREAM.

I guess some lock, perhaps the u->bindlock could be dropped before
acquiring the next one (sb_writer), but I need to double check.

Re: fs, net: deadlock between bind/splice on af_unix

2016-12-08 Thread Al Viro 
On Thu, Dec 08, 2016 at 04:08:27PM -0800, Cong Wang wrote:
> On Thu, Dec 8, 2016 at 8:30 AM, Dmitry Vyukov  wrote:
> > Chain exists of:
> >  Possible unsafe locking scenario:
> >
> >CPU0CPU1
> >
> >   lock(sb_writers#5);
> >lock(>bindlock);
> >lock(sb_writers#5);
> >   lock(>mutex/1);
> 
> This looks false positive, probably just needs lockdep_set_class()
> to set keys for pipe->mutex and unix->bindlock.

I'm afraid that it's not a false positive at all.

Preparations:
* create an AF_UNIX socket.
* set SOCK_PASSCRED on it.
* create a pipe.

Child 1: splice from pipe to socket; locks pipe and proceeds down towards
unix_dgram_sendmsg().

Child 2: splice from pipe to /mnt/foo/bar; requests write access to /mnt
and blocks on attempt to lock the pipe already locked by (1).

Child 3: freeze /mnt; blocks until (2) is done

Child 4: bind() the socket to /mnt/barf; grabs ->bindlock on the socket and
proceeds to create /mnt/barf, which blocks due to fairness of freezer (no
extra write accesses to something that is in process of being frozen).

_Now_ (1) gets around to unix_dgram_sendmsg().  We still have NULL u->addr,
since bind() has not gotten through yet.  We also have SOCK_PASSCRED set,
so we attempt autobind; it blocks on the ->bindlock, which won't be
released until bind() is done (at which point we'll see non-NULL u->addr
and bugger off from autobind), but bind() won't succeed until /mnt
goes through the freeze-thaw cycle, which won't happen until (2) finishes,
which won't happen until (1) unlocks the pipe.  Deadlock.

Granted, ->bindlock is taken interruptibly, so it's not that much of
a problem (you can kill the damn thing), but you would need to intervene
and kill it.

Why do we do autobind there, anyway, and why is it conditional on
SOCK_PASSCRED?  Note that e.g. for SOCK_STREAM we can bloody well get
to sending stuff without autobind ever done - just use socketpair()
to create that sucker and we won't be going through the connect()
at all.

Re: fs, net: deadlock between bind/splice on af_unix

2016-12-08 Thread Cong Wang 
On Thu, Dec 8, 2016 at 8:30 AM, Dmitry Vyukov  wrote:
> Chain exists of:
>  Possible unsafe locking scenario:
>
>CPU0CPU1
>
>   lock(sb_writers#5);
>lock(>bindlock);
>lock(sb_writers#5);
>   lock(>mutex/1);

This looks false positive, probably just needs lockdep_set_class()
to set keys for pipe->mutex and unix->bindlock.

Re: fs, net: deadlock between bind/splice on af_unix

2016-12-08 Thread Dmitry Vyukov 
On Thu, Dec 8, 2016 at 3:47 PM, Dmitry Vyukov  wrote:
> Hello,
>
> I am getting the following deadlock reports while running syzkaller
> fuzzer on 318c8932ddec5c1c26a4af0f3c053784841c598e (Dec 7).
>
>
> [ INFO: possible circular locking dependency detected ]
> 4.9.0-rc8+ #77 Not tainted
> ---
> syz-executor0/3155 is trying to acquire lock:
>  (>bindlock){+.+.+.}, at: []
> unix_autobind.isra.26+0xca/0x8a0 net/unix/af_unix.c:852
> but task is already holding lock:
>  (>mutex/1){+.+.+.}, at: [< inline >] pipe_lock_nested
> fs/pipe.c:66
>  (>mutex/1){+.+.+.}, at: []
> pipe_lock+0x5b/0x70 fs/pipe.c:74
> which lock already depends on the new lock.
>
> the existing dependency chain (in reverse order) is:
>
>[  202.103497] [< inline >] validate_chain
> kernel/locking/lockdep.c:2265
>[  202.103497] []
> __lock_acquire+0x2156/0x3380 kernel/locking/lockdep.c:3338
>[  202.103497] [] lock_acquire+0x2a2/0x790
> kernel/locking/lockdep.c:3749
>[  202.103497] [< inline >] __mutex_lock_common
> kernel/locking/mutex.c:521
>[  202.103497] []
> mutex_lock_nested+0x23f/0xf20 kernel/locking/mutex.c:621
>[  202.103497] [< inline >] pipe_lock_nested fs/pipe.c:66
>[  202.103497] [] pipe_lock+0x5b/0x70 fs/pipe.c:74
>[  202.103497] []
> iter_file_splice_write+0x267/0xfa0 fs/splice.c:717
>[  202.103497] [< inline >] do_splice_from fs/splice.c:869
>[  202.103497] [< inline >] do_splice fs/splice.c:1160
>[  202.103497] [< inline >] SYSC_splice fs/splice.c:1410
>[  202.103497] [] SyS_splice+0x7d7/0x16a0
> fs/splice.c:1393
>[  202.103497] [] entry_SYSCALL_64_fastpath+0x23/0xc6
>
>[  202.103497] [< inline >] validate_chain
> kernel/locking/lockdep.c:2265
>[  202.103497] []
> __lock_acquire+0x2156/0x3380 kernel/locking/lockdep.c:3338
>[  202.103497] [] lock_acquire+0x2a2/0x790
> kernel/locking/lockdep.c:3749
>[  202.103497] [< inline >]
> percpu_down_read_preempt_disable include/linux/percpu-rwsem.h:35
>[  202.103497] [< inline >] percpu_down_read
> include/linux/percpu-rwsem.h:58
>[  202.103497] []
> __sb_start_write+0x193/0x2a0 fs/super.c:1252
>[  202.103497] [< inline >] sb_start_write
> include/linux/fs.h:1549
>[  202.103497] [] mnt_want_write+0x44/0xb0
> fs/namespace.c:389
>[  202.103497] [] filename_create+0x156/0x620
> fs/namei.c:3598
>[  202.103497] [] kern_path_create+0x38/0x50
> fs/namei.c:3644
>[  202.103497] [< inline >] unix_mknod net/unix/af_unix.c:967
>[  202.103497] [] unix_bind+0x4d1/0xe60
> net/unix/af_unix.c:1035
>[  202.103497] [] SYSC_bind+0x20e/0x4c0
> net/socket.c:1382
>[  202.103497] [] SyS_bind+0x29/0x30 
> net/socket.c:1368
>[  202.103497] [] entry_SYSCALL_64_fastpath+0x23/0xc6
>
>[  202.103497] [< inline >] check_prev_add
> kernel/locking/lockdep.c:1828
>[  202.103497] []
> check_prevs_add+0xaab/0x1c20 kernel/locking/lockdep.c:1938
>[  202.103497] [< inline >] validate_chain
> kernel/locking/lockdep.c:2265
>[  202.103497] []
> __lock_acquire+0x2156/0x3380 kernel/locking/lockdep.c:3338
>[  202.103497] [] lock_acquire+0x2a2/0x790
> kernel/locking/lockdep.c:3749
>[  202.103497] [< inline >] __mutex_lock_common
> kernel/locking/mutex.c:521
>[  202.103497] []
> mutex_lock_interruptible_nested+0x2d2/0x11d0
> kernel/locking/mutex.c:650
>[  202.103497] []
> unix_autobind.isra.26+0xca/0x8a0 net/unix/af_unix.c:852
>[  202.103497] []
> unix_dgram_sendmsg+0x105d/0x1730 net/unix/af_unix.c:1667
>[  202.103497] []
> unix_seqpacket_sendmsg+0xf8/0x170 net/unix/af_unix.c:2071
>[  202.103497] [< inline >] sock_sendmsg_nosec net/socket.c:621
>[  202.103497] [] sock_sendmsg+0xcf/0x110
> net/socket.c:631
>[  202.103497] [] kernel_sendmsg+0x4c/0x60
> net/socket.c:639
>[  202.103497] []
> sock_no_sendpage+0x20d/0x310 net/core/sock.c:2321
>[  202.103497] [] kernel_sendpage+0x95/0xf0
> net/socket.c:3289
>[  202.103497] [] sock_sendpage+0xa2/0xd0
> net/socket.c:775
>[  202.103497] []
> pipe_to_sendpage+0x2ae/0x390 fs/splice.c:469
>[  202.103497] [< inline >] splice_from_pipe_feed 
> fs/splice.c:520
>[  202.103497] []
> __splice_from_pipe+0x31f/0x750 fs/splice.c:644
>[  202.103497] []
> splice_from_pipe+0x1dc/0x300 fs/splice.c:679
>[  202.103497] []
> generic_splice_sendpage+0x45/0x60 fs/splice.c:850
>[  202.103497] [< inline >] do_splice_from fs/splice.c:869
>[  202.103497] [< inline >] do_splice fs/splice.c:1160
>[  202.103497] [< inline >] SYSC_splice fs/splice.c:1410
>[  202.103497] []

fs, net: deadlock between bind/splice on af_unix

2016-12-08 Thread Dmitry Vyukov 
Hello,

I am getting the following deadlock reports while running syzkaller
fuzzer on 318c8932ddec5c1c26a4af0f3c053784841c598e (Dec 7).


[ INFO: possible circular locking dependency detected ]
4.9.0-rc8+ #77 Not tainted
---
syz-executor0/3155 is trying to acquire lock:
 (>bindlock){+.+.+.}, at: []
unix_autobind.isra.26+0xca/0x8a0 net/unix/af_unix.c:852
but task is already holding lock:
 (>mutex/1){+.+.+.}, at: [< inline >] pipe_lock_nested
fs/pipe.c:66
 (>mutex/1){+.+.+.}, at: []
pipe_lock+0x5b/0x70 fs/pipe.c:74
which lock already depends on the new lock.

the existing dependency chain (in reverse order) is:

   [  202.103497] [< inline >] validate_chain
kernel/locking/lockdep.c:2265
   [  202.103497] []
__lock_acquire+0x2156/0x3380 kernel/locking/lockdep.c:3338
   [  202.103497] [] lock_acquire+0x2a2/0x790
kernel/locking/lockdep.c:3749
   [  202.103497] [< inline >] __mutex_lock_common
kernel/locking/mutex.c:521
   [  202.103497] []
mutex_lock_nested+0x23f/0xf20 kernel/locking/mutex.c:621
   [  202.103497] [< inline >] pipe_lock_nested fs/pipe.c:66
   [  202.103497] [] pipe_lock+0x5b/0x70 fs/pipe.c:74
   [  202.103497] []
iter_file_splice_write+0x267/0xfa0 fs/splice.c:717
   [  202.103497] [< inline >] do_splice_from fs/splice.c:869
   [  202.103497] [< inline >] do_splice fs/splice.c:1160
   [  202.103497] [< inline >] SYSC_splice fs/splice.c:1410
   [  202.103497] [] SyS_splice+0x7d7/0x16a0
fs/splice.c:1393
   [  202.103497] [] entry_SYSCALL_64_fastpath+0x23/0xc6

   [  202.103497] [< inline >] validate_chain
kernel/locking/lockdep.c:2265
   [  202.103497] []
__lock_acquire+0x2156/0x3380 kernel/locking/lockdep.c:3338
   [  202.103497] [] lock_acquire+0x2a2/0x790
kernel/locking/lockdep.c:3749
   [  202.103497] [< inline >]
percpu_down_read_preempt_disable include/linux/percpu-rwsem.h:35
   [  202.103497] [< inline >] percpu_down_read
include/linux/percpu-rwsem.h:58
   [  202.103497] []
__sb_start_write+0x193/0x2a0 fs/super.c:1252
   [  202.103497] [< inline >] sb_start_write
include/linux/fs.h:1549
   [  202.103497] [] mnt_want_write+0x44/0xb0
fs/namespace.c:389
   [  202.103497] [] filename_create+0x156/0x620
fs/namei.c:3598
   [  202.103497] [] kern_path_create+0x38/0x50
fs/namei.c:3644
   [  202.103497] [< inline >] unix_mknod net/unix/af_unix.c:967
   [  202.103497] [] unix_bind+0x4d1/0xe60
net/unix/af_unix.c:1035
   [  202.103497] [] SYSC_bind+0x20e/0x4c0
net/socket.c:1382
   [  202.103497] [] SyS_bind+0x29/0x30 net/socket.c:1368
   [  202.103497] [] entry_SYSCALL_64_fastpath+0x23/0xc6

   [  202.103497] [< inline >] check_prev_add
kernel/locking/lockdep.c:1828
   [  202.103497] []
check_prevs_add+0xaab/0x1c20 kernel/locking/lockdep.c:1938
   [  202.103497] [< inline >] validate_chain
kernel/locking/lockdep.c:2265
   [  202.103497] []
__lock_acquire+0x2156/0x3380 kernel/locking/lockdep.c:3338
   [  202.103497] [] lock_acquire+0x2a2/0x790
kernel/locking/lockdep.c:3749
   [  202.103497] [< inline >] __mutex_lock_common
kernel/locking/mutex.c:521
   [  202.103497] []
mutex_lock_interruptible_nested+0x2d2/0x11d0
kernel/locking/mutex.c:650
   [  202.103497] []
unix_autobind.isra.26+0xca/0x8a0 net/unix/af_unix.c:852
   [  202.103497] []
unix_dgram_sendmsg+0x105d/0x1730 net/unix/af_unix.c:1667
   [  202.103497] []
unix_seqpacket_sendmsg+0xf8/0x170 net/unix/af_unix.c:2071
   [  202.103497] [< inline >] sock_sendmsg_nosec net/socket.c:621
   [  202.103497] [] sock_sendmsg+0xcf/0x110
net/socket.c:631
   [  202.103497] [] kernel_sendmsg+0x4c/0x60
net/socket.c:639
   [  202.103497] []
sock_no_sendpage+0x20d/0x310 net/core/sock.c:2321
   [  202.103497] [] kernel_sendpage+0x95/0xf0
net/socket.c:3289
   [  202.103497] [] sock_sendpage+0xa2/0xd0
net/socket.c:775
   [  202.103497] []
pipe_to_sendpage+0x2ae/0x390 fs/splice.c:469
   [  202.103497] [< inline >] splice_from_pipe_feed fs/splice.c:520
   [  202.103497] []
__splice_from_pipe+0x31f/0x750 fs/splice.c:644
   [  202.103497] []
splice_from_pipe+0x1dc/0x300 fs/splice.c:679
   [  202.103497] []
generic_splice_sendpage+0x45/0x60 fs/splice.c:850
   [  202.103497] [< inline >] do_splice_from fs/splice.c:869
   [  202.103497] [< inline >] do_splice fs/splice.c:1160
   [  202.103497] [< inline >] SYSC_splice fs/splice.c:1410
   [  202.103497] [] SyS_splice+0x7d7/0x16a0
fs/splice.c:1393
   [  202.103497] [] entry_SYSCALL_64_fastpath+0x23/0xc6

other info that might help us debug this:

Chain exists of:
 Possible unsafe locking scenario:

   CPU0CPU1
   
  lock(>mutex/1);