linux-kernel - Re: possible deadlock in pipe

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 11 Feb 2019 13:37:43 +0100
From:   Miklos Szeredi <miklos@...redi.hu>
To:     Amir Goldstein <amir73il@...il.com>, Jan Kara <jack@...e.cz>
Cc:     linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        syzkaller-bugs@...glegroups.com, Al Viro <viro@...iv.linux.org.uk>,
        syzbot <syzbot+31d8b84465a7cbfd8515@...kaller.appspotmail.com>,
        overlayfs <linux-unionfs@...r.kernel.org>
Subject: Re: possible deadlock in pipe_lock (2)

On Mon, Feb 11, 2019 at 1:06 PM Miklos Szeredi <miklos@...redi.hu> wrote:
>
> On Mon, Feb 11, 2019 at 8:38 AM Amir Goldstein <amir73il@...il.com> wrote:
> >
> > On Sun, Feb 10, 2019 at 8:23 PM syzbot
> > <syzbot+31d8b84465a7cbfd8515@...kaller.appspotmail.com> wrote:
>
> > > -> #1 (&ovl_i_mutex_key[depth]){+.+.}:
> > >         down_write+0x38/0x90 kernel/locking/rwsem.c:70
> > >         inode_lock include/linux/fs.h:757 [inline]
> > >         ovl_write_iter+0x148/0xc20 fs/overlayfs/file.c:231
> > >         call_write_iter include/linux/fs.h:1863 [inline]
> > >         new_sync_write fs/read_write.c:474 [inline]
> > >         __vfs_write+0x613/0x8e0 fs/read_write.c:487
> > > kobject: 'loop4' (000000009e2b886d): kobject_uevent_env
> > >         __kernel_write+0x110/0x3b0 fs/read_write.c:506
> > >         write_pipe_buf+0x15d/0x1f0 fs/splice.c:797
> > >         splice_from_pipe_feed fs/splice.c:503 [inline]
> > >         __splice_from_pipe+0x39a/0x7e0 fs/splice.c:627
> > >         splice_from_pipe+0x108/0x170 fs/splice.c:662
> > >         default_file_splice_write+0x3c/0x90 fs/splice.c:809
>
> Irrelevant to the lockdep splat, but why isn't there an
> ovl_splice_write() that just recurses into realfile->splice_write()?
> Sounds like a much more efficient way to handle splice read and
> write...
>
> [...]
>
> > Miklos,
> >
> > Its good that this report popped up again, because I went to
> > look back at my notes from previous report [1].
> > If I was right in my previous analysis then we must have a real
> > deadlock in current "lazy copy up" WIP patches. Right?
>
> Hmm, AFAICS this circular dependency translated into layman's terms:
>
> pipe lock -> ovl inode lock  (splice to ovl file)
>
> ovl inode lock -> upper freeze lock (truncate of ovl file)
>
> upper freeze lock -> pipe lock (splice to upper file)

So what can we do with this?

The "freeze lock -> inode lock" dependency is fixed.   This is
reversed in overlay to "ovl inode lock -> upper freeze lock", which is
okay, because this is a nesting that cannot be reversed.   But in
splice the pipe locks comes in between: "freeze lock -> pipe lock ->
inode lock" which breaks this nesting direction and creates a true
reverse dependency between ovl inode lock and upper freeze lock.

The only way I see this could be fixed is to move the freeze lock
inside the pipe lock.  But that would mean splice/sendfile/etc could
be frozen with the pipe lock held.  It doesn't look nice.

Any other ideas?

Thanks,
Miklos