On Wed, May 8, 2019 at 1:03 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > On Sat, Mar 30, 2019 at 10:45 PM Miklos Szeredi <miklos@xxxxxxxxxx> wrote: > > > > On Thu, Mar 28, 2019 at 4:38 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > > > > > This nasty little syzbot repro: > > > https://syzkaller.appspot.com/x/repro.syz?x=12c7a94f400000 > > > > > > Creates overlay mounts where the same directory is both in upper > > > and lower layers. Simplified example: > > > > > > mkdir foo work > > > mount -t overlay none foo -o"lowerdir=.,upperdir=foo,workdir=work" > > > > Shouldn't the mount fail in this case? > > > > Does it make any sense to allow overlapping layers? > > > > If doing the check the dumb way, the number of d_ancestor() calls > > would increase quadratically with the number of layers, but I think > > it's possible to do it in linear time if necessary. > > > > Miklos, > > I saw you did not pick this one for next. > IMO, regardless of preventing mount with overlapping layers, > the WARN_ON() is inappropriate and this patch that replaces it with > pr_warn_reatelimited() has merit on its own. > > WARN_ON() should reflect a case that we don't think is possible > in current code and as API constrain assertion. > Neither is the case here. > We know that it *is* possible to hit this case, even with checking overlapping > layers on mount and user does get an error when we hit the case. Okay. Picked up this one too. Thanks, Miklos