Re: [PATCH v5 16/22] fsnotify: remove check that source dentry is positive

From: Amir Goldstein <amir73il@gmail.com>
To: Jan Kara <jack@suse.cz>
Cc: linux-fsdevel <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH v5 16/22] fsnotify: remove check that source dentry is positive
Date: Thu, 16 Jul 2020 16:54:08 +0300	[thread overview]
Message-ID: <CAOQ4uxjXQWRtxWbiL8Y_01dMC8ntbYi4QnzpfHQqiJ3pP=i5Vg@mail.gmail.com> (raw)
In-Reply-To: <20200716132941.GD5022@quack2.suse.cz>

On Thu, Jul 16, 2020 at 4:29 PM Jan Kara <jack@suse.cz> wrote:
>
> On Thu 16-07-20 15:13:11, Jan Kara wrote:
> > On Thu 16-07-20 11:42:24, Amir Goldstein wrote:
> > > Remove the unneeded check for positive source dentry in
> > > fsnotify_move().
> > >
> > > fsnotify_move() hook is mostly called from vfs_rename() under
> > > lock_rename() and vfs_rename() starts with may_delete() test that
> > > verifies positive source dentry.  The only other caller of
> > > fsnotify_move() - debugfs_rename() also verifies positive source.
> > >
> > > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> >
> > But in vfs_rename() if RENAME_EXCHANGE is set and target is NULL,
> > new_dentry can be negative when calling fsnotify_move() AFAICT, cannot it?
>
> FWIW, renameat2() doesn't allow RENAME_EXCHANGE with non-existent target
> and I didn't find any other place that could call vfs_rename() with
> RENAME_EXCHANGE and negative target but still vfs_rename() seems to support
> that and so fsnotify should likely handle that as well.

If some code did call vfs_rename() like that d_exchange() will barf:

void d_exchange(struct dentry *dentry1, struct dentry *dentry2)
{
        write_seqlock(&rename_lock);

        WARN_ON(!dentry1->d_inode);
        WARN_ON(!dentry2->d_inode);

Thanks,
Amir.