* [PATCH] smb: client: fix cifs_close_deferred_file_under_dentry()
@ 2025-10-22 23:43 Paulo Alcantara
2025-10-23 7:21 ` David Howells
0 siblings, 1 reply; 4+ messages in thread
From: Paulo Alcantara @ 2025-10-22 23:43 UTC (permalink / raw)
To: smfrench
Cc: Anoop C S, Paulo Alcantara (Red Hat), David Howells, Xiaoli Feng,
linux-cifs
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] smb: client: fix cifs_close_deferred_file_under_dentry()
2025-10-22 23:43 [PATCH] smb: client: fix cifs_close_deferred_file_under_dentry() Paulo Alcantara
@ 2025-10-23 7:21 ` David Howells
2025-10-24 11:46 ` Shyam Prasad N
0 siblings, 1 reply; 4+ messages in thread
From: David Howells @ 2025-10-23 7:21 UTC (permalink / raw)
To: Paulo Alcantara; +Cc: dhowells, smfrench, Anoop C S, Xiaoli Feng, linux-cifs
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>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] smb: client: fix cifs_close_deferred_file_under_dentry()
2025-10-23 7:21 ` David Howells
@ 2025-10-24 11:46 ` Shyam Prasad N
2025-10-24 15:04 ` Paulo Alcantara
0 siblings, 1 reply; 4+ messages in thread
From: Shyam Prasad N @ 2025-10-24 11:46 UTC (permalink / raw)
To: David Howells
Cc: Paulo Alcantara, smfrench, Anoop C S, Xiaoli Feng, linux-cifs
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] smb: client: fix cifs_close_deferred_file_under_dentry()
2025-10-24 11:46 ` Shyam Prasad N
@ 2025-10-24 15:04 ` Paulo Alcantara
0 siblings, 0 replies; 4+ messages in thread
From: Paulo Alcantara @ 2025-10-24 15:04 UTC (permalink / raw)
To: Shyam Prasad N, David Howells
Cc: smfrench, Anoop C S, Xiaoli Feng, linux-cifs
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-10-24 15:04 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-22 23:43 [PATCH] smb: client: fix cifs_close_deferred_file_under_dentry() Paulo Alcantara
2025-10-23 7:21 ` David Howells
2025-10-24 11:46 ` Shyam Prasad N
2025-10-24 15:04 ` Paulo Alcantara
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox