* WARNING: lock held when returning to user space in fuse_lock_inode @ 2018-07-12 15:49 syzbot 2018-07-17 11:14 ` Miklos Szeredi 0 siblings, 1 reply; 5+ messages in thread From: syzbot @ 2018-07-12 15:49 UTC (permalink / raw) To: linux-fsdevel, linux-kernel, miklos, syzkaller-bugs 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: syzbot+3f7b29af1baa9d0a55be@syzkaller.appspotmail.com 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 syzkaller@googlegroups.com. 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: WARNING: lock held when returning to user space in fuse_lock_inode 2018-07-12 15:49 WARNING: lock held when returning to user space in fuse_lock_inode syzbot @ 2018-07-17 11:14 ` Miklos Szeredi 2018-07-17 11:36 ` Dmitry Vyukov 0 siblings, 1 reply; 5+ messages in thread From: Miklos Szeredi @ 2018-07-17 11:14 UTC (permalink / raw) To: syzbot; +Cc: linux-fsdevel, linux-kernel, syzkaller-bugs On Thu, Jul 12, 2018 at 5:49 PM, syzbot <syzbot+3f7b29af1baa9d0a55be@syzkaller.appspotmail.com> 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: syzbot+3f7b29af1baa9d0a55be@syzkaller.appspotmail.com > > 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 syzkaller@googlegroups.com. > > 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: WARNING: lock held when returning to user space in fuse_lock_inode 2018-07-17 11:14 ` Miklos Szeredi @ 2018-07-17 11:36 ` Dmitry Vyukov 2018-07-17 12:46 ` Miklos Szeredi 0 siblings, 1 reply; 5+ messages in thread From: Dmitry Vyukov @ 2018-07-17 11:36 UTC (permalink / raw) To: Miklos Szeredi; +Cc: syzbot, linux-fsdevel, LKML, syzkaller-bugs, astrachan On Tue, Jul 17, 2018 at 1:14 PM, Miklos Szeredi <miklos@szeredi.hu> wrote: > On Thu, Jul 12, 2018 at 5:49 PM, syzbot > <syzbot+3f7b29af1baa9d0a55be@syzkaller.appspotmail.com> 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: syzbot+3f7b29af1baa9d0a55be@syzkaller.appspotmail.com >> >> 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(). === ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: WARNING: lock held when returning to user space in fuse_lock_inode 2018-07-17 11:36 ` Dmitry Vyukov @ 2018-07-17 12:46 ` Miklos Szeredi 2018-07-17 17:12 ` Alistair Strachan 0 siblings, 1 reply; 5+ messages in thread From: Miklos Szeredi @ 2018-07-17 12:46 UTC (permalink / raw) To: Dmitry Vyukov; +Cc: syzbot, linux-fsdevel, LKML, syzkaller-bugs, astrachan [-- Attachment #1: Type: text/plain, Size: 2908 bytes --] On Tue, Jul 17, 2018 at 1:36 PM, Dmitry Vyukov <dvyukov@google.com> wrote: > On Tue, Jul 17, 2018 at 1:14 PM, Miklos Szeredi <miklos@szeredi.hu> wrote: >> On Thu, Jul 12, 2018 at 5:49 PM, syzbot >> <syzbot+3f7b29af1baa9d0a55be@syzkaller.appspotmail.com> 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: syzbot+3f7b29af1baa9d0a55be@syzkaller.appspotmail.com >>> >>> 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 [-- Attachment #2: fuse-fix-initial-parallel-dirops.patch --] [-- Type: text/x-patch, Size: 2651 bytes --] From: Miklos Szeredi <mszeredi@redhat.com> Subject: fuse: fix inital parallel dirops If parallel dirops are enabled in FUSE_INIT reply, then first operation may leave fi->mutex held. Reported-by: syzbot+3f7b29af1baa9d0a55be@syzkaller.appspotmail.com Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> --- fs/fuse/dir.c | 10 ++++++---- fs/fuse/fuse_i.h | 4 ++-- fs/fuse/inode.c | 14 ++++++++++---- 3 files changed, 18 insertions(+), 10 deletions(-) --- a/fs/fuse/dir.c +++ b/fs/fuse/dir.c @@ -355,11 +355,12 @@ static struct dentry *fuse_lookup(struct struct inode *inode; struct dentry *newent; bool outarg_valid = true; + bool locked; - fuse_lock_inode(dir); + locked = fuse_lock_inode(dir); err = fuse_lookup_name(dir->i_sb, get_node_id(dir), &entry->d_name, &outarg, &inode); - fuse_unlock_inode(dir); + fuse_unlock_inode(dir, locked); if (err == -ENOENT) { outarg_valid = false; err = 0; @@ -1340,6 +1341,7 @@ static int fuse_readdir(struct file *fil struct fuse_conn *fc = get_fuse_conn(inode); struct fuse_req *req; u64 attr_version = 0; + bool locked; if (is_bad_inode(inode)) return -EIO; @@ -1367,9 +1369,9 @@ static int fuse_readdir(struct file *fil fuse_read_fill(req, file, ctx->pos, PAGE_SIZE, FUSE_READDIR); } - fuse_lock_inode(inode); + locked = fuse_lock_inode(inode); fuse_request_send(fc, req); - fuse_unlock_inode(inode); + fuse_unlock_inode(inode, locked); nbytes = req->out.args[0].size; err = req->out.h.error; fuse_put_request(fc, req); --- a/fs/fuse/fuse_i.h +++ b/fs/fuse/fuse_i.h @@ -974,8 +974,8 @@ int fuse_do_setattr(struct dentry *dentr void fuse_set_initialized(struct fuse_conn *fc); -void fuse_unlock_inode(struct inode *inode); -void fuse_lock_inode(struct inode *inode); +void fuse_unlock_inode(struct inode *inode, bool locked); +bool fuse_lock_inode(struct inode *inode); int fuse_setxattr(struct inode *inode, const char *name, const void *value, size_t size, int flags); --- a/fs/fuse/inode.c +++ b/fs/fuse/inode.c @@ -357,15 +357,21 @@ int fuse_reverse_inval_inode(struct supe return 0; } -void fuse_lock_inode(struct inode *inode) +bool fuse_lock_inode(struct inode *inode) { - if (!get_fuse_conn(inode)->parallel_dirops) + bool locked = false; + + if (!get_fuse_conn(inode)->parallel_dirops) { mutex_lock(&get_fuse_inode(inode)->mutex); + locked = true; + } + + return locked; } -void fuse_unlock_inode(struct inode *inode) +void fuse_unlock_inode(struct inode *inode, bool locked) { - if (!get_fuse_conn(inode)->parallel_dirops) + if (locked) mutex_unlock(&get_fuse_inode(inode)->mutex); } ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: WARNING: lock held when returning to user space in fuse_lock_inode 2018-07-17 12:46 ` Miklos Szeredi @ 2018-07-17 17:12 ` Alistair Strachan 0 siblings, 0 replies; 5+ messages in thread From: Alistair Strachan @ 2018-07-17 17:12 UTC (permalink / raw) To: miklos Cc: dvyukov, syzbot+3f7b29af1baa9d0a55be, linux-fsdevel, linux-kernel, syzkaller-bugs On Tue, Jul 17, 2018 at 5:46 AM Miklos Szeredi <miklos@szeredi.hu> wrote: > On Tue, Jul 17, 2018 at 1:36 PM, Dmitry Vyukov <dvyukov@google.com> wrote: > > On Tue, Jul 17, 2018 at 1:14 PM, Miklos Szeredi <miklos@szeredi.hu> wrote: > >> On Thu, Jul 12, 2018 at 5:49 PM, syzbot > >> <syzbot+3f7b29af1baa9d0a55be@syzkaller.appspotmail.com> 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: syzbot+3f7b29af1baa9d0a55be@syzkaller.appspotmail.com > >>> > >>> 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 <astrachan@google.com> > Thanks, > Miklos ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-07-17 17:12 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-07-12 15:49 WARNING: lock held when returning to user space in fuse_lock_inode syzbot 2018-07-17 11:14 ` Miklos Szeredi 2018-07-17 11:36 ` Dmitry Vyukov 2018-07-17 12:46 ` Miklos Szeredi 2018-07-17 17:12 ` Alistair Strachan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).