On Tue, 30 Nov 2021 at 06:21, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>
> On Mon, Nov 29, 2021 at 11:33 PM Eric Sandeen <sandeen@xxxxxxxxxxx> 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
