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

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

[Ian Bridges]

A metadata-only open holds none of FILE_EXECUTE, FILE_READ_DATA,
FILE_WRITE_DATA, FILE_APPEND_DATA and DELETE. An open for READ_CONTROL
is one example. ksmbd still counts such opens in sharing checks. Held
without sharing, one blocks later reads, writes and deletes of the file.
Held on a stream without FILE_SHARE_DELETE, one blocks deleting the
file.

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

This series was developed with LLM assistance.

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

 fs/smb/server/smb_common.c | 9 +++++++++
 fs/smb/server/smb_common.h | 4 ++++
 fs/smb/server/vfs_cache.c  | 7 ++++++-
 3 files changed, 19 insertions(+), 1 deletion(-)


base-commit: e49296d51ccbf2ad0ed0484db1382f1d7217bd6e
-- 
2.47.3

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

[Ian Bridges]

A READ_CONTROL open without FILE_SHARE_DELETE makes a later DELETE open
of the file fail with STATUS_SHARING_VIOLATION.

ksmbd_smb_check_shared_mode() compares the sharing modes of two opens
even when one holds none of FILE_EXECUTE, FILE_READ_DATA,
FILE_WRITE_DATA, FILE_APPEND_DATA and DELETE. It skips a pair only when
exactly one of them asks for nothing but attribute rights.

Skip a pair of opens unless both hold one of those rights.

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 it on any share, and a guest can on a
    share with "guest ok = yes". On a share without stored DACLs an open for
    FILE_READ_ATTRIBUTES and READ_CONTROL needs only search permission on
    the directory. A user with that permission can hold such an open without
    sharing and block the owner's reads, writes and deletes of a file in it.
    
    Tested on ksmbd-for-next e49296d51ccb with KASAN, with no reports or
    warnings. In targeted sharing tests and a KUnit matrix every changed
    result removes a sharing violation that MS-FSA does not make.
    smbtorture gives the same results as on the base.
    
    A reproducer is available on request.
    
    This fix was developed with LLM assistance.

 fs/smb/server/smb_common.c | 9 +++++++++
 fs/smb/server/smb_common.h | 4 ++++
 2 files changed, 13 insertions(+)

diff --git a/fs/smb/server/smb_common.c b/fs/smb/server/smb_common.c
index f9fb3741ada3..920e94017bf6 100644
--- a/fs/smb/server/smb_common.c
+++ b/fs/smb/server/smb_common.c
@@ -719,6 +719,15 @@ int ksmbd_smb_check_shared_mode(struct file *filp, struct ksmbd_file *curr_fp)
 		if (prev_fp->attrib_only != curr_fp->attrib_only)
 			continue;
 
+		/*
+		 * 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 (!(prev_fp->daccess & FILE_SHARE_CHECKED_ACCESS_LE) ||
+		    !(curr_fp->daccess & 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,
diff --git a/fs/smb/server/smb_common.h b/fs/smb/server/smb_common.h
index a9cc09e31721..4d404d1592c6 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 */
-- 
2.47.3

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

[Ian Bridges]

A READ_CONTROL open of a named stream without FILE_SHARE_DELETE makes a
DELETE open of the file fail with STATUS_SHARING_VIOLATION. Setting the
file delete pending fails the same way.

ksmbd_has_stream_without_delete_share() counts every stream open without
FILE_SHARE_DELETE, whatever access it holds.

Count a stream open only when it holds FILE_EXECUTE, FILE_READ_DATA,
FILE_WRITE_DATA, FILE_APPEND_DATA or DELETE.

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

Notes:
    Any share with "vfs objects = streams_xattr" is affected. A READ_CONTROL
    stream open without FILE_SHARE_DELETE blocks every DELETE open of the
    file and setting it delete pending.
    
    Tested on ksmbd-for-next e49296d51ccb with KASAN, with no reports or
    warnings. In targeted stream tests and a KUnit matrix every changed
    result removes a sharing violation that MS-FSA does not make.
    smbtorture gives the same results as on the base.
    
    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..6675c6652c13 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
+		 * never conflicts
+		 */
+		if (!(prev_fp->saccess & FILE_SHARE_DELETE_LE) &&
+		    prev_fp->daccess & FILE_SHARE_CHECKED_ACCESS_LE) {
 			ret = true;
 			break;
 		}
-- 
2.47.3