ksmbd: start file id allocation at 1

ksmbd allocates both the volatile id (per-session file table) and the
persistent id (global file table) with idr_alloc_cyclic() starting at 0.
The first open after the module loads therefore gets volatile id 0 and
persistent id 0, and ksmbd returns an SMB2 FileId of {0, 0} in the create
response.

Clients treat an all-zero FileId as a null handle. smbtorture's
smb2_util_handle_empty() considers {0, 0} empty, so tests that guard the
close with it (e.g. smb2.oplock.statopen1, smb2.lease.statopen*) never
close that first handle. The leaked open keeps the inode's oplock count
non-zero, so a later batch oplock request on the same file is downgraded
to level II and the test fails.

Start the id allocation at 1 (KSMBD_START_FID) so no handle is ever
assigned a {0, 0} FileId, matching the behaviour of other SMB servers.

Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>

diff --git a/fs/smb/server/vfs_cache.c b/fs/smb/server/vfs_cache.c
index 11b51320b96e610dedbb2e0b20121fbfc6ffe3a4..7495166b0262f2ceb789e0f418cbaeff83c8e290 100644 (file)
--- a/fs/smb/server/vfs_cache.c
+++ b/fs/smb/server/vfs_cache.c
@@ -992,7 +992,8 @@ static int __open_id(struct ksmbd_file_table *ft, struct ksmbd_file *fp,
 
        idr_preload(KSMBD_DEFAULT_GFP);
        write_lock(&ft->lock);
-       ret = idr_alloc_cyclic(ft->idr, fp, 0, INT_MAX - 1, GFP_NOWAIT);
+       ret = idr_alloc_cyclic(ft->idr, fp, KSMBD_START_FID, INT_MAX - 1,
+                              GFP_NOWAIT);
        if (ret >= 0) {
                id = ret;
                ret = 0;

diff --git a/fs/smb/server/vfs_cache.h b/fs/smb/server/vfs_cache.h
index f85021c11d6e47cc38683694cfbb98e982852120..287f3e675cd30ab3aca328822eea92c03f4d12cc 100644 (file)
--- a/fs/smb/server/vfs_cache.h
+++ b/fs/smb/server/vfs_cache.h
@@ -23,7 +23,12 @@
 #define        FILE_GENERIC_WRITE      0x120116
 #define        FILE_GENERIC_EXECUTE    0X1200a0
 
-#define KSMBD_START_FID                0
+/*
+ * Start volatile/persistent file id allocation at 1. A file id of 0 yields an
+ * SMB2 FileId of {0, 0}, which clients (e.g. Windows, Samba) treat as a null
+ * handle and never close, leaking the open on the server.
+ */
+#define KSMBD_START_FID                1
 #define KSMBD_NO_FID           (INT_MAX)
 #define SMB2_NO_FID            (0xFFFFFFFFFFFFFFFFULL)