[PATCH v2 0/2] ksmbd: exclude metadata-only opens from sharing checks

[PATCH v2 0/2] ksmbd: exclude metadata-only opens from sharing checks

[Ian Bridges]

ksmbd includes opens that hold no data, execute or delete access in
sharing checks. Such a metadata-only open without sharing can block
later reads, writes and deletes. A metadata-only stream open can also
block deleting the file.

Patch 1 fixes the general sharing check. Patch 2 fixes the stream delete
check added in v7.2. They are separate so patch 1 can be backported
without the stream helper. Patch 2 uses the helper patch 1 adds.

This series was developed with LLM assistance.

---
Changes in v2:
- Use disposition-implied access in both sharing checks. This covers
  Namjae's READ_CONTROL plus FILE_OVERWRITE case and named streams.
- Keep the requested access on the returned handle.
- Leave the attribute-only pairing rule and Time Machine AFP_AfpInfo
  synthesis unchanged.
- Treat FILE_SUPERSEDE as writing because ksmbd implements it by
  truncating, although MS-FSA grants DELETE.

This differs from my reply to v1, which proposed changing only the
truncate path and leaving patch 2 unchanged.

v1: https://lore.kernel.org/linux-cifs/20261104183702.192452-1-icb@fastmail.org/

Ian Bridges (2):
  ksmbd: skip metadata-only opens in sharing checks
  ksmbd: skip metadata-only stream opens in the delete check

 fs/smb/server/smb2pdu.c    | 30 ++++++++++++++++++++++--------
 fs/smb/server/smb_common.c | 34 +++++++++++++++++++++++++++++++++-
 fs/smb/server/smb_common.h |  5 +++++
 fs/smb/server/vfs_cache.c  |  7 ++++++-
 fs/smb/server/vfs_cache.h  |  2 ++
 5 files changed, 68 insertions(+), 10 deletions(-)


base-commit: 957772008502b579bc220aa2167629472f9aa696
-- 
2.47.3

[PATCH v2 1/2] ksmbd: skip metadata-only opens in sharing checks

[Ian Bridges]

A READ_CONTROL open without sharing can block later reads, writes and
deletes even though it holds no data, execute or delete access.

Keep the existing attribute-only exception. For other pairs, require
both effective access masks to hold one of those rights. MS-FSA
2.1.5.1.2 grants FILE_WRITE_DATA when an open overwrites a file or
stream, or creates a named stream on an existing file. Use that granted
access for sharing checks while keeping the requested access on the
returned handle.

The Time Machine AFP_AfpInfo synthesis keeps its requested access.
ksmbd implements FILE_SUPERSEDE by truncating the file, so count it as
a write.

This also prevents an overwrite from truncating a file while another
open that holds data, execute or delete access does not share writing.

Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Cc: stable@vger.kernel.org
Signed-off-by: Ian Bridges <icb@fastmail.org>
---

Notes:
    An authenticated user can trigger this on any share, and a guest can on
    a guest-enabled share. Without a stored DACL, directory search
    permission can be enough to hold a metadata-only open without sharing
    and block the file owner's access.
    
    Tested on ksmbd-for-next 957772008502 with KASAN. Targeted tests and
    KUnit matrices covered access masks, sharing modes, create dispositions
    and streams. smbtorture matched the base. There were no KASAN reports or
    warnings.
    
    A reproducer is available on request.
    
    This fix was developed with LLM assistance.

 fs/smb/server/smb2pdu.c    | 30 ++++++++++++++++++++++--------
 fs/smb/server/smb_common.c | 34 +++++++++++++++++++++++++++++++++-
 fs/smb/server/smb_common.h |  5 +++++
 fs/smb/server/vfs_cache.h  |  2 ++
 4 files changed, 62 insertions(+), 9 deletions(-)

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index b084cca8813c..5bbc33d5a3a8 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -3541,7 +3541,8 @@ static int smb2_set_ea(struct smb2_ea_info *eabuf, unsigned int buf_len,
 
 static noinline int smb2_set_stream_name_xattr(const struct path *path,
 					       struct ksmbd_file *fp,
-					       char *stream_name, int s_type)
+					       char *stream_name, int s_type,
+					       bool *created)
 {
 	const struct mnt_idmap *idmap = mnt_idmap(path->mnt);
 	size_t xattr_stream_size;
@@ -3605,6 +3606,8 @@ static noinline int smb2_set_stream_name_xattr(const struct path *path,
 	rc = ksmbd_vfs_setxattr(idmap, path, xattr_stream_name, NULL, 0, 0, false);
 	if (rc < 0)
 		pr_err("Failed to store XATTR stream name :%d\n", rc);
+	else
+		*created = true;
 	return 0;
 }
 
@@ -4394,6 +4397,7 @@ int smb2_open(struct ksmbd_work *work)
 	char *stream_name = NULL;
 	bool file_present = false, created = false, already_permitted = false;
 	bool app_instance_id_checked = false, app_instance_closed;
+	bool stream_created = false;
 	int share_ret, need_truncate = 0;
 	u64 time, alloc_size = 0;
 	umode_t posix_mode = 0;
@@ -5138,7 +5142,8 @@ int smb2_open(struct ksmbd_work *work)
 		rc = smb2_set_stream_name_xattr(&path,
 						fp,
 						stream_name,
-						s_type);
+						s_type,
+						&stream_created);
 		if (rc)
 			goto err_out;
 		file_info = FILE_CREATED;
@@ -5146,13 +5151,25 @@ int smb2_open(struct ksmbd_work *work)
 
 	fp->attrib_only = !(req->DesiredAccess & ~(FILE_READ_ATTRIBUTES_LE |
 			FILE_WRITE_ATTRIBUTES_LE | FILE_SYNCHRONIZE_LE));
+	need_truncate = !S_ISDIR(file_inode(filp)->i_mode) &&
+			open_flags & O_TRUNC && !fp->attrib_only && !stream_name;
+	/*
+	 * Count the FILE_WRITE_DATA that MS-FSA 2.1.5.1.2 grants an overwrite,
+	 * supersede included, or a new named stream on an existing file.
+	 * An attribute-only open of the file is not truncated, and the
+	 * synthesized AFP_AfpInfo stream is not created by the client.
+	 */
+	fp->implied_write = need_truncate ||
+			    (stream_name &&
+			     (open_flags & O_TRUNC ||
+			      (file_present && stream_created)));
 
 	fp->is_posix_ctxt = posix_ctxt;
 	fp->posix_append = posix_ctxt && !!(open_flags & O_APPEND);
 
 	/* fp should be searchable through ksmbd_inode.m_fp_list
-	 * after daccess, saccess, attrib_only, and stream are
-	 * initialized.
+	 * after daccess, saccess, attrib_only, implied_write, and
+	 * stream are initialized.
 	 */
 	down_write(&fp->f_ci->m_lock);
 	list_add(&fp->node, &fp->f_ci->m_fp_list);
@@ -5173,11 +5190,8 @@ int smb2_open(struct ksmbd_work *work)
 	if (file_present || created)
 		path_put(&path);
 
-	if (!S_ISDIR(file_inode(filp)->i_mode) && open_flags & O_TRUNC &&
-	    !fp->attrib_only && !stream_name) {
+	if (need_truncate)
 		smb_break_all_oplock(work, fp);
-		need_truncate = 1;
-	}
 
 	share_ret = ksmbd_smb_check_shared_mode(fp->filp, fp);
 	if (!test_share_config_flag(work->tcon->share_conf, KSMBD_SHARE_FLAG_OPLOCKS) ||
diff --git a/fs/smb/server/smb_common.c b/fs/smb/server/smb_common.c
index f9fb3741ada3..e7a987af64ba 100644
--- a/fs/smb/server/smb_common.c
+++ b/fs/smb/server/smb_common.c
@@ -695,10 +695,24 @@ static void smb_shared_mode_error(int error, struct ksmbd_file *prev_fp,
 		    prev_fp->saccess, curr_fp->daccess);
 }
 
+/*
+ * Sharing checks use fp->daccess plus the FILE_WRITE_DATA that MS-FSA
+ * grants an open that overwrites data or creates a named stream.
+ * fp->daccess keeps the open's own access.
+ */
+__le32 ksmbd_sharing_daccess(const struct ksmbd_file *fp)
+{
+	if (fp->implied_write)
+		return fp->daccess | FILE_WRITE_DATA_LE;
+	return fp->daccess;
+}
+
 int ksmbd_smb_check_shared_mode(struct file *filp, struct ksmbd_file *curr_fp)
 {
 	int rc = 0;
 	struct ksmbd_file *prev_fp;
+	__le32 curr_access = ksmbd_sharing_daccess(curr_fp);
+	__le32 access;
 
 	/*
 	 * Lookup fp in master fp list, and check desired access and
@@ -719,6 +733,24 @@ int ksmbd_smb_check_shared_mode(struct file *filp, struct ksmbd_file *curr_fp)
 		if (prev_fp->attrib_only != curr_fp->attrib_only)
 			continue;
 
+		/*
+		 * MS-FSA checks sharing per stream, so a named stream open's
+		 * implied write is not a write against the primary stream.
+		 * It still decides whether the pair is checked.
+		 */
+		access = curr_access;
+		if (ksmbd_stream_fd(curr_fp) && !ksmbd_stream_fd(prev_fp))
+			access = curr_fp->daccess;
+
+		/*
+		 * Two opens conflict only if both hold a right in
+		 * FILE_SHARE_CHECKED_ACCESS_LE (MS-FSA 2.1.5.1.2.1 and
+		 * 2.1.5.1.2.2)
+		 */
+		if (!(ksmbd_sharing_daccess(prev_fp) & FILE_SHARE_CHECKED_ACCESS_LE) ||
+		    !(curr_access & FILE_SHARE_CHECKED_ACCESS_LE))
+			continue;
+
 		if (!(prev_fp->saccess & FILE_SHARE_DELETE_LE) &&
 		    curr_fp->daccess & FILE_DELETE_LE) {
 			smb_shared_mode_error(SHARE_DELETE_ERROR,
@@ -745,7 +777,7 @@ int ksmbd_smb_check_shared_mode(struct file *filp, struct ksmbd_file *curr_fp)
 		}
 
 		if (!(prev_fp->saccess & FILE_SHARE_WRITE_LE) &&
-		    curr_fp->daccess & (FILE_WRITE_DATA_LE | FILE_APPEND_DATA_LE)) {
+		    access & (FILE_WRITE_DATA_LE | FILE_APPEND_DATA_LE)) {
 			smb_shared_mode_error(SHARE_WRITE_ERROR,
 					      prev_fp,
 					      curr_fp);
diff --git a/fs/smb/server/smb_common.h b/fs/smb/server/smb_common.h
index a9cc09e31721..d3d19f1a23ea 100644
--- a/fs/smb/server/smb_common.h
+++ b/fs/smb/server/smb_common.h
@@ -69,6 +69,10 @@
 		FILE_EXECUTE | FILE_DELETE_CHILD | \
 		FILE_READ_ATTRIBUTES | FILE_WRITE_ATTRIBUTES)
 
+/* Rights that make an open take part in sharing checks */
+#define FILE_SHARE_CHECKED_ACCESS_LE	(FILE_EXECUTE_LE | FILE_READ_DATA_LE | \
+		FILE_WRITE_DATA_LE | FILE_APPEND_DATA_LE | FILE_DELETE_LE)
+
 #define SMB_COM_NEGOTIATE		0x72 /* See MS-CIFS 2.2.2.1 */
 
 /* See MS-CIFS 2.2.3.1 */
@@ -284,6 +288,7 @@ int ksmbd_extract_shortname(struct ksmbd_conn *conn,
 
 int ksmbd_smb_negotiate_common(struct ksmbd_work *work, unsigned int command);
 
+__le32 ksmbd_sharing_daccess(const struct ksmbd_file *fp);
 int ksmbd_smb_check_shared_mode(struct file *filp, struct ksmbd_file *curr_fp);
 int __ksmbd_override_fsids(struct ksmbd_work *work,
 			   struct ksmbd_share_config *share);
diff --git a/fs/smb/server/vfs_cache.h b/fs/smb/server/vfs_cache.h
index a0239ec178d8..57c81287efe6 100644
--- a/fs/smb/server/vfs_cache.h
+++ b/fs/smb/server/vfs_cache.h
@@ -131,6 +131,8 @@ struct ksmbd_file {
 
 	bool				is_nt_open;
 	bool				attrib_only;
+	/* writes data on open, see ksmbd_sharing_daccess() */
+	bool				implied_write;
 	bool				allocation_size_set;
 
 	char				client_guid[16];
-- 
2.47.3

[PATCH v2 2/2] ksmbd: skip metadata-only stream opens in the delete check

[Ian Bridges]

A metadata-only named-stream open without FILE_SHARE_DELETE blocks a
later DELETE open of the file. It also blocks setting the file delete
pending.

Ignore a stream open unless its effective access includes data, execute
or delete access. Reuse the sharing-access helper so overwriting a
stream, or creating one on an existing file, still counts as
FILE_WRITE_DATA. The Time Machine AFP_AfpInfo synthesis keeps only its
requested access.

Fixes: 19043971c947 ("ksmbd: honor stream delete sharing for base file")
Cc: stable@vger.kernel.org
Signed-off-by: Ian Bridges <icb@fastmail.org>
---

Notes:
    This affects shares that use "vfs objects = streams_xattr".
    
    Tested on ksmbd-for-next 957772008502 with KASAN. Targeted stream tests
    and a KUnit matrix covered metadata-only, create and overwrite cases.
    smbtorture matched the base. There were no KASAN reports or warnings.
    
    A reproducer is available on request.
    
    This fix was developed with LLM assistance.

 fs/smb/server/vfs_cache.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/smb/server/vfs_cache.c b/fs/smb/server/vfs_cache.c
index f9079c529d07..c846023e5ada 100644
--- a/fs/smb/server/vfs_cache.c
+++ b/fs/smb/server/vfs_cache.c
@@ -420,7 +420,12 @@ bool ksmbd_has_stream_without_delete_share(struct ksmbd_file *fp)
 		if (file_inode(fp->filp) != file_inode(prev_fp->filp))
 			continue;
 
-		if (!(prev_fp->saccess & FILE_SHARE_DELETE_LE)) {
+		/*
+		 * A stream open with no right in FILE_SHARE_CHECKED_ACCESS_LE,
+		 * implied writes included, never conflicts.
+		 */
+		if (!(prev_fp->saccess & FILE_SHARE_DELETE_LE) &&
+		    ksmbd_sharing_daccess(prev_fp) & FILE_SHARE_CHECKED_ACCESS_LE) {
 			ret = true;
 			break;
 		}
-- 
2.47.3

Re: [PATCH v2 0/2] ksmbd: exclude metadata-only opens from sharing checks

[Namjae Jeon]

On Fri, Nov 6, 2026 at 12:53 PM Ian Bridges <icb@fastmail.org> wrote:
>
> ksmbd includes opens that hold no data, execute or delete access in
> sharing checks. Such a metadata-only open without sharing can block
> later reads, writes and deletes. A metadata-only stream open can also
> block deleting the file.
>
> Patch 1 fixes the general sharing check. Patch 2 fixes the stream delete
> check added in v7.2. They are separate so patch 1 can be backported
> without the stream helper. Patch 2 uses the helper patch 1 adds.
>
> This series was developed with LLM assistance.
>
> ---
> Changes in v2:
> - Use disposition-implied access in both sharing checks. This covers
>   Namjae's READ_CONTROL plus FILE_OVERWRITE case and named streams.
> - Keep the requested access on the returned handle.
> - Leave the attribute-only pairing rule and Time Machine AFP_AfpInfo
>   synthesis unchanged.
> - Treat FILE_SUPERSEDE as writing because ksmbd implements it by
>   truncating, although MS-FSA grants DELETE.
>
> This differs from my reply to v1, which proposed changing only the
> truncate path and leaving patch 2 unchanged.
>
> v1: https://lore.kernel.org/linux-cifs/20261104183702.192452-1-icb@fastmail.org/
>
> Ian Bridges (2):
>   ksmbd: skip metadata-only opens in sharing checks
>   ksmbd: skip metadata-only stream opens in the delete check
Can you check Sashiko's review comments ?

https://sashiko.dev/#/patchset/20261106035345.728454-1-icb%40fastmail.org

Thanks.

Re: [PATCH v2 0/2] ksmbd: exclude metadata-only opens from sharing checks

[Ian Bridges]

Hi Namjae,

Both issues are older than this series. A test that runs smb2_open()
on the base and on v2 gives the same results for each.

The first is about the earlier open. When an open overwrites or
supersedes the file, or creates a named stream, without asking for
FILE_WRITE_DATA, a later open that does not share write is still
checked only against the earlier open's requested access. So it is not
refused, also in the window before the earlier open truncates. v2
counts the implied write for the new open, which is the case you
reported on v1. For the earlier open, v2 uses the implied write only
to decide whether the pair is checked.

The second is about attribute-only stream opens. Such an open that
creates or overwrites a named stream is still skipped against opens
that are not attribute-only, as before. The series keeps that rule
unchanged. In the stream delete check, the base already refused every
stream open without FILE_SHARE_DELETE, so patch 2 gives this open the
same result as before.

So both look like real bugs, but they are separate from what this
series changes. I'd argue that they should be dealt with in separate
patches. But if you prefer, I can look into addressing them as part of
this patch series. I suspect the fixes won't be small, though.

Thanks,
Ian