Re: XFS: Assertion failed: !(flags & (RENAME_NOREPLACE | RENAME_EXCHANGE))

[Miklos Szeredi <miklos@szeredi.hu>]

On Tue, 30 Nov 2021 at 06:21, Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Mon, Nov 29, 2021 at 11:33 PM Eric Sandeen <sandeen@sandeen.net> wrote:
> >
> > On 11/26/21 9:56 AM, Paolo Bonzini wrote:
> > > Hi all,
> > >
> > > I have reached the following ASSERT today running a kernel from
> > > git commit 5d9f4cf36721:
> > >
> > >          /*
> > >           * If we are doing a whiteout operation, allocate the whiteout inode
> > >           * we will be placing at the target and ensure the type is set
> > >           * appropriately.
> > >           */
> > >          if (flags & RENAME_WHITEOUT) {
> > >                  ASSERT(!(flags & (RENAME_NOREPLACE | RENAME_EXCHANGE)));
> > >                  error = xfs_rename_alloc_whiteout(mnt_userns, target_dp, &wip);
> > >                  if (error)
> > >                          return error;
> > >
> > >                  /* setup target dirent info as whiteout */
> > >                  src_name->type = XFS_DIR3_FT_CHRDEV;
> > >          }
> >
> >
> > Hmm.  Is our ASSERT correct?  rename(2) says:
> >
> > RENAME_NOREPLACE can't be employed together with RENAME_EXCHANGE.
> > RENAME_WHITEOUT  can't be employed together with RENAME_EXCHANGE.
> >
> > do_renameat2() does enforce this:
> >
> >          if ((flags & (RENAME_NOREPLACE | RENAME_WHITEOUT)) &&
> >              (flags & RENAME_EXCHANGE))
> >                  goto put_names;
> >
> > but our assert seems to check for something different: that neither
> > NOREPLACE nor EXCHANGE is employed with WHITEOUT. Is that a thinko?
>
> Probably.
>
> RENAME_NOREPLACE and RENAME_WHITEOUT are independent -
> The former has to do with the target and enforced by generic vfs.
> The latter has to do with the source and is implemented by specific fs.
>
> Overlayfs adds RENAME_WHITEOUT flag is some cases to a rename
> before performing it on underlying fs (i.e. xfs) to leave a whiteout instead
> of the renamed path, so renameat2(NOREPLACE) on overlayfs could
> end up with (RENAME_NOREPLACE | RENAME_WHITEOUT) to xfs.

Agreed, the assert makes no sense.

Thanks,
Miklos