| Message ID | 166063173439.5425.8345694210902041629@noble.neil.brown.name (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | NFS: unlink/rmdir shouldn't call d_delete() twice on ENOENT | expand |
On Tue, Aug 16, 2022 at 4:20 AM NeilBrown <neilb@suse.de> 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@suse.de> 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 >
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:
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@suse.de> --- fs/nfs/dir.c | 1 - 1 file changed, 1 deletion(-)