2018-07-12 15:49:55

by syzbot

[permalink] [raw]
Subject: WARNING: lock held when returning to user space in fuse_lock_inode

Hello,

syzbot found the following crash on:

HEAD commit: c25c74b7476e Merge tag 'trace-v4.18-rc3-2' of git://git.ke..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=177bcec2400000
kernel config: https://syzkaller.appspot.com/x/.config?x=25856fac4e580aa7
dashboard link: https://syzkaller.appspot.com/bug?extid=3f7b29af1baa9d0a55be
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=13aa7678400000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=17492678400000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: [email protected]

random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)

================================================
WARNING: lock held when returning to user space!
4.18.0-rc4+ #143 Not tainted
------------------------------------------------
syz-executor012/4539 is leaving the kernel with locks still held!
1 lock held by syz-executor012/4539:
#0: (____ptrval____) (&fi->mutex){+.+.}, at: fuse_lock_inode+0xaf/0xe0
fs/fuse/inode.c:363


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at [email protected].

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches


2018-07-17 11:15:47

by Miklos Szeredi

[permalink] [raw]
Subject: Re: WARNING: lock held when returning to user space in fuse_lock_inode

On Thu, Jul 12, 2018 at 5:49 PM, syzbot
<[email protected]> wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: c25c74b7476e Merge tag 'trace-v4.18-rc3-2' of git://git.ke..
> git tree: upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=177bcec2400000
> kernel config: https://syzkaller.appspot.com/x/.config?x=25856fac4e580aa7
> dashboard link: https://syzkaller.appspot.com/bug?extid=3f7b29af1baa9d0a55be
> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
> syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=13aa7678400000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=17492678400000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: [email protected]
>
> random: sshd: uninitialized urandom read (32 bytes read)
> random: sshd: uninitialized urandom read (32 bytes read)
> random: sshd: uninitialized urandom read (32 bytes read)
>
> ================================================
> WARNING: lock held when returning to user space!
> 4.18.0-rc4+ #143 Not tainted
> ------------------------------------------------
> syz-executor012/4539 is leaving the kernel with locks still held!
> 1 lock held by syz-executor012/4539:
> #0: (____ptrval____) (&fi->mutex){+.+.}, at: fuse_lock_inode+0xaf/0xe0
> fs/fuse/inode.c:363

False positive.

fi->mutex is definitely not held by the acquiring task when returning
to userspace. Maybe syzkaller is confused by the fact that there are
several interdependent tasks involved with fuse: the one calling into
fuse by doing something (looking up ./file0/file0) and the one that
reads the fuse device (returning with the LOOKUP request for "file0").
The second one will return with that lock held, but it's not the one
that acquired it, so there's no bug at all here.

Thanks,
Miklos

>
>
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at [email protected].
>
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
> syzbot.
> syzbot can test patches for this bug, for details see:
> https://goo.gl/tpsmEJ#testing-patches

2018-07-17 11:37:24

by Dmitry Vyukov

[permalink] [raw]
Subject: Re: WARNING: lock held when returning to user space in fuse_lock_inode

On Tue, Jul 17, 2018 at 1:14 PM, Miklos Szeredi <[email protected]> wrote:
> On Thu, Jul 12, 2018 at 5:49 PM, syzbot
> <[email protected]> wrote:
>> Hello,
>>
>> syzbot found the following crash on:
>>
>> HEAD commit: c25c74b7476e Merge tag 'trace-v4.18-rc3-2' of git://git.ke..
>> git tree: upstream
>> console output: https://syzkaller.appspot.com/x/log.txt?x=177bcec2400000
>> kernel config: https://syzkaller.appspot.com/x/.config?x=25856fac4e580aa7
>> dashboard link: https://syzkaller.appspot.com/bug?extid=3f7b29af1baa9d0a55be
>> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
>> syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=13aa7678400000
>> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=17492678400000
>>
>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>> Reported-by: [email protected]
>>
>> random: sshd: uninitialized urandom read (32 bytes read)
>> random: sshd: uninitialized urandom read (32 bytes read)
>> random: sshd: uninitialized urandom read (32 bytes read)
>>
>> ================================================
>> WARNING: lock held when returning to user space!
>> 4.18.0-rc4+ #143 Not tainted
>> ------------------------------------------------
>> syz-executor012/4539 is leaving the kernel with locks still held!
>> 1 lock held by syz-executor012/4539:
>> #0: (____ptrval____) (&fi->mutex){+.+.}, at: fuse_lock_inode+0xaf/0xe0
>> fs/fuse/inode.c:363
>
> False positive.
>
> fi->mutex is definitely not held by the acquiring task when returning
> to userspace. Maybe syzkaller is confused by the fact that there are
> several interdependent tasks involved with fuse: the one calling into
> fuse by doing something (looking up ./file0/file0) and the one that
> reads the fuse device (returning with the LOOKUP request for "file0").
> The second one will return with that lock held, but it's not the one
> that acquired it, so there's no bug at all here.

Hi Miklos,

syzkaller is unrelated here. That's what kernel self-detects and
prints. So either way there is something to fix in kernel here: either
fuse or lockdep.

+Alistair did some analysis offline, hope you don't mind if I repost
your description:
===
Just from reading the code, I think I can see how this happens. Fuse
is wrapping its inode mutex with a check for "parallel_dirops", which
is set up in process_init_reply(). The FUSE_PARALLEL_DIROPS appears to
always be set, in fuse_send_init(), but its initial state is to be
disabled. So if the mutex gets taken, and it'll never be unlocked if
the initial command is flushed by fuse_readdir()'s use of
fuse_lock_inode().
===

2018-07-17 12:47:41

by Miklos Szeredi

[permalink] [raw]
Subject: Re: WARNING: lock held when returning to user space in fuse_lock_inode

On Tue, Jul 17, 2018 at 1:36 PM, Dmitry Vyukov <[email protected]> wrote:
> On Tue, Jul 17, 2018 at 1:14 PM, Miklos Szeredi <[email protected]> wrote:
>> On Thu, Jul 12, 2018 at 5:49 PM, syzbot
>> <[email protected]> wrote:
>>> Hello,
>>>
>>> syzbot found the following crash on:
>>>
>>> HEAD commit: c25c74b7476e Merge tag 'trace-v4.18-rc3-2' of git://git.ke..
>>> git tree: upstream
>>> console output: https://syzkaller.appspot.com/x/log.txt?x=177bcec2400000
>>> kernel config: https://syzkaller.appspot.com/x/.config?x=25856fac4e580aa7
>>> dashboard link: https://syzkaller.appspot.com/bug?extid=3f7b29af1baa9d0a55be
>>> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
>>> syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=13aa7678400000
>>> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=17492678400000
>>>
>>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>>> Reported-by: [email protected]
>>>
>>> random: sshd: uninitialized urandom read (32 bytes read)
>>> random: sshd: uninitialized urandom read (32 bytes read)
>>> random: sshd: uninitialized urandom read (32 bytes read)
>>>
>>> ================================================
>>> WARNING: lock held when returning to user space!
>>> 4.18.0-rc4+ #143 Not tainted
>>> ------------------------------------------------
>>> syz-executor012/4539 is leaving the kernel with locks still held!
>>> 1 lock held by syz-executor012/4539:
>>> #0: (____ptrval____) (&fi->mutex){+.+.}, at: fuse_lock_inode+0xaf/0xe0
>>> fs/fuse/inode.c:363
>>
>> False positive.
>>
>> fi->mutex is definitely not held by the acquiring task when returning
>> to userspace. Maybe syzkaller is confused by the fact that there are
>> several interdependent tasks involved with fuse: the one calling into
>> fuse by doing something (looking up ./file0/file0) and the one that
>> reads the fuse device (returning with the LOOKUP request for "file0").
>> The second one will return with that lock held, but it's not the one
>> that acquired it, so there's no bug at all here.
>
> Hi Miklos,
>
> syzkaller is unrelated here. That's what kernel self-detects and
> prints. So either way there is something to fix in kernel here: either
> fuse or lockdep.
>
> +Alistair did some analysis offline, hope you don't mind if I repost
> your description:
> ===
> Just from reading the code, I think I can see how this happens. Fuse
> is wrapping its inode mutex with a check for "parallel_dirops", which
> is set up in process_init_reply(). The FUSE_PARALLEL_DIROPS appears to
> always be set, in fuse_send_init(), but its initial state is to be
> disabled. So if the mutex gets taken, and it'll never be unlocked if
> the initial command is flushed by fuse_readdir()'s use of
> fuse_lock_inode().
> ===

Ah, indeed. Fix attached.

Thanks,
Miklos


Attachments:
fuse-fix-initial-parallel-dirops.patch (2.59 kB)

2018-07-17 17:14:34

by Alistair Strachan

[permalink] [raw]
Subject: Re: WARNING: lock held when returning to user space in fuse_lock_inode

On Tue, Jul 17, 2018 at 5:46 AM Miklos Szeredi <[email protected]> wrote:
> On Tue, Jul 17, 2018 at 1:36 PM, Dmitry Vyukov <[email protected]> wrote:
> > On Tue, Jul 17, 2018 at 1:14 PM, Miklos Szeredi <[email protected]> wrote:
> >> On Thu, Jul 12, 2018 at 5:49 PM, syzbot
> >> <[email protected]> wrote:
> >>> Hello,
> >>>
> >>> syzbot found the following crash on:
> >>>
> >>> HEAD commit: c25c74b7476e Merge tag 'trace-v4.18-rc3-2' of git://git.ke..
> >>> git tree: upstream
> >>> console output: https://syzkaller.appspot.com/x/log.txt?x=177bcec2400000
> >>> kernel config: https://syzkaller.appspot.com/x/.config?x=25856fac4e580aa7
> >>> dashboard link: https://syzkaller.appspot.com/bug?extid=3f7b29af1baa9d0a55be
> >>> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
> >>> syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=13aa7678400000
> >>> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=17492678400000
> >>>
> >>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> >>> Reported-by: [email protected]
> >>>
> >>> random: sshd: uninitialized urandom read (32 bytes read)
> >>> random: sshd: uninitialized urandom read (32 bytes read)
> >>> random: sshd: uninitialized urandom read (32 bytes read)
> >>>
> >>> ================================================
> >>> WARNING: lock held when returning to user space!
> >>> 4.18.0-rc4+ #143 Not tainted
> >>> ------------------------------------------------
> >>> syz-executor012/4539 is leaving the kernel with locks still held!
> >>> 1 lock held by syz-executor012/4539:
> >>> #0: (____ptrval____) (&fi->mutex){+.+.}, at: fuse_lock_inode+0xaf/0xe0
> >>> fs/fuse/inode.c:363
> >>
> >> False positive.
> >>
> >> fi->mutex is definitely not held by the acquiring task when returning
> >> to userspace. Maybe syzkaller is confused by the fact that there are
> >> several interdependent tasks involved with fuse: the one calling into
> >> fuse by doing something (looking up ./file0/file0) and the one that
> >> reads the fuse device (returning with the LOOKUP request for "file0").
> >> The second one will return with that lock held, but it's not the one
> >> that acquired it, so there's no bug at all here.
> >
> > Hi Miklos,
> >
> > syzkaller is unrelated here. That's what kernel self-detects and
> > prints. So either way there is something to fix in kernel here: either
> > fuse or lockdep.
> >
> > +Alistair did some analysis offline, hope you don't mind if I repost
> > your description:
> > ===
> > Just from reading the code, I think I can see how this happens. Fuse
> > is wrapping its inode mutex with a check for "parallel_dirops", which
> > is set up in process_init_reply(). The FUSE_PARALLEL_DIROPS appears to
> > always be set, in fuse_send_init(), but its initial state is to be
> > disabled. So if the mutex gets taken, and it'll never be unlocked if
> > the initial command is flushed by fuse_readdir()'s use of
> > fuse_lock_inode().
> > ===
>
> Ah, indeed. Fix attached.

Looks good to me.

Tested-by: Alistair Strachan <[email protected]>

> Thanks,
> Miklos