From: Amir Goldstein Commit 669bb5af15d4 ("fs: make sure to call fsnotify_inoderemove() only once when inode is dead") did two things. It made dentry_unlink_inode() call fsnotify_inoderemove() only once the inode has neither links nor aliases left. And it made __d_obtain_alias() fail with -ESTALE for an inode in that state so that open_by_handle_at() can't create a new alias after FS_DELETE_SELF has been reported and the inode marks are gone. The second part is wrong for network filesystems. __d_obtain_alias() backs d_obtain_alias() and d_obtain_root(). Those are what the ->fh_to_dentry() and ->get_parent() methods use to turn the inode they looked up into a dentry for exportfs_decode_fh_raw(), i.e., for open_by_handle_at() and for nfsd file handle decoding. For a local filesystem an in-core inode without links and aliases is an orphan that goes away on the final iput(). For a network filesystem that's not true. i_nlink is just the link count the server last reported and whether the object is still reachable through its file handle is up to the server. If a file is unlinked while it is still open elsewhere, e.g., by another NFSv4 client or by a process on the server, the server keeps the object alive and its file handle stays valid. nfs_fh_to_dentry() fetches the attributes for such a handle, nfs_fhget() sets i_nlink to 0 from them, the new inode has no aliases yet and so __d_obtain_alias() now fails with -ESTALE. The same applies to fuse_get_dentry() which takes i_nlink from the fuse server. Filesystems that care about unlinked inodes already make that decision themselves in ->fh_to_dentry(), e.g., ceph's __fh_to_dentry() fetches the link count with CEPH_CAP_LINK_SHARED and only refuses an unlinked inode if it isn't open. Note that on a network filesystem i_nlink drops to zero through attribute updates when another client unlinks the file. So the inode might never have gone through dentry_unlink_inode() with i_nlink == 0, i.e., fsnotify_inoderemove() might never have been called for it and there's nothing for the -ESTALE to protect. So drop the two checks from __d_obtain_alias() again and go back to the unconditional alias lookup. While at it, update the comment above inode_notify_dead() since it no longer blocks new aliases. Amir sent this as v3 to replace 669bb5af15d4 but that commit was already in mainline by then. So do it as a fix on top. The resulting code is identical to v3. This means open_by_handle_at() can again create a new alias for an unlinked inode after FS_DELETE_SELF has been reported. Closing that file reports a second FS_DELETE_SELF and the file has none of the inode marks that were torn down. That's what Daehyeon's open_by_handle_at() LTP test checks so it is expected to fail again. This needs to be handled separately. A mark that is added through such a file while fsnotify_destroy_marks() is still running is rejected with -ENOENT since 36ebddca56b9 ("fsnotify: prevent mark additions during connector teardown") so it can't end up on a detached connector. Christian Brauner says: Amir's v3 replaced 669bb5af15d4 as a whole. I turned it into an incremental fix on top of mainline and wrote a new commit message. Jan's Reviewed-by was given on v3 and the resulting code is identical. Fixes: 669bb5af15d4 ("fs: make sure to call fsnotify_inoderemove() only once when inode is dead") Cc: stable@vger.kernel.org Reported-by: Daehyeon Ko <4ncienth@gmail.com> Signed-off-by: Amir Goldstein Link: https://patch.msgid.link/CAOQ4uxg7Qt436SiKhKEJKOjtCjeB_XyTjazYF7HMyQAGGcgx2A@mail.gmail.com Reviewed-by: Jan Kara Signed-off-by: Christian Brauner (Amutable) --- fs/dcache.c | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/fs/dcache.c b/fs/dcache.c index efd9332c8407..810ac48ae753 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -452,10 +452,9 @@ static void dentry_free(struct dentry *dentry) } /* - * If inode is unlinked and doesn't have any aliases (i.e., all fds pointing to - * it are closed), it is pretty much dead. Except that file handle lookup could - * still revive it which causes issues to fsnotify. So once inode reaches this - * state we make sure to block creating any new aliases. + * When inode is unlinked and doesn't have any aliases (i.e., all fds pointing + * to it are closed) we report FS_DELETE_SELF to fsnotify and cleanup all inode + * marks. */ static bool inode_notify_dead(struct inode *inode) { @@ -2312,12 +2311,7 @@ static struct dentry *__d_obtain_alias(struct inode *inode, bool disconnected) sb = inode->i_sb; - spin_lock(&inode->i_lock); - if (!inode_notify_dead(inode)) - res = __d_find_any_alias(inode); /* existing alias? */ - else - res = ERR_PTR(-ESTALE); - spin_unlock(&inode->i_lock); + res = d_find_any_alias(inode); /* existing alias? */ if (res) goto out; @@ -2329,10 +2323,7 @@ static struct dentry *__d_obtain_alias(struct inode *inode, bool disconnected) security_d_instantiate(new, inode); spin_lock(&inode->i_lock); - if (!inode_notify_dead(inode)) - res = __d_find_any_alias(inode); /* recheck under lock */ - else - res = ERR_PTR(-ESTALE); + res = __d_find_any_alias(inode); /* recheck under lock */ if (likely(!res)) { /* still no alias, attach a disconnected dentry */ unsigned add_flags = d_flags_for_inode(inode); --- base-commit: b44341e39d856e4cc6ad8ed94ebffd440f39846c change-id: 20261030-work-dcache-obtain_alias-b03dfb84e37f