[PATCH] smb: client: fix cifs_close_deferred_file_under_dentry()

[PATCH] smb: client: fix cifs_close_deferred_file_under_dentry()

[Paulo Alcantara]

The dentry passed in cifs_close_deferred_file_under_dentry() could
have been unhashed from its parents hash list and then looked up
again, so matching @cfile->dentry with @dentry would no longer work.
This would then fail to close the deferred file prior to renaming or
unlinking it.

Fix this by matching filenames instead of dentry pointers.

This problem can be reproduced with LTP rename14 testcase:

  rename14 0 TINFO : Using /mnt/1/ltp-a5w7It6Osi/LTP_renffzJE1 as
  tmpdir (unknown filesystem)
  rename14    1  TPASS  :  Test Passed
  rename14 0 TWARN : tst_tmpdir.c:347: tst_rmdir:
  rmobj(/mnt/1/ltp-a5w7It6Osi/LTP_renffzJE1) failed:
  unlink(/mnt/1/ltp-a5w7It6Osi/LTP_renffzJE1/.__smb0021) failed;
  errno=2: ENOENT
  <<<execution_status>>>
  initiation_status="ok"
  duration=5 termination_type=exited termination_id=4 corefile=no
  cutime=0 cstime=587
  <<<test_end>>>
  INFO: ltp-pan reported some tests FAIL
  LTP Version: 20250930-14-g9bb94efa3
         ###############################################################
              Done executing testcases.
              LTP Version:  20250930-14-g9bb94efa3
         ###############################################################
  -------------------------------------------
  INFO: runltp script is deprecated, try kirk
  https://github.com/linux-test-project/kirk
  -------------------------------------------
  rm: cannot remove '/mnt/1/ltp-a5w7It6Osi/LTP_renffzJE1': Directory not empty

Reported-by: Anoop C S <anoopcs@samba.org>
Fixes: 93ed9a295130 ("smb: client: fix filename matching of deferred files")
Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Xiaoli Feng <xifeng@redhat.com>
Cc: linux-cifs@vger.kernel.org
---
 fs/smb/client/cifsproto.h | 2 +-
 fs/smb/client/inode.c     | 6 +++---
 fs/smb/client/misc.c      | 8 ++++++--
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
index fb1813cbe0eb..65abbb5041b8 100644
--- a/fs/smb/client/cifsproto.h
+++ b/fs/smb/client/cifsproto.h
@@ -314,7 +314,7 @@ extern void cifs_close_deferred_file(struct cifsInodeInfo *cifs_inode);
 extern void cifs_close_all_deferred_files(struct cifs_tcon *cifs_tcon);
 
 void cifs_close_deferred_file_under_dentry(struct cifs_tcon *cifs_tcon,
-					   struct dentry *dentry);
+					   const char *full_path);
 
 extern void cifs_mark_open_handles_for_deleted_file(struct inode *inode,
 				const char *path);
diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index 098a79b7a959..1a4369abba32 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -1984,7 +1984,7 @@ static int __cifs_unlink(struct inode *dir, struct dentry *dentry, bool sillyren
 	}
 
 	netfs_wait_for_outstanding_io(inode);
-	cifs_close_deferred_file_under_dentry(tcon, dentry);
+	cifs_close_deferred_file_under_dentry(tcon, full_path);
 #ifdef CONFIG_CIFS_ALLOW_INSECURE_LEGACY
 	if (cap_unix(tcon->ses) && (CIFS_UNIX_POSIX_PATH_OPS_CAP &
 				le64_to_cpu(tcon->fsUnixInfo.Capability))) {
@@ -2553,10 +2553,10 @@ cifs_rename2(struct mnt_idmap *idmap, struct inode *source_dir,
 		goto cifs_rename_exit;
 	}
 
-	cifs_close_deferred_file_under_dentry(tcon, source_dentry);
+	cifs_close_deferred_file_under_dentry(tcon, from_name);
 	if (d_inode(target_dentry) != NULL) {
 		netfs_wait_for_outstanding_io(d_inode(target_dentry));
-		cifs_close_deferred_file_under_dentry(tcon, target_dentry);
+		cifs_close_deferred_file_under_dentry(tcon, to_name);
 	}
 
 	rc = cifs_do_rename(xid, source_dentry, from_name, target_dentry,
diff --git a/fs/smb/client/misc.c b/fs/smb/client/misc.c
index 340c44dc7b5b..0b1b25c6e0cc 100644
--- a/fs/smb/client/misc.c
+++ b/fs/smb/client/misc.c
@@ -834,15 +834,18 @@ cifs_close_all_deferred_files(struct cifs_tcon *tcon)
 }
 
 void cifs_close_deferred_file_under_dentry(struct cifs_tcon *tcon,
-					   struct dentry *dentry)
+					   const char *full_path)
 {
 	struct file_list *tmp_list, *tmp_next_list;
+	void *page = alloc_dentry_path();
 	struct cifsFileInfo *cfile;
 	LIST_HEAD(file_head);
 
 	spin_lock(&tcon->open_file_lock);
 	list_for_each_entry(cfile, &tcon->openFileList, tlist) {
-		if ((cfile->dentry == dentry) &&
+		const char *path = build_path_from_dentry(cfile->dentry, page);
+
+		if (!IS_ERR(path) && !strcmp(full_path, path) &&
 		    delayed_work_pending(&cfile->deferred) &&
 		    cancel_delayed_work(&cfile->deferred)) {
 			spin_lock(&CIFS_I(d_inode(cfile->dentry))->deferred_lock);
@@ -863,6 +866,7 @@ void cifs_close_deferred_file_under_dentry(struct cifs_tcon *tcon,
 		list_del(&tmp_list->list);
 		kfree(tmp_list);
 	}
+	free_dentry_path(page);
 }
 
 /*
-- 
2.51.0

Re: [PATCH] smb: client: fix cifs_close_deferred_file_under_dentry()

[David Howells]

Paulo Alcantara <pc@manguebit.org> wrote:

> The dentry passed in cifs_close_deferred_file_under_dentry() could
> have been unhashed from its parents hash list and then looked up
> again, so matching @cfile->dentry with @dentry would no longer work.
> This would then fail to close the deferred file prior to renaming or
> unlinking it.
> 
> Fix this by matching filenames instead of dentry pointers.
> 
> This problem can be reproduced with LTP rename14 testcase:
> 
>   rename14 0 TINFO : Using /mnt/1/ltp-a5w7It6Osi/LTP_renffzJE1 as
>   tmpdir (unknown filesystem)
>   rename14    1  TPASS  :  Test Passed
>   rename14 0 TWARN : tst_tmpdir.c:347: tst_rmdir:
>   rmobj(/mnt/1/ltp-a5w7It6Osi/LTP_renffzJE1) failed:
>   unlink(/mnt/1/ltp-a5w7It6Osi/LTP_renffzJE1/.__smb0021) failed;
>   errno=2: ENOENT
>   <<<execution_status>>>
>   initiation_status="ok"
>   duration=5 termination_type=exited termination_id=4 corefile=no
>   cutime=0 cstime=587
>   <<<test_end>>>
>   INFO: ltp-pan reported some tests FAIL
>   LTP Version: 20250930-14-g9bb94efa3
>          ###############################################################
>               Done executing testcases.
>               LTP Version:  20250930-14-g9bb94efa3
>          ###############################################################
>   -------------------------------------------
>   INFO: runltp script is deprecated, try kirk
>   https://github.com/linux-test-project/kirk
>   -------------------------------------------
>   rm: cannot remove '/mnt/1/ltp-a5w7It6Osi/LTP_renffzJE1': Directory not empty
> 
> Reported-by: Anoop C S <anoopcs@samba.org>
> Fixes: 93ed9a295130 ("smb: client: fix filename matching of deferred files")
> Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
> Cc: David Howells <dhowells@redhat.com>
> Cc: Xiaoli Feng <xifeng@redhat.com>
> Cc: linux-cifs@vger.kernel.org

Reviewed-by: David Howells <dhowells@redhat.com>

Re: [PATCH] smb: client: fix cifs_close_deferred_file_under_dentry()

[Shyam Prasad N]

On Thu, Oct 23, 2025 at 12:58 PM David Howells <dhowells@redhat.com> wrote:
>
> Paulo Alcantara <pc@manguebit.org> wrote:
>
> > The dentry passed in cifs_close_deferred_file_under_dentry() could
> > have been unhashed from its parents hash list and then looked up
> > again, so matching @cfile->dentry with @dentry would no longer work.
> > This would then fail to close the deferred file prior to renaming or
> > unlinking it.
> >
> > Fix this by matching filenames instead of dentry pointers.
> >
> > This problem can be reproduced with LTP rename14 testcase:
> >
> >   rename14 0 TINFO : Using /mnt/1/ltp-a5w7It6Osi/LTP_renffzJE1 as
> >   tmpdir (unknown filesystem)
> >   rename14    1  TPASS  :  Test Passed
> >   rename14 0 TWARN : tst_tmpdir.c:347: tst_rmdir:
> >   rmobj(/mnt/1/ltp-a5w7It6Osi/LTP_renffzJE1) failed:
> >   unlink(/mnt/1/ltp-a5w7It6Osi/LTP_renffzJE1/.__smb0021) failed;
> >   errno=2: ENOENT
> >   <<<execution_status>>>
> >   initiation_status="ok"
> >   duration=5 termination_type=exited termination_id=4 corefile=no
> >   cutime=0 cstime=587
> >   <<<test_end>>>
> >   INFO: ltp-pan reported some tests FAIL
> >   LTP Version: 20250930-14-g9bb94efa3
> >          ###############################################################
> >               Done executing testcases.
> >               LTP Version:  20250930-14-g9bb94efa3
> >          ###############################################################
> >   -------------------------------------------
> >   INFO: runltp script is deprecated, try kirk
> >   https://github.com/linux-test-project/kirk
> >   -------------------------------------------
> >   rm: cannot remove '/mnt/1/ltp-a5w7It6Osi/LTP_renffzJE1': Directory not empty
> >
> > Reported-by: Anoop C S <anoopcs@samba.org>
> > Fixes: 93ed9a295130 ("smb: client: fix filename matching of deferred files")
> > Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
> > Cc: David Howells <dhowells@redhat.com>
> > Cc: Xiaoli Feng <xifeng@redhat.com>
> > Cc: linux-cifs@vger.kernel.org
>
> Reviewed-by: David Howells <dhowells@redhat.com>
>
>

Hi Paulo,

AFAICT this would just be a problem only for __cifs_unlink as it drops
the dentry before the call to cifs_close_deferred_file_under_dentry.
Why not just move the call to cifs_close_deferred_file_under_dentry to
before where the dentry is dropped?

-- 
Regards,
Shyam

Re: [PATCH] smb: client: fix cifs_close_deferred_file_under_dentry()

[Paulo Alcantara]

Shyam Prasad N <nspmangalore@gmail.com> writes:

> AFAICT this would just be a problem only for __cifs_unlink as it drops
> the dentry before the call to cifs_close_deferred_file_under_dentry.
> Why not just move the call to cifs_close_deferred_file_under_dentry to
> before where the dentry is dropped?

I don't know about __cifs_unlink().  The problem was originally found
due to d_drop() being called in cifs_do_rename() to force a new lookup
on the moved dentry.  The "smb: client: get rid of d_drop() in
cifs_do_rename()" removes the d_drop() call, but that is unrelated to
this commit.

This commit is to simply fix the filename lookup without risking of
causing any more regressions.

If we want to go ahead with optimising it by matching the dentry pointer
directly, then let's do in a separate patch and make sure that it will
always be called with a dentry pointer that is expected to be found in
the list of open files.