[PATCH v3 resend 0/3] smb: fix xfstests generic/035

[PATCH v3 resend 0/3] smb: fix xfstests generic/035

[ChenXiaoSong]

From: ChenXiaoSong <chenxiaosong@kylinos.cn>

Resending patchset due to mail server issue.

v2->v3:
  - Patch #02: iterate over `global_ft.idr`.

v2: https://lore.kernel.org/linux-cifs/20260804123301.285434-1-chenxiaosong@chenxiaosong.com/

ChenXiaoSong (3):
  smb/server: rename to ksmbd_has_nonposix_open_child()
  smb/server: deny overwriting targets with non-POSIX opens
  smb/client: fix nlink of an overwritten open file

 fs/smb/client/inode.c     | 11 ++++++-----
 fs/smb/server/vfs.c       | 15 ++++++++++++++-
 fs/smb/server/vfs_cache.c | 29 ++++++++++++++++++++++++++++-
 fs/smb/server/vfs_cache.h |  3 ++-
 4 files changed, 50 insertions(+), 8 deletions(-)

-- 
2.54.0

[PATCH v3 resend 1/3] smb/server: rename to ksmbd_has_nonposix_open_child()

[ChenXiaoSong]

From: ChenXiaoSong <chenxiaosong@kylinos.cn>

The original function name `ksmbd_has_open_files()` could be confused with
the function name introduced in the next patch, and it does not accurately
describe what this function does.

Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
---
 fs/smb/server/vfs.c       | 2 +-
 fs/smb/server/vfs_cache.c | 2 +-
 fs/smb/server/vfs_cache.h | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
index 28940c7b5f83..16ef8d051b18 100644
--- a/fs/smb/server/vfs.c
+++ b/fs/smb/server/vfs.c
@@ -721,7 +721,7 @@ int ksmbd_vfs_rename(struct ksmbd_work *work, struct ksmbd_file *old_fp,
 	if (err)
 		goto out_drop_write;
 
-	if (d_is_dir(old_child) && ksmbd_has_open_files(old_fp)) {
+	if (d_is_dir(old_child) && ksmbd_has_nonposix_open_child(old_fp)) {
 		err = -EACCES;
 		goto out3;
 	}
diff --git a/fs/smb/server/vfs_cache.c b/fs/smb/server/vfs_cache.c
index eac9886eb6e3..028bc1b0f652 100644
--- a/fs/smb/server/vfs_cache.c
+++ b/fs/smb/server/vfs_cache.c
@@ -1137,7 +1137,7 @@ struct ksmbd_file *ksmbd_lookup_fd_inode(struct dentry *dentry)
 	return NULL;
 }
 
-bool ksmbd_has_open_files(struct ksmbd_file *old_fp)
+bool ksmbd_has_nonposix_open_child(struct ksmbd_file *old_fp)
 {
 	struct dentry *dentry = old_fp->filp->f_path.dentry;
 	struct ksmbd_file *fp;
diff --git a/fs/smb/server/vfs_cache.h b/fs/smb/server/vfs_cache.h
index 127ea4987e3f..9dca617bc429 100644
--- a/fs/smb/server/vfs_cache.h
+++ b/fs/smb/server/vfs_cache.h
@@ -212,7 +212,7 @@ bool ksmbd_has_stream_without_delete_share(struct ksmbd_file *fp);
 int ksmbd_close_fd_app_instance_id(char *app_instance_id);
 struct ksmbd_file *ksmbd_lookup_fd_cguid(char *cguid);
 struct ksmbd_file *ksmbd_lookup_fd_inode(struct dentry *dentry);
-bool ksmbd_has_open_files(struct ksmbd_file *old_fp);
+bool ksmbd_has_nonposix_open_child(struct ksmbd_file *old_fp);
 unsigned int ksmbd_open_durable_fd(struct ksmbd_file *fp);
 struct ksmbd_file *ksmbd_open_fd(struct ksmbd_work *work, struct file *filp);
 void ksmbd_launch_ksmbd_durable_scavenger(void);
-- 
2.54.0

[PATCH v3 resend 2/3] smb/server: deny overwriting targets with non-POSIX opens

[ChenXiaoSong]

From: ChenXiaoSong <chenxiaosong@kylinos.cn>

Reproducer:

  1. server: systemctl start ksmbd
  2. client: mount without `posix` option
     mount -t cifs //${server_ip}/export /mnt
  3. client: touch /mnt/file1 /mnt/file2
  4. client: C program: int fd = open("/mnt/file2", O_RDONLY);
  5. client: C program: rename("/mnt/file1", "/mnt/file2");
  6. client: C program: struct stat stbuf; fstat(fd, &stbuf);
                        stbuf.st_nlink is 1, should be 0

This patch fixes xfstests generic/035 when mounted without `posix` option.

Suggested-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
---
 fs/smb/server/vfs.c       | 13 +++++++++++++
 fs/smb/server/vfs_cache.c | 27 +++++++++++++++++++++++++++
 fs/smb/server/vfs_cache.h |  1 +
 3 files changed, 41 insertions(+)

diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
index 16ef8d051b18..9102ab47a3aa 100644
--- a/fs/smb/server/vfs.c
+++ b/fs/smb/server/vfs.c
@@ -726,6 +726,19 @@ int ksmbd_vfs_rename(struct ksmbd_work *work, struct ksmbd_file *old_fp,
 		goto out3;
 	}
 
+	/*
+	 * See MS-FSA 2.1.5.15.12.
+	 * An overwrite rename must fail with STATUS_ACCESS_DENIED if the
+	 * existing target still has a non-POSIX open.
+	 */
+	if (!(flags & (RENAME_NOREPLACE | RENAME_EXCHANGE)) &&
+	    d_inode(rd.new_dentry) &&
+	    d_inode(rd.new_dentry) != d_inode(old_child) &&
+	    ksmbd_has_other_nonposix_open(rd.new_dentry)) {
+		err = -EACCES;
+		goto out3;
+	}
+
 	err = ksmbd_vfs_check_rename_share(work, old_path);
 	if (err)
 		goto out3;
diff --git a/fs/smb/server/vfs_cache.c b/fs/smb/server/vfs_cache.c
index 028bc1b0f652..90348a409162 100644
--- a/fs/smb/server/vfs_cache.c
+++ b/fs/smb/server/vfs_cache.c
@@ -1137,6 +1137,33 @@ struct ksmbd_file *ksmbd_lookup_fd_inode(struct dentry *dentry)
 	return NULL;
 }
 
+bool ksmbd_has_other_nonposix_open(struct dentry *dentry)
+{
+	struct ksmbd_file *fp;
+	struct inode *inode = d_inode(dentry);
+	unsigned int id;
+	bool ret = false;
+
+	if (!inode)
+		return false;
+
+	read_lock(&global_ft.lock);
+	idr_for_each_entry(global_ft.idr, fp, id) {
+		if (READ_ONCE(fp->f_state) != FP_INITED)
+			continue;
+		if (inode != file_inode(fp->filp))
+			continue;
+		if (fp->is_posix_ctxt)
+			continue;
+
+		ret = true;
+		break;
+	}
+	read_unlock(&global_ft.lock);
+
+	return ret;
+}
+
 bool ksmbd_has_nonposix_open_child(struct ksmbd_file *old_fp)
 {
 	struct dentry *dentry = old_fp->filp->f_path.dentry;
diff --git a/fs/smb/server/vfs_cache.h b/fs/smb/server/vfs_cache.h
index 9dca617bc429..5cac022b540b 100644
--- a/fs/smb/server/vfs_cache.h
+++ b/fs/smb/server/vfs_cache.h
@@ -212,6 +212,7 @@ bool ksmbd_has_stream_without_delete_share(struct ksmbd_file *fp);
 int ksmbd_close_fd_app_instance_id(char *app_instance_id);
 struct ksmbd_file *ksmbd_lookup_fd_cguid(char *cguid);
 struct ksmbd_file *ksmbd_lookup_fd_inode(struct dentry *dentry);
+bool ksmbd_has_other_nonposix_open(struct dentry *dentry);
 bool ksmbd_has_nonposix_open_child(struct ksmbd_file *old_fp);
 unsigned int ksmbd_open_durable_fd(struct ksmbd_file *fp);
 struct ksmbd_file *ksmbd_open_fd(struct ksmbd_work *work, struct file *filp);
-- 
2.54.0

Re: [PATCH v3 resend 2/3] smb/server: deny overwriting targets with non-POSIX opens

[ChenXiaoSong]

The following are my test steps for the hardlink case mentioned by Namjae.

```
--- a/fs/smb/server/vfs_cache.c
+++ b/fs/smb/server/vfs_cache.c
@@ -1156,6 +1156,7 @@ bool ksmbd_has_other_nonposix_open(struct dentry 
*dentry)
                 if (fp->is_posix_ctxt)
                         continue;

+               printk("%s:%d\n", __func__, __LINE__);
                 ret = true;
                 break;
         }
```

1. Add the above debug log.
2. server: systemctl start ksmbd
3. client: mount without `posix` option:
            mount -t cifs //${server_ip}/export /mnt
4. client: touch /mnt/file1 /mnt/file2
5. client: ln /mnt/file2 /mnt/link1; sleep 3
6. client: tail -f /mnt/link1 # open link1
7. tcpdump -i any tcp port 445 -w test-open-hardlink.pcap
8. client: mv /mnt/file1 /mnt/file2
9. server log:
    [102256.516146] ksmbd_has_other_nonposix_open:1159
    [102256.518393] ksmbd_has_other_nonposix_open:1159
    [102256.520443] ksmbd_has_other_nonposix_open:1159
    [102256.522616] ksmbd_has_other_nonposix_open:1159
10. Packets captured by tcpdump:
     SetInfo Response, Error: STATUS_ACCESS_DENIED

On 8/5/26 10:17, Namjae Jeon wrote:
> The new check is limited to the target dentry's
> ksmbd_inode->m_fp_list. Therefore, it does not cover a struct
> ksmbd_file opened through another hardlink dentry, even though
> file_inode(fp->filp) == d_inode(dentry). For example, if file2.link is
> a hardlink to file2 and file2.link is opened with a non-POSIX context,
> an overwrite rename of another file to file2 can miss that open
> handle.
> This appears to be an existing limitation rather than a regression
> introduced by this patch. However, since this patch is intended to
> reject overwrites when the target has a non-POSIX ksmbd_file...


On 8/5/26 13:11, ChenXiaoSong wrote:
> From: ChenXiaoSong <chenxiaosong@kylinos.cn>
> 
> Reproducer:
> 
>    1. server: systemctl start ksmbd
>    2. client: mount without `posix` option
>       mount -t cifs //${server_ip}/export /mnt
>    3. client: touch /mnt/file1 /mnt/file2
>    4. client: C program: int fd = open("/mnt/file2", O_RDONLY);
>    5. client: C program: rename("/mnt/file1", "/mnt/file2");
>    6. client: C program: struct stat stbuf; fstat(fd, &stbuf);
>                          stbuf.st_nlink is 1, should be 0
> 
> This patch fixes xfstests generic/035 when mounted without `posix` option.
> 
> Suggested-by: Namjae Jeon <linkinjeon@kernel.org>
> Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
> ---
>   fs/smb/server/vfs.c       | 13 +++++++++++++
>   fs/smb/server/vfs_cache.c | 27 +++++++++++++++++++++++++++
>   fs/smb/server/vfs_cache.h |  1 +
>   3 files changed, 41 insertions(+)
> 
> diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c
> index 16ef8d051b18..9102ab47a3aa 100644
> --- a/fs/smb/server/vfs.c
> +++ b/fs/smb/server/vfs.c
> @@ -726,6 +726,19 @@ int ksmbd_vfs_rename(struct ksmbd_work *work, struct ksmbd_file *old_fp,
>   		goto out3;
>   	}
>   
> +	/*
> +	 * See MS-FSA 2.1.5.15.12.
> +	 * An overwrite rename must fail with STATUS_ACCESS_DENIED if the
> +	 * existing target still has a non-POSIX open.
> +	 */
> +	if (!(flags & (RENAME_NOREPLACE | RENAME_EXCHANGE)) &&
> +	    d_inode(rd.new_dentry) &&
> +	    d_inode(rd.new_dentry) != d_inode(old_child) &&
> +	    ksmbd_has_other_nonposix_open(rd.new_dentry)) {
> +		err = -EACCES;
> +		goto out3;
> +	}
> +
>   	err = ksmbd_vfs_check_rename_share(work, old_path);
>   	if (err)
>   		goto out3;
> diff --git a/fs/smb/server/vfs_cache.c b/fs/smb/server/vfs_cache.c
> index 028bc1b0f652..90348a409162 100644
> --- a/fs/smb/server/vfs_cache.c
> +++ b/fs/smb/server/vfs_cache.c
> @@ -1137,6 +1137,33 @@ struct ksmbd_file *ksmbd_lookup_fd_inode(struct dentry *dentry)
>   	return NULL;
>   }
>   
> +bool ksmbd_has_other_nonposix_open(struct dentry *dentry)
> +{
> +	struct ksmbd_file *fp;
> +	struct inode *inode = d_inode(dentry);
> +	unsigned int id;
> +	bool ret = false;
> +
> +	if (!inode)
> +		return false;
> +
> +	read_lock(&global_ft.lock);
> +	idr_for_each_entry(global_ft.idr, fp, id) {
> +		if (READ_ONCE(fp->f_state) != FP_INITED)
> +			continue;
> +		if (inode != file_inode(fp->filp))
> +			continue;
> +		if (fp->is_posix_ctxt)
> +			continue;
> +
> +		ret = true;
> +		break;
> +	}
> +	read_unlock(&global_ft.lock);
> +
> +	return ret;
> +}
> +
>   bool ksmbd_has_nonposix_open_child(struct ksmbd_file *old_fp)
>   {
>   	struct dentry *dentry = old_fp->filp->f_path.dentry;
> diff --git a/fs/smb/server/vfs_cache.h b/fs/smb/server/vfs_cache.h
> index 9dca617bc429..5cac022b540b 100644
> --- a/fs/smb/server/vfs_cache.h
> +++ b/fs/smb/server/vfs_cache.h
> @@ -212,6 +212,7 @@ bool ksmbd_has_stream_without_delete_share(struct ksmbd_file *fp);
>   int ksmbd_close_fd_app_instance_id(char *app_instance_id);
>   struct ksmbd_file *ksmbd_lookup_fd_cguid(char *cguid);
>   struct ksmbd_file *ksmbd_lookup_fd_inode(struct dentry *dentry);
> +bool ksmbd_has_other_nonposix_open(struct dentry *dentry);
>   bool ksmbd_has_nonposix_open_child(struct ksmbd_file *old_fp);
>   unsigned int ksmbd_open_durable_fd(struct ksmbd_file *fp);
>   struct ksmbd_file *ksmbd_open_fd(struct ksmbd_work *work, struct file *filp);

-- 
ChenXiaoSong <chenxiaosong@chenxiaosong.com>
Chinese Homepage: https://chenxiaosong.com
English Homepage: https://chenxiaosong.com/en

[PATCH v3 resend 3/3] smb/client: fix nlink of an overwritten open file

[ChenXiaoSong]

From: ChenXiaoSong <chenxiaosong@kylinos.cn>

Reproducer:

  1. server: systemctl start ksmbd
  2. client: mount with `posix` option
     mount -t cifs -o posix //${server_ip}/export /mnt
  3. client: touch /mnt/file1 /mnt/file2
  4. client: C program: int fd = open("/mnt/file2", O_RDONLY);
  5. client: C program: rename("/mnt/file1", "/mnt/file2");
  6. client: C program: struct stat stbuf; fstat(fd, &stbuf);
                        stbuf.st_nlink is 1, should be 0

This patch fixes xfstests generic/035 when mounted with `posix` option.

Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
---
 fs/smb/client/inode.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
index 0afff761aab9..0b6273ce01a8 100644
--- a/fs/smb/client/inode.c
+++ b/fs/smb/client/inode.c
@@ -2648,11 +2648,8 @@ cifs_rename2(struct mnt_idmap *idmap, struct inode *source_dir,
 	if (d_really_is_positive(target_dentry)) {
 		if (!rc) {
 			struct inode *inode = d_inode(target_dentry);
-			/*
-			 * Samba and ksmbd servers allow renaming a target
-			 * directory that is open, so make sure to update
-			 * ->i_nlink and then mark it as delete pending.
-			 */
+
+			/* Update the target link count after rename. */
 			if (S_ISDIR(inode->i_mode)) {
 				drop_cached_dir_by_name(xid, tcon, to_name, cifs_sb);
 				spin_lock(&inode->i_lock);
@@ -2663,6 +2660,10 @@ cifs_rename2(struct mnt_idmap *idmap, struct inode *source_dir,
 				CIFS_I(inode)->time = 0; /* force reval */
 				inode_set_ctime_current(inode);
 				inode_set_mtime_to_ts(inode, inode_set_ctime_current(inode));
+			} else {
+				cifs_mark_open_handles_for_deleted_file(inode, to_name);
+				cifs_drop_nlink(inode);
+				inode_set_ctime_current(inode);
 			}
 		} else if (rc == -EACCES || rc == -EEXIST) {
 			/*
-- 
2.54.0

Re: [PATCH v3 resend 3/3] smb/client: fix nlink of an overwritten open file

[hehuiwen]

The AI review of the overwrite rename patch raised two possible races.

1. A handle opened through a surviving hard link may be marked deleted:

   target_fh -- target --+
                         +--> inode X (nlink = 2)
   alias_fh  -- alias  --+

   CPU A                                   CPU B
   -----                                   -----
   rename(source, target)
     server nlink: 2 -> 1

                                           statx(alias,
                                                 AT_STATX_FORCE_SYNC)
                                             cached i_nlink: 2 -> 1

   cifs_mark_open_handles_for_deleted_file()
     sees i_nlink == 1
     marks every handle:
       target_fh: deleted                   correct
       alias_fh:  deleted                   wrong

2. A concurrently refreshed link count may be decremented again:

   CPU A                                   CPU B
   -----                                   -----
   rename(source, target)
     server nlink: 2 -> 1

                                           statx(alias,
                                                 AT_STATX_FORCE_SYNC)
                                             cached i_nlink: 2 -> 1

   cifs_drop_nlink()
     actual:   cached i_nlink: 1 -> 0       wrong
     expected: cached i_nlink remains 1

I tried addressing these problems by taking an i_nlink snapshot before
the unlink/rename request, using it to decide which handles to mark,
and decrementing the cached link count only if it still matched the
snapshot.

Unfortunately, an nlink snapshot does not show whether revalidation
happened before or after unlink/rename. Therefore, it cannot fully
address these races or reject stale attribute responses that arrive late.

NFS addresses these issues with per-dentry silly rename, attribute
generation counters, and cache invalidation. A similar CIFS solution
may be challenging, as it must coordinate namespace operations,
open-handle state, and asynchronous attribute updates.

Do you have any ideas?

Thanks,
Huiwen



在 2026/8/5 13:11, ChenXiaoSong 写道:
> From: ChenXiaoSong <chenxiaosong@kylinos.cn>
> 
> Reproducer:
> 
>    1. server: systemctl start ksmbd
>    2. client: mount with `posix` option
>       mount -t cifs -o posix //${server_ip}/export /mnt
>    3. client: touch /mnt/file1 /mnt/file2
>    4. client: C program: int fd = open("/mnt/file2", O_RDONLY);
>    5. client: C program: rename("/mnt/file1", "/mnt/file2");
>    6. client: C program: struct stat stbuf; fstat(fd, &stbuf);
>                          stbuf.st_nlink is 1, should be 0
> 
> This patch fixes xfstests generic/035 when mounted with `posix` option.
> 
> Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
> ---
>   fs/smb/client/inode.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c
> index 0afff761aab9..0b6273ce01a8 100644
> --- a/fs/smb/client/inode.c
> +++ b/fs/smb/client/inode.c
> @@ -2648,11 +2648,8 @@ cifs_rename2(struct mnt_idmap *idmap, struct inode *source_dir,
>   	if (d_really_is_positive(target_dentry)) {
>   		if (!rc) {
>   			struct inode *inode = d_inode(target_dentry);
> -			/*
> -			 * Samba and ksmbd servers allow renaming a target
> -			 * directory that is open, so make sure to update
> -			 * ->i_nlink and then mark it as delete pending.
> -			 */
> +
> +			/* Update the target link count after rename. */
>   			if (S_ISDIR(inode->i_mode)) {
>   				drop_cached_dir_by_name(xid, tcon, to_name, cifs_sb);
>   				spin_lock(&inode->i_lock);
> @@ -2663,6 +2660,10 @@ cifs_rename2(struct mnt_idmap *idmap, struct inode *source_dir,
>   				CIFS_I(inode)->time = 0; /* force reval */
>   				inode_set_ctime_current(inode);
>   				inode_set_mtime_to_ts(inode, inode_set_ctime_current(inode));
> +			} else {
> +				cifs_mark_open_handles_for_deleted_file(inode, to_name);
> +				cifs_drop_nlink(inode);
> +				inode_set_ctime_current(inode);
>   			}
>   		} else if (rc == -EACCES || rc == -EEXIST) {
>   			/*

Re: [PATCH v3 resend 0/3] smb: fix xfstests generic/035

[Namjae Jeon]

On Wed, Aug 5, 2026 at 2:12 PM ChenXiaoSong
<chenxiaosong@chenxiaosong.com> wrote:
>
> From: ChenXiaoSong <chenxiaosong@kylinos.cn>
>
> Resending patchset due to mail server issue.
>
> v2->v3:
>   - Patch #02: iterate over `global_ft.idr`.
>
> v2: https://lore.kernel.org/linux-cifs/20260804123301.285434-1-chenxiaosong@chenxiaosong.com/
>
> ChenXiaoSong (3):
>   smb/server: rename to ksmbd_has_nonposix_open_child()
>   smb/server: deny overwriting targets with non-POSIX opens
Applied ksmbd's patches to #ksmbd-for-next-next.
Thanks!