On Tue, Aug 16, 2022 at 4:20 AM NeilBrown <neilb@xxxxxxx> wrote:
>
>
> nfs_unlink() calls d_delete() twice if it receives ENOENT from the
> server - once in nfs_dentry_handle_enoent() from nfs_safe_remove and
> once in nfs_dentry_remove_handle_error().
>
> nfs_rmddir() also calls it twice - the nfs_dentry_handle_enoent() call
> is direct and inside a region locked with ->rmdir_sem
>
> It is safe to call d_delete() twice if the refcount > 1 as the dentry is
> simply unhashed.
> If the refcount is 1, the first call sets d_inode to NULL and the second
> call crashes.
>
> Refcount is almost always 1 in nfs_unlink() so this must almost never
> happen else we would have seen crashes before.
>
> This patch removes the d_delete() call from nfs_dentry_handle_enoent()
> so as to leave the one under ->remdir_sem in case that is important.
>
> Fixes: 9019fb391de0 ("NFS: Label the dentry with a verifier in nfs_rmdir() and nfs_unlink()")
> Signed-off-by: NeilBrown <neilb@xxxxxxx>
This patch addresses the problem I previously reported. So Tested-by.
> ---
> fs/nfs/dir.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
> index dbab3caa15ed..4648b421025c 100644
> --- a/fs/nfs/dir.c
> +++ b/fs/nfs/dir.c
> @@ -2382,7 +2382,6 @@ static void nfs_dentry_remove_handle_error(struct inode *dir,
> {
> switch (error) {
> case -ENOENT:
> - d_delete(dentry);
> nfs_set_verifier(dentry, nfs_save_change_attribute(dir));
> break;
> case 0:
> --
> 2.37.1
>
