On Fri, Jul 26, 2019 at 11:11 AM Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > On Thu, Jul 25, 2019 at 7:24 AM syzbot > <syzbot+032bc63605089a199d30@xxxxxxxxxxxxxxxxxxxxxxxxx> wrote: > > > > syzbot has bisected this bug to: > > > > commit 387e3746d01c34457d6a73688acd90428725070b > > Author: Amir Goldstein <amir73il@xxxxxxxxx> > > Date: Fri Jun 7 14:24:38 2019 +0000 > > > > locks: eliminate false positive conflicts for write lease > > > > bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=15a79594600000 > > start commit: c6dd78fc Merge branch 'x86-urgent-for-linus' of git://git... > > git tree: upstream > > final crash: https://syzkaller.appspot.com/x/report.txt?x=17a79594600000 > > console output: https://syzkaller.appspot.com/x/log.txt?x=13a79594600000 > > kernel config: https://syzkaller.appspot.com/x/.config?x=3c8985c08e1f9727 > > dashboard link: https://syzkaller.appspot.com/bug?extid=032bc63605089a199d30 > > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15855334600000 > > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=17fcc4c8600000 > > > > Reported-by: syzbot+032bc63605089a199d30@xxxxxxxxxxxxxxxxxxxxxxxxx > > Fixes: 387e3746d01c ("locks: eliminate false positive conflicts for write > > lease") > > > > For information about bisection process see: https://goo.gl/tpsmEJ#bisection > > The repro: > #{"repeat":true,"procs":1,"sandbox":"none","fault_call":-1,"cgroups":true,"close_fds":true,"tmpdir":true} > mkdir(&(0x7f0000000100)='./file0\x00', 0x0) > mkdirat$cgroup_root(0xffffffffffffff9c, > &(0x7f0000000000)='./cgroup.net/syz1\x00', 0x1ff) > mount$fuse(0x20000000, &(0x7f0000000140)='./file0\x00', 0x0, 0x1004, 0x0) > mount$overlay(0x400000, &(0x7f0000000100)='./file0\x00', > &(0x7f00000001c0)='overlay\x00', 0x0, > &(0x7f0000000040)=ANY=[@ANYBLOB=',lowerdir=.:file0']) > r0 = open(&(0x7f0000000500)='./file0\x00', 0x0, 0x0) > r1 = openat$cgroup_procs(r0, &(0x7f00000004c0)='cgroup.procs\x00', 0x48, 0x0) > dup3(r1, r0, 0x0) > fcntl$setlease(r0, 0x400, 0x1) > lseek(r0, 0x4, 0x0) > > I though we would stop these family of overlapping layers fuzzers with: > 146d62e5a586 ("ovl: detect overlapping layers") > > But syzbot got the upper hand, because we do not check for overlapping layers > that cross fs boundary. Not sure if we should (?). No, we shouldn't care about that. overlayfs doesn't follow cross-fs in underlying layers. > > ./ is a tmpfs dir and ./file0/ is some kind of fuse mount (?) > then after one cycle, ./file0/ itself is an overlapping overlay mount > (lowerdir=./:./file0/) > and after another cycle, ./file0/ is a nested overlapping overlayfs mount. > Fine. Whatever. But damage can still be created if a lower overlayfs layer overlaps with the another nested overlay lower underlying layer. It actually shouldn't be too hard to add a guard also on the nested overlay lower underlying layer inode. > > What I don't understand is if dup3 succeeds r0 should not be an overlayfs fd > and even if dup3 fails r0 should be an overlayfs directory fd (./file0/), so how > the hell did we get to ovl_llseek => ... ovl_change_flags() with this repro?? > > There is not a single regular file in this test. > I was wrong here of course. ./file0/cgroup.procs is a regular overlayfs file (I was confused by the name) which is later also exposed at ./file0/file0/cgroup.procs in the nested overlay mount. Still not sure about the rest of the way to ovl_change_flags() failure, but I think I'll try to block this new syzbot overlap attack. Thanks, Amir.