Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1497890 > unrolled thread

Re: net: BUG still has locks held in unix_stream_splice_read

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: net: BUG still has locks held in unix_stream_splice_read Dmitry Vyukov <dvyukov@google.com> - 2016-10-09 12:10 +0200
    Re: net: BUG still has locks held in unix_stream_splice_read Al Viro <viro@ZenIV.linux.org.uk> - 2016-10-10 04:50 +0200
      Re: net: BUG still has locks held in unix_stream_splice_read Al Viro <viro@ZenIV.linux.org.uk> - 2016-10-10 05:20 +0200
        Re: net: BUG still has locks held in unix_stream_splice_read Dmitry Vyukov <dvyukov@google.com> - 2016-10-10 10:10 +0200

#1497890 — Re: net: BUG still has locks held in unix_stream_splice_read

Hello,

While running syzkaller fuzzer on commit
b66484cd74706fa8681d051840fe4b18a3da40ff (Oct 7), I am getting:

[ BUG: syz-executor/15138 still has locks held! ]
4.8.0+ #29 Not tainted
-------------------------------------
1 lock held by syz-executor/15138:
 #0:  (&pipe->mutex/1){+.+.+.}, at: [<     inline     >]
pipe_lock_nested fs/pipe.c:66
 #0:  (&pipe->mutex/1){+.+.+.}, at: [<ffffffff81844c8b>]
pipe_lock+0x5b/0x70 fs/pipe.c:74

stack backtrace:
CPU: 1 PID: 15138 Comm: syz-executor Not tainted 4.8.0+ #29
Hardware name: Google Google Compute Engine/Google Compute Engine,
BIOS Google 01/01/2011
 ffff880044d4fa38 ffffffff82d383c9 ffffffff00000000 fffffbfff1097248
 ffff88005a44a3c0 ffff88005a44a3c0 dffffc0000000000 ffff88005a44a3c0
 ffff8800541fb9b8 ffff880044d4fa58 ffffffff81463cd5 0000000000000000
Call Trace:
 [<     inline     >] __dump_stack lib/dump_stack.c:15
 [<ffffffff82d383c9>] dump_stack+0x12e/0x185 lib/dump_stack.c:51
 [<     inline     >] print_held_locks_bug kernel/locking/lockdep.c:4296
 [<ffffffff81463cd5>] debug_check_no_locks_held+0x125/0x140
kernel/locking/lockdep.c:4302
 [<     inline     >] try_to_freeze include/linux/freezer.h:65
 [<     inline     >] freezer_count include/linux/freezer.h:127
 [<     inline     >] freezable_schedule_timeout include/linux/freezer.h:192
 [<     inline     >] unix_stream_data_wait net/unix/af_unix.c:2223
 [<ffffffff860bae67>] unix_stream_read_generic+0x1317/0x1b70
net/unix/af_unix.c:2332
 [<ffffffff860bb81b>] unix_stream_splice_read+0x15b/0x1d0
net/unix/af_unix.c:2506
 [<ffffffff85afc56e>] sock_splice_read+0xbe/0x100 net/socket.c:775
 [<ffffffff818d121f>] do_splice_to+0x10f/0x170 fs/splice.c:908
 [<     inline     >] do_splice fs/splice.c:1196
 [<     inline     >] SYSC_splice fs/splice.c:1420
 [<ffffffff818d6aec>] SyS_splice+0x114c/0x15b0 fs/splice.c:1403
 [<ffffffff86da6d05>] entry_SYSCALL_64_fastpath+0x23/0xc6


I suspect this is:

commit 25869262ef7af24ccde988867ac3eb1c3d4b88d4
Author: Al Viro <viro@zeniv.linux.org.uk>
Date:   Sat Sep 17 21:02:10 2016 -0400
    skb_splice_bits(): get rid of callback
    since pipe_lock is the outermost now, we don't need to drop/regain
    socket locks around the call of splice_to_pipe() from skb_splice_bits(),
    which kills the need to have a socket-specific callback; we can just
    call splice_to_pipe() and be done with that.

[toc] | [next] | [standalone]

#1498066

On Sun, Oct 09, 2016 at 12:06:14PM +0200, Dmitry Vyukov wrote:
> I suspect this is:
> 
> commit 25869262ef7af24ccde988867ac3eb1c3d4b88d4
> Author: Al Viro <viro@zeniv.linux.org.uk>
> Date:   Sat Sep 17 21:02:10 2016 -0400
>     skb_splice_bits(): get rid of callback
>     since pipe_lock is the outermost now, we don't need to drop/regain
>     socket locks around the call of splice_to_pipe() from skb_splice_bits(),
>     which kills the need to have a socket-specific callback; we can just
>     call splice_to_pipe() and be done with that.

Unlikely, since that particular commit removes unlocking/relocking ->iolock
around the call of splice_to_pipe().  Original would've retaken the same
lock on the way out; it's not as if we could leave the syscall there.

It might be splice-related, but I don't believe that you've got the right
commit here.

[toc] | [prev] | [next] | [standalone]

#1498073

On Mon, Oct 10, 2016 at 03:46:07AM +0100, Al Viro wrote:
> On Sun, Oct 09, 2016 at 12:06:14PM +0200, Dmitry Vyukov wrote:
> > I suspect this is:
> > 
> > commit 25869262ef7af24ccde988867ac3eb1c3d4b88d4
> > Author: Al Viro <viro@zeniv.linux.org.uk>
> > Date:   Sat Sep 17 21:02:10 2016 -0400
> >     skb_splice_bits(): get rid of callback
> >     since pipe_lock is the outermost now, we don't need to drop/regain
> >     socket locks around the call of splice_to_pipe() from skb_splice_bits(),
> >     which kills the need to have a socket-specific callback; we can just
> >     call splice_to_pipe() and be done with that.
> 
> Unlikely, since that particular commit removes unlocking/relocking ->iolock
> around the call of splice_to_pipe().  Original would've retaken the same
> lock on the way out; it's not as if we could leave the syscall there.
> 
> It might be splice-related, but I don't believe that you've got the right
> commit here.

It's not that commit, all right - it's "can't call unix_stream_read_generic()
with any locks held" stepped onto a couple of commits prior by
"splice: lift pipe_lock out of splice_to_pipe()".  Could somebody explain
what is that about?

E.g what will happen if some code does a read on AF_UNIX socket with
some local mutex held?  AFAICS, there are exactly two callers of
freezable_schedule_timeout() - this one and one in XFS; the latter is
in a kernel thread where we do have good warranties about the locking
environment, but here it's in the bleeding ->recvmsg/->splice_read and
for those assumption that caller doesn't hold any locks is pretty
strong, especially since it's not documented anywhere.

What's going on there?

[toc] | [prev] | [next] | [standalone]

#1498157

On Mon, Oct 10, 2016 at 5:14 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Mon, Oct 10, 2016 at 03:46:07AM +0100, Al Viro wrote:
>> On Sun, Oct 09, 2016 at 12:06:14PM +0200, Dmitry Vyukov wrote:
>> > I suspect this is:
>> >
>> > commit 25869262ef7af24ccde988867ac3eb1c3d4b88d4
>> > Author: Al Viro <viro@zeniv.linux.org.uk>
>> > Date:   Sat Sep 17 21:02:10 2016 -0400
>> >     skb_splice_bits(): get rid of callback
>> >     since pipe_lock is the outermost now, we don't need to drop/regain
>> >     socket locks around the call of splice_to_pipe() from skb_splice_bits(),
>> >     which kills the need to have a socket-specific callback; we can just
>> >     call splice_to_pipe() and be done with that.
>>
>> Unlikely, since that particular commit removes unlocking/relocking ->iolock
>> around the call of splice_to_pipe().  Original would've retaken the same
>> lock on the way out; it's not as if we could leave the syscall there.
>>
>> It might be splice-related, but I don't believe that you've got the right
>> commit here.
>
> It's not that commit

It's highly likely. Sorry for falsely pointing to your commit.


> , all right - it's "can't call unix_stream_read_generic()
> with any locks held" stepped onto a couple of commits prior by
> "splice: lift pipe_lock out of splice_to_pipe()".  Could somebody explain
> what is that about?
>
> E.g what will happen if some code does a read on AF_UNIX socket with
> some local mutex held?  AFAICS, there are exactly two callers of
> freezable_schedule_timeout() - this one and one in XFS; the latter is
> in a kernel thread where we do have good warranties about the locking
> environment, but here it's in the bleeding ->recvmsg/->splice_read and
> for those assumption that caller doesn't hold any locks is pretty
> strong, especially since it's not documented anywhere.
>
> What's going on there?

I never saw that warning before. There is some possibility that fuzzer
has discovered some new paths, but it's much more likely that
something has changed recently (the stack looks quite simple -- just a
splice from unix socket). And my previous pull was like a week ago.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web