On Mon, Mar 23, 2020 at 9:15 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Mon, Mar 23, 2020 at 7:27 PM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > On Mon, Mar 23, 2020 at 4:53 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
> > >
> > > On Mon, Mar 23, 2020 at 3:21 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
> > > >
> > > > On Mon, Mar 23, 2020 at 2:24 PM Amir Goldstein <amir73il@gmail.com> wrote:
> > >
> > > > > IDGI. coming from vfs_unlink() and vfs_rename() it doesn't look like
> > > > > it is possible for victim inode not to have a hashed alias, so the
> > > > > alias test seems futile.
> > > >
> > > > Yeah, needs a comment: both ovl_remove_upper() and
> > > > ovl_remove_and_whiteout() unhash the dentry before returning, so
> > > > d_find_alias() will find another hashed dentry or none.
> > >
> > > Except that doesn't seem to be true for the overwriting rename case...
> > >
> > > Attached patch should work for both.
> > >
> >
> > It still looks quite hacky.
> > Why do we not look at upper->i_nlink in order to fix the situation?
> >
> > For index=on, there is already code to handle lower hardlink skew case,
> > including pr_warn and several xfstests (overlay/034 for example).
> > The check is buried in ovl_nlink_end() => ovl_cleanup_index().
> > It's keeping overlay i_nlink above upper i_nlink.
> >
> > In fact, if you change one line in overlay/034 it triggers the reported
> > bug, so we can just fork this test.
> >
> > -lowerdir=$OVL_BASE_SCRATCH_MNT/$OVL_LOWER
> > +lowerdir=$OVL_BASE_SCRATCH_MNT/$OVL_UPPER
> >  workdir=$OVL_BASE_SCRATCH_MNT/$OVL_WORK
> >
> > How about adding a check in ovl_nlink_start() to fix overlay i_nlink
> > below upper i_link?
> > It would be a valid check for both index=off and on.
> > I will try to write it up later.
> >
>
> https://lore.kernel.org/linux-unionfs/20200323190850.3091-1-amir73il@gmail.com/T/#u
>

Attached xfstest. I will post it once the kernel fix commit is finalized.

Thanks,
Amir.