Re: WARNING in ovl_instantiate

[Amir Goldstein]

On Mon, Apr 22, 2019 at 1:12 PM Dmitry Vyukov <dvyukov@xxxxxxxxxx> wrote:
>
> On Fri, Apr 19, 2019 at 11:21 AM Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> >
> > On Tue, Mar 26, 2019 at 2:10 PM syzbot
> > <syzbot+9c69c282adc4edd2b540@xxxxxxxxxxxxxxxxxxxxxxxxx> wrote:
> > >
> > > syzbot has bisected this bug to:
> > >
> > > commit 01b39dcc95680b04c7af5de7f39f577e9c4865e3
> > > Author: Amir Goldstein <amir73il@xxxxxxxxx>
> > > Date:   Fri May 11 08:15:15 2018 +0000
> > >
> > >      ovl: use inode_insert5() to hash a newly created inode
> > >
> > > bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=176da0cd200000
> > > start commit:   de6629eb Merge tag 'pci-v5.0-fixes-1' of git://git.kernel...
> > > git tree:       upstream
> > > final crash:    https://syzkaller.appspot.com/x/report.txt?x=14eda0cd200000
> > > console output: https://syzkaller.appspot.com/x/log.txt?x=10eda0cd200000
> > > kernel config:  https://syzkaller.appspot.com/x/.config?x=edf1c3031097c304
> > > dashboard link: https://syzkaller.appspot.com/bug?extid=9c69c282adc4edd2b540
> > > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=12c7a94f400000
> > >
> > > Reported-by: syzbot+9c69c282adc4edd2b540@xxxxxxxxxxxxxxxxxxxxxxxxx
> > > Fixes: 01b39dcc9568 ("ovl: use inode_insert5() to hash a newly created
> > > inode")
> > >
> >
> > Dmitry,
> >
> > The root cause of this bug is that repro is mounting overlapping overlay
> > layers (i.e. upperdir=./file0,lowerdir=.:file0).
> > Miklos claimed that the fix should be to fail such mounts.
> > Below is a patch to test:
> >
> > #syz test: https://github.com/amir73il/linux.git ovl-check-overlap
> >
> > However, I see that this specific overlapping layers mount has already
> > mutated to several other repros out there, like the ones in this bug:
> >
> > https://syzkaller.appspot.com/bug?extid=a55ccfc8a853d3cff213
> >
> > I believe that disallowing overlapping layers will silence some
> > bugs, whose root cause may be different.
> >
> > Besides doing the overlapping layers mount, this repro family also
> > does extensive access to overlay underlying layers concurrently
> > with overlay access and *that* is the root cause for most of these
> > "possible deadlock" bugs (some false positives and some real).
> >
> > Assuming that ovl-check-overlap will get merged, you may need to
> > hint syzkaller that overlapping layers is no longer a valid input or
> > maybe it will figure it out on its own?...
>
> Hi Amir,
>
> It should figure it out on its own, it's coverage-guided fuzzer. And
> unlearning things is easier then learning them :) But thanks for
> thinking about this.
> But maybe there is something else important in overlayfs that's not
> covered. Here you can see the current coverage of overlayfs:
> https://storage.googleapis.com/syzkaller/cover/ci-upstream-linux-next-kasan-gce-root.html#e2f448f0ca2e4397fd609ff8c42d4cd118411148

That's nice, but the actual possible deadlocks that syzbot has currently
unveiled are not strictly by covering overlayfs code but rather by covering
VFS code that is *also* used by overlayfs.

See this thread for example:
https://lore.kernel.org/lkml/CAJfpegvt6eVhX8v5faMP76K0LEkqKFDQE8gyNOinxonRdjq3eA@xxxxxxxxxxxxxx/

Documentation/filesystems/overlayfs.txt says:
"Changes to the underlying filesystems while part of a mounted overlay
filesystem are not allowed.  If the underlying filesystem is changed,
the behavior of the overlay is undefined, though it will not result in
a crash or deadlock."

The part of "will not result in crash or deadlock" is only proven
empirically, so long as syzbot is not reproducing a crash or deadlock...

Thanks,
Amir.