* [PATCH v2 1/2] ksmbd: skip metadata-only opens in sharing checks
2026-11-06 3:53 [PATCH v2 0/2] ksmbd: exclude metadata-only opens from sharing checks Ian Bridges
@ 2026-11-06 3:53 ` Ian Bridges
2026-11-06 3:53 ` [PATCH v2 2/2] ksmbd: skip metadata-only stream opens in the delete check Ian Bridges
1 sibling, 0 replies; 3+ messages in thread
From: Ian Bridges @ 2026-11-06 3:53 UTC (permalink / raw)
To: linkinjeon
Cc: senozhatsky, tom, chenxiaosong, linux-cifs, linux-kernel,
Ian Bridges, stable
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
^ permalink raw reply [flat|nested] 3+ messages in thread