| Age | Commit message (Collapse) | Author |
|---|
|
POSIX append-only opens do not set O_APPEND, and the SMB append sentinel
is rejected as a negative write offset.
Track append-only POSIX opens, set O_APPEND and require the U64_MAX
offset sentinel for their writes. Accept FILE_WRITE_DATA or
FILE_APPEND_DATA for WRITE requests, preserving ordinary SMB write
permissions. Use the POSIX append flag and offset check to prevent
POSIX append-only opens from overwriting existing data.
For synchronous writes, derive the fsync range from the completed write
position so append writes flush the range actually written.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
Arm the oplock break acknowledgment timer once the common notification
path accepts a classic break for transmission. Preserve its shared retry
deadline when another channel is tried. Store a deadline per open and scan
the global open table when the earliest deadline expires. Drop expired
breaks to NONE and restart the timer while other breaks remain.
Serialize timer expiry with client acknowledgments and close using the
oplock state lock. Wake blocked requests when a break expires or sending
the notification fails, and invalidate durable reconnect for the open.
Use wake_up_all() for acknowledgment, timeout, close and send failure.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
SMB2 lock ranges use unsigned 64-bit offsets, but file_lock stores signed
loff_t values. Track valid ranges above OFFSET_MAX in ksmbd's inode-based
lock index and arbitrate conflicts there. Keep these offsets out of
file_lock, and handle blocking requests, unlocks, rollback, close cleanup
and cancellation in the SMB layer.
Reserve large ranges under the index spinlock until the full request
commits. Keep lookup and ownership independent of the connection so
channel removal and hard-link opens use the same conflict checks.
Finish large-range unlocks in the SMB layer after unlocking any VFS
portion. Wake large-range lock waiters when normal locks are removed
by unlock or close.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
A completed DurableHandleReqV2 CREATE can be replayed through another
channel while its original channel is exiting. The existing replay path
returns the open but leaves its connection and oplock references pointing
at the old channel. Its open count also stays on that channel, and a
later CLOSE through the surviving channel decrements the wrong counter.
After checking the owner, lease key and persistent flag, verify that an
active open is still published in the requesting session's file table.
Do this even when no oplock was granted, and before processing a consumed
replay. Do not read a borrowed opinfo session pointer for this check.
Keep a healthy original binding. If the original channel is exiting or
releasing, move the open and its own oplock references to the requesting
channel. Serialize this with CLOSE using the session file-table lock,
and with detach, reconnect and notification snapshots using m_lock.
The earlier lock index and lease-table changes make their lifetimes
independent of this connection change. No old-channel r_count wait is
needed, including when an unrelated request remains active there.
Account final close against fp->conn in one place, balance durable detach
and reconnect, and transfer the count with a replay binding change. A
final global lookup put may run after CLOSE has removed the volatile id;
unlink the open from the inode list on that final close as well.
A live completed CREATE replay may follow a break that lowered the
durable open's batch oplock. Apply the batch eligibility check only to
detached reconnects, while retaining the owner, lease key and persistent
flag checks for active replays.
Fixes: bb8bf7eb13b5 ("ksmbd: add SMB3 request replay support")
Reported-by: Pan Guihua <pan.guihua@datatom.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
SMB locks belong to an open, but the lock conflict and UNLOCK scan only
visits connections in conn_list. When a channel is removed, its locks
remain attached to live opens and become invisible to another channel.
An exact UNLOCK returns STATUS_RANGE_NOT_LOCKED, and zero-byte lock
conflicts are no longer detected. A request finishing on the old channel
after it is removed can publish another invisible lock.
Index granted SMB locks by VFS inode, with the open's lock_list retaining
ownership. Protect lookup, publication and removal with one spinlock.
Use the inode rather than the dentry-based ksmbd_inode so hard-link opens
still participate in the same conflict checks.
Keep these entries through durable detach and reconnect. Publish a batch
only after the existing LOCK handler commits it, and remove every entry
before final close drops the file reference. Drop the per-lock connection
references and connection-local lists; channel changes no longer require
moving lock entries or waiting for unrelated requests to finish.
Prefer an exclusive lock when several locks on an open match an UNLOCK.
This preserves stacked zero-byte lock behavior despite the inode hash
inserting newer locks before older entries.
Fixes: d63528eb0d43 ("ksmbd: free ksmbd_lock when file is closed")
Reported-by: Pan Guihua <pan.guihua@datatom.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
AppInstanceId lookup currently selects the first open with the matching
identifier. CREATE for an unrelated share or path can therefore
force-close another session's open before authorization of the new
target. Matching only the base path also conflates alternate data streams
with the unnamed stream.
MS-SMB2 requires the old open to match the share and path, belong to a
different ClientGuid, and be closed only when the requester has
GENERIC_READ access. Search all opens with the ID and match share,
resolved path, stream, and the current connection's client GUID. Retain a
share configuration reference in ksmbd_file and reuse its name instead of
allocating a name for every open. This keeps matching safe during tree
connection teardown and preserves name-based matching across configuration
updates.
Refresh the share reference on durable reconnect and restore the old
reference if reopening fails. Serialize connection and share comparisons
with durable disconnect/reconnect, and recheck the target before closing
it. Keep the original ClientGuid for durable replay and lease checks.
Check AppInstanceVersion before querying access, even when GENERIC_READ
is absent, and before disconnected delete-on-close cleanup. Rejected
CREATE must leave other durable opens untouched. Calculate maximal access
on the matched open's referenced path so a rename cannot substitute another
file's authorization. Require read access with the effective VFS
credentials as well as the NT DACL before takeover. Access denial only
skips takeover and leaves CREATE to its normal access checks. Restore
filesystem credentials before returning a takeover error.
Handle takeover before checking FILE_CREATE collisions, and re-resolve
the path after closing the old open. This lets delete-on-close remove the
old file before a new one is created. Include SYNCHRONIZE in the baseline
maximal-access mask so read-only files report GENERIC_READ correctly.
Fixes: 16c30649709d ("ksmbd: handle durable v2 app instance id")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Sung Byeongchan <tjdqudcks0424@naver.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
The previous design registered a synthetic work struct (in_work)
directly on conn->async_requests and deferred the response to a
workqueue -- a bespoke async/cancel implementation duplicating what
setup_async_work(), release_async_work(), and smb2_send_interim_resp()
already provide for smb2_lock()'s pending byte-range lock.
Replace it with that same pattern: setup_async_work() on the calling
work itself, registered on fp->blocked_works, woken by cancel or by
the handle closing via the existing set_close_state_blocked_works().
This removes the synthetic work struct, the notify_pendings list and
its close-time drain, and the deferred workqueue send, leaving
smb2_notify() sharing the same async/cancel machinery as smb2_lock()
instead of its own separate copy.
The worker now blocks on ksmbd_wq for as long as the watch stays
open, instead of returning immediately. This also makes the skeleton
ready for a future event-delivery implementation on the same
blocking wait.
Suggested-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Gael Blivet <gael.blivet@gmail.com>
Assisted-by: Claude:claude-sonnet-5
Tested-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
SMB3 multichannel allows requests for one session to run on multiple
connections. Wait for all channels bound to a session before freeing
shared session objects.
A deferred byte-range lock remains counted as a running request and only
wakes when its file closes. Wake blocked locks during the drain without
unpublishing or modifying their file objects. Synchronous CANCEL requests
must invoke their cancellation callback to wake pending operations, while
CHANGE_NOTIFY completion remains specific to the asynchronous path.
Serialize session teardown with channel registration and previous-session
cleanup, and use atomic work-state transitions so LOGOFF, CANCEL, and
connection teardown invoke cancellation callbacks only once.
Fixes: 76e98a158b20 ("ksmbd: fix race condition between destroy_previous_session() and smb2 operations()")
Reported-by: Cheryl Babcock <cheryl@renat.io>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
BVT_AppInstanceVersion_SMB311_GreaterVersion,
BVT_AppInstanceVersion_SMB311_SameVersion,
BVT_AppInstanceVersion_SMB311_LowerAppInstanceVersionHigh, and
BVT_AppInstanceVersion_SMB311_LowerAppInstanceVersionLow exercise
ordered opens using the same AppInstanceId. ksmbd tracked the
AppInstanceId, but did not parse the version context or enforce the
version ordering, so versioned opens returned incorrect sharing
violations.
Parse and retain the 24-byte AppInstanceVersion context with each open.
Reject a version that is lower than or equal to the active version with
STATUS_FILE_FORCED_CLOSED, reject an unversioned open against a versioned
handle, and close the previous handle for a newer takeover. Do not apply
the takeover check to durable reconnect or replay requests.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
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>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
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>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
Pass the source ksmbd_file to the rename helpers and use the per-handle
POSIX create-context state when deciding whether open children block a
directory rename.
work->tcon->posix_extensions only records whether POSIX extensions were
negotiated on the connection. It does not indicate that the handles were
opened with POSIX create contexts.
Reproducer:
1. server: systemctl start ksmbd
2. client: mount -t cifs //${server_ip}/export /mnt
# without posix option
3. client: mkdir /mnt/dir1/; touch /mnt/dir1/file
4. client: tail -f /mnt/dir1/file # open file
5. client: mv /mnt/dir1 /mnt/dir2
Without this fix, the rename can succeed when it should fail with
"Permission denied".
Fixes: c841bd3d8dec ("ksmbd: deny renaming directory with open children")
Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
Ordinary opens initialize their allocation size from stat.blocks.
Buffered writes can leave delayed allocation pending, so separate handles
can cache different block counts for the same file.
This makes generic/568 fail when a zero write used for fallocate
emulation is followed by an overwrite of the same range. The first query
can report the pre-writeback block count, while the second query reports
the block count after delayed allocation is completed.
Complete writeback and refresh the cached block count before returning
allocation information for ordinary opens. Track client-specified
allocation sizes separately so CREATE allocation contexts and
FILE_ALLOCATION_INFORMATION continue to return the requested value.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
The server proc entry does not expose configured limits or enough outcome
data to distinguish protocol errors from transport stalls.
Report the server state, listener and signing configuration, connection
limits, timeout values, current client and open-file totals, IPC activity,
and durable scavenger state. Classify processed SMB2 response statuses by
NTSTATUS severity and provide counters for common error groups while
retaining the per-command counters.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
SMB3 clients can replay selected requests after a channel disconnect by
setting SMB2_FLAGS_REPLAY_OPERATION. The command sequence window rejects
duplicate MessageIds on one connection, but it does not validate requests
resent on another channel with a new MessageId.
Add the state and validation required to replay durable CREATE and
file-handle operations:
- track each open ChannelSequence, outstanding request counts, and lock
sequence entries.
- retain a request-owned open reference until the common response path
completes ChannelSequence accounting.
- replay DurableHandleReqV2 CREATE requests by CreateGuid, validating the
durable state, SecurityContext, session, lease key, and persistent flag.
- publish CreateGuid and SecurityContext before an oplock or lease break
can defer CREATE, rejecting replays of that pending CREATE with
STATUS_FILE_NOT_AVAILABLE.
- retain the original CREATE action and replay completed CreateGuid
requests, including requests that did not receive a durable-handle grant,
without modifying the existing open.
- make replayed oplock and lease break acknowledgements idempotent. And
- preserve SMB2_FLAGS_REPLAY_OPERATION in responses.
Return STATUS_FILE_NOT_AVAILABLE when ChannelSequence validation rejects a
replayed WRITE, IOCTL, or SET_INFO request.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
smb2_notify() currently returns STATUS_NOT_IMPLEMENTED synchronously
for every CHANGE_NOTIFY request. Genuine SMB2 servers never complete a
CHANGE_NOTIFY spontaneously -- it's satisfied only by a real directory
change or with STATUS_NOTIFY_CLEANUP when the watched handle is
closed. macOS smbfs.kext depends on this deferred-completion contract:
receiving STATUS_NOT_IMPLEMENTED instead makes it hard-freeze on
unmount, since it never sees the cleanup it's waiting for.
Add a notify_pendings list on struct ksmbd_file (protected by the
existing f_lock) and a notify_entry list_head on struct ksmbd_work to
link onto it. smb2_notify() now replies STATUS_PENDING immediately and
queues a deferred STATUS_NOTIFY_CLEANUP response on the watched
handle; __ksmbd_close_fd() drains and sends any pending notifications
when the handle is actually closed. The drain splices the list out
under fp->f_lock first, then processes the detached copy without the
lock -- smb2_notify() on another connection can be adding to the same
list at the same time a close happens on this one, and
ksmbd_conn_write() can sleep (it takes the connection's write mutex),
so it must not be called while the spinlock is held.
Also handle the FileId=FFFF...FFFF share-root sentinel that macOS
backupd sends to watch for changes without holding an open handle --
without an immediate STATUS_PENDING/STATUS_NOTIFY_CLEANUP reply here,
backupd aborts Time Machine setup with STATUS_FILE_CLOSED.
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Gael Blivet <gael.blivet@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
ksmbd_fd_set_delete_pending()/ksmbd_fd_clear_delete_pending()
to keep a stream's FileDispositionInformation from marking the whole
file for deletion, but used the inode-wide S_DEL_ON_CLS_STREAM flag to
do it -- the exact same problem class the commit was fixing, one level
up.
S_DEL_ON_CLS_STREAM lives on the shared ksmbd_inode, not on any
specific stream handle. If a file has multiple stream handles open and
one gets marked delete-pending via FileDispositionInformation, the
flag can't record *which* stream should be deleted: whichever stream
handle happens to close first (not necessarily the one that was
actually marked) sees S_DEL_ON_CLS_STREAM set and has its xattr
removed. Two clients (or two handles from the same client) touching
different streams on the same file can end up deleting the wrong one.
ksmbd_inode_pending_delete() has the same issue: it only checks
S_DEL_PENDING, which is never set for a stream handle, so a client
querying FileStandardInformation.DeletePending on a stream marked via
this path would incorrectly see 0.
Track this per-handle instead (stream_del_pending on struct
ksmbd_file), matching the file itself rather than the shared inode.
ksmbd_fd_set_delete_on_close() (the CREATE-time FILE_DELETE_ON_CLOSE
option, a separate call path from FileDispositionInformation) still
uses the inode-wide flag; __ksmbd_inode_close() now checks both, since
either one should trigger removing the stream's xattr on close.
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Gael Blivet <gael.blivet@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
set_file_disposition_info() calls ksmbd_set_inode_pending_delete() /
ksmbd_clear_inode_pending_delete() unconditionally, which always sets
S_DEL_PENDING on the whole inode (ci->m_flags), regardless of whether
the handle being closed is a regular file or an alternate data stream.
Requesting delete-pending on a single stream handle (e.g. deleting
just an alternate data stream some clients keep alongside a file)
would therefore incorrectly schedule deletion of the entire file's
data, not just the stream.
Add ksmbd_fd_set_delete_pending()/ksmbd_fd_clear_delete_pending(),
following the same stream-vs-whole-file routing pattern already used
by ksmbd_fd_set_delete_on_close() for the CREATE-time DeleteOnClose
option, and switch set_file_disposition_info() to use them.
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Gael Blivet <gael.blivet@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
Windows reports automatic write-time updates with a resolution of roughly
15 milliseconds. If a file is written and closed within that interval, a
close response requesting full information can report the write time from
the open rather than the filesystem's finer-grained mtime update.
ksmbd currently converts the filesystem mtime directly in SMB2 CLOSE, so
even a sub-millisecond write is visible to the client. This makes
smb2.timestamp_resolution.resolution1 fail because the immediate write
changes LastWriteTime.
Save the write time returned by SMB2 CREATE in the file handle. When CLOSE
requests post-query attributes, coalesce a positive mtime change smaller
than 15 milliseconds to that saved value. Larger changes remain visible,
including the test's write after a 20 millisecond delay.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
|
|
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>
|
|
smb2.create.blob sends an SMB2_CREATE_ALLOCATION_SIZE create context with
a 1MiB allocation size and expects the create response AllocationSize field
to match the requested size. smb2.create.open additionally compares the
AllocationSize returned in the CREATE response with the AllocationSize
returned by FILE_ALL_INFORMATION on the same handle.
ksmbd applies the allocation with fallocate(), but then fills both the
create response and handle-based information from stat.blocks << 9. On
filesystems such as ext4 this can include filesystem allocation rounding
and metadata effects, causing a response larger than the SMB2 allocation
size context and a disagreement between the two queries.
Remember the requested allocation size while processing the create context,
store the reported allocation size in struct ksmbd_file, and use it for
both the create response and handle-based allocation size responses. Update
the stored value when FILE_ALLOCATION_INFORMATION changes it, and fall back
to stat.blocks << 9 when no allocation size context was provided.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
|
|
smb2.streams.delete opens an alternate data stream without
FILE_SHARE_DELETE and then tries to delete the base file. Windows rejects
the base-file delete with STATUS_SHARING_VIOLATION while the stream handle
is open. ksmbd tracks stream opens on the same ksmbd_inode as the base
file, but the delete-on-close path only checked delete access on the base
handle before marking the inode delete-pending. As a result, deleting
the base file succeeded even though an open stream handle denied delete
sharing. Add a helper to detect open stream handles on the same inode that
do not allow FILE_SHARE_DELETE, and reject base-file delete pending and
DELETE opens with a sharing violation in that case.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
|
|
Windows denies renaming a directory while a file below that directory is
still open. smb2.rename.rename_dir_openfile checks this by keeping a file
handle open under the directory and then attempting to rename the directory
handle. ksmbd did not check open children before calling vfs_rename(), so
the rename incorrectly succeeded.
For non-POSIX clients, scan the global open file table for active handles
whose dentries are below the directory being renamed. If any child is
open, fail the rename with -EACCES so the client receives
STATUS_ACCESS_DENIED.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
|
|
inode ctime is updated when a file is renamed. ksmbd returned that
ctime directly as SMB2 ChangeTime for handle-based query information.
This makes ChangeTime change after a rename through an already-open
handle, while Windows keeps the handle's ChangeTime stable for this
case.
Store the SMB ChangeTime in struct ksmbd_file when the handle is opened
and use that value for create, close, and handle-based query information
responses. If a client explicitly sets FILE_BASIC_INFORMATION
ChangeTime, update the stored value as well.
This fixes smbtorture smb2.rename.simple_modtime, which expects
ChangeTime and LastWriteTime to remain unchanged after renaming an
already-open file.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
|
|
The SMB2_CREATE_APP_INSTANCE_ID create context is used with durable v2
opens to identify another open from the same application instance. When
a new durable v2 open arrives with the same AppInstanceId as an existing
open, the server should close the previous open without sending an
oplock break notification.
ksmbd ignored this create context. A second durable v2 batch oplock open
with the same AppInstanceId therefore went through the normal competing
open path and sent an oplock break to the first opener. smbtorture
smb2.durable-v2-open.app-instance expects no oplock break and then
expects the old handle to be closed.
Parse and store AppInstanceId for durable v2 opens. Before creating the
new open, find an existing file with the same AppInstanceId and close it
through the normal close teardown path without issuing an oplock break.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
|
|
When a durable handle is preserved after a disconnect, its oplock state
can still block later opens. If another client opens the same file and
the preserved oplock or lease has to be broken, the old durable handle
must no longer be reconnectable after the break cannot be acknowledged.
ksmbd was treating a missing connection, or an oplock break timeout, as a
successful break only by downgrading the oplock state. The old durable
handle remained reconnectable, so a later durable reconnect for that
stale handle could succeed.
The open path can also see a detached durable handle before the break
notification helpers fully dispose of it. Invalidate such a preserved
durable handle directly when a competing open has to break its batch or
exclusive oplock, while leaving ordinary durable reconnects without a
competing open untouched.
If the old handle still reaches the reconnect path, reject it when the
same inode already has another active open. This matches the
smb2.durable-open.open2-lease/open2-oplock sequence where a later open
replaces the disconnected durable owner and the stale first handle must
not be reclaimed.
Also, reconnect lookup used only the persistent id. A new durable open
can get a persistent id that matches the stale reconnect request after
the old durable state is invalidated. Preserve the disconnected
handle's old volatile id and require durable reconnect contexts to match
it, so a stale reconnect cannot attach to a different durable open.
Windows allows the later open to proceed and rejects the old reconnect
with STATUS_OBJECT_NAME_NOT_FOUND. The smbtorture
smb2.durable-open.oplock test covers this case.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
|
|
A durable handle opened with FILE_DELETE_ON_CLOSE is preserved across a
disconnect so it can be reclaimed by a durable reconnect.
smb2.durable-open.delete_on_close2 disconnects such a handle and then
reconnects it, expecting the reconnect to succeed.
When the client does not reconnect but instead opens the same name with a
new delete-on-close create, the preserved handle keeps the file present
with delete-on-close set. ksmbd then rejects the new open with
STATUS_ACCESS_DENIED on the file_present + FILE_DELETE_ON_CLOSE +
OPEN_IF/OVERWRITE_IF path. smb2.durable-open.delete_on_close1 expects this
open to create a fresh, empty file instead, i.e. the disconnected handle's
delete-on-close must take effect first.
Add ksmbd_close_disconnected_durable_delete_on_close(), which closes
disconnected (conn == NULL) durable handles that keep a delete-on-close
file present. The final close promotes S_DEL_ON_CLS to S_DEL_PENDING and
unlinks the file, so a re-resolved path is absent and the new open creates
it fresh. Call it from smb2_open() before the delete-on-close conflict
check, only for the conflicting open shapes. A live (connected) handle
still keeps the file and blocks the open as before.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
|
|
SMB2_LOCK adds each granted byte-range lock to both the file lock list
and the lock list of the connection which handled the request. The
final close and durable handle paths, however, remove the connection
list entry while holding fp->conn->llist_lock.
With SMB3 multichannel, the connection handling the LOCK request can be
different from the connection which opened the file. The entry can
therefore be removed under a different spinlock from the one protecting
the list it belongs to. A concurrent traversal can then access freed
struct ksmbd_lock and struct file_lock objects.
Record the connection owning each lock's clist entry and hold a
reference to it while the entry is linked. Use that connection and its
llist_lock for unlock, rollback, close, and durable preserve. Durable
reconnect assigns the new connection as the owner when publishing the
locks again.
Fixes: f5a544e3bab7 ("ksmbd: add support for SMB3 multichannel")
Cc: stable@vger.kernel.org
Reported-by: Musaab Khan <musaab.khan@protonmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
|
|
smb2_query_dir() stores a pointer to its stack-allocated private data in
the ksmbd_file readdir_data. Concurrent QUERY_DIRECTORY requests using the
same file handle can overwrite this pointer while an iterate_dir() callback
is still using it, resulting in a stack use-after-free.
Add a per-file mutex and hold it while accessing the shared directory
enumeration state. The lock covers scan restart, dot entry state,
readdir_data setup and iteration, and response construction. This prevents
another request from replacing readdir_data.private before the current
request has finished using it and also serializes the shared file position.
Cc: stable@vger.kernel.org
Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-30527
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
|
|
__close_file_table_ids() is the per-session teardown that closes every
fp belonging to a session (or to one tree connect on that session) by
walking the session's volatile-id idr. The current loop has three
related problems on busy or racing workloads:
* Sleeping under ft->lock. The session-teardown skip callback,
session_fd_check(), already sleeps in ksmbd_vfs_copy_durable_owner()
-> kstrdup(GFP_KERNEL) and down_write(&fp->f_ci->m_lock) (a
rw_semaphore). Running the callback inside write_lock(&ft->lock)
trips CONFIG_DEBUG_ATOMIC_SLEEP / CONFIG_PROVE_LOCKING on a
durable-fd workload.
* Refcount accounting blind to f_state. The unconditional
atomic_dec_and_test(&fp->refcount) does not distinguish
FP_INITED (idr-owned reference still intact) from FP_CLOSED (an
earlier ksmbd_close_fd() already consumed the idr-owned reference
while leaving fp in the idr because a holder kept refcount
non-zero). When the latter races with teardown the same path
over-decrements into a holder reference and ksmbd_fd_put() later
UAFs that holder.
* FP_NEW window. Between __open_id() publishing fp into the
session idr and ksmbd_update_fstate(..., FP_INITED) committing the
transition at the end of smb2_open(), an fp is in FP_NEW and an
intervening teardown that takes a transient reference and
unpublishes the volatile id leaves the original idr-owned
reference orphaned -- the opener is unaware that fp has been
unpublished, returns success to the client, and the fp leaks at
refcount = 1.
Refactor __close_file_table_ids() to take a transient reference on fp
and unpublish fp from the session idr *under ft->lock* before calling
skip() outside the lock. A transient ref protects lifetime but not
concurrent field mutation, so the idr_remove() is what keeps
__ksmbd_lookup_fd() through this session's idr from granting a new
ksmbd_fp_get() reference to an fp whose fp->conn / fp->tcon /
fp->volatile_id / op->conn / lock_list links are about to be rewritten
by session_fd_check(). Durable reconnect is unaffected because it
reaches fp through the global durable table (ksmbd_lookup_durable_fd
-> global_ft).
Decide n_to_drop together with any FP_INITED -> FP_CLOSED transition
under ft->lock so teardown and ksmbd_close_fd() never both consume the
idr-owned reference. See ksmbd_mark_fp_closed() for the per-state
accounting. For the FP_NEW path to be safe, the opener has to learn
that fp was unpublished: ksmbd_update_fstate() now returns -ENOENT
when an FP_NEW -> FP_INITED transition finds f_state already advanced
or the volatile id cleared (both committed by teardown under
ft->lock); smb2_open() propagates that as STATUS_OBJECT_NAME_INVALID
and drops the original reference via ksmbd_fd_put().
The list removal cannot be left for a deferred final putter because
fp->volatile_id has already been cleared and __ksmbd_remove_fd() will
intentionally skip both idr_remove() and list_del_init(). Move the
m_fp_list unlink in __ksmbd_remove_fd() above the volatile-id check so
that an FP_NEW fp that happened to be added to m_fp_list (smb2_open()
adds fp->node before ksmbd_update_fstate() runs) is still cleaned up
on the deferred putter path; list_del_init() on an empty node is a
no-op and remains safe for fps that were never added.
Add a defensive guard in session_fd_check() that refuses non-FP_INITED
fps so that even if a teardown reaches an FP_NEW fp it falls into the
close branch (where the n_to_drop = 1 accounting keeps the opener's
reference alive) instead of the durable-preserve branch (which mutates
fp->conn / fp->tcon).
Validation on a debug kernel additionally built with CONFIG_DEBUG_LIST
and CONFIG_DEBUG_OBJECTS_WORK used a same-session two-tcon workload
(open/write storm on one tcon, 50 tree disconnects on the other) and
reported no list-corruption, work_struct ODEBUG, sleep-in-atomic,
lockdep or kmemleak reports. Reverting only the
__close_file_table_ids() hunk while keeping a forced-is_reconnectable()
harness produced the expected sleep-in-atomic at vfs_cache.c:1095,
confirming the ft->lock-out-of-sleepable-skip discipline.
KASAN-enabled direct SMB2 coverage with durable handles enabled
exercised ksmbd_close_tree_conn_fds(), ksmbd_close_session_fds(),
the FP_NEW failure path, tree_conn_fd_check(), and a non-zero
session_fd_check() durable-preserve return. This produced no KASAN,
DEBUG_LIST, ODEBUG, or WARNING reports.
Fixes: f44158485826 ("cifsd: add file operations")
Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
|
|
Currently, ksmbd does not verify if the user attempting to reconnect
to a durable handle is the same user who originally opened the file.
This allows any authenticated user to hijack an orphaned durable handle
by predicting or brute-forcing the persistent ID.
According to MS-SMB2, the server MUST verify that the SecurityContext
of the reconnect request matches the SecurityContext associated with
the existing open.
Add a durable_owner structure to ksmbd_file to store the original opener's
UID, GID, and account name. and catpure the owner information when a file
handle becomes orphaned. and implementing ksmbd_vfs_compare_durable_owner()
to validate the identity of the requester during SMB2_CREATE (DHnC).
Fixes: c8efcc786146 ("ksmbd: add support for durable handles v1/v2")
Reported-by: Davide Ornaghi <d.ornaghi97@gmail.com>
Reported-by: Navaneeth K <knavaneeth786@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
|
|
If the client sends SMB2_CREATE_POSIX_CONTEXT to ksmbd, allow the filename
to contain a colon (':'). This requires disabling the support for Alternate
Data Streams (ADS), which are denoted by a colon-separated suffix to the
filename on Windows. This should not be an issue, since this concept is not
known to POSIX anyway and the client has to explicitly request a POSIX
context to get this behavior.
Link: https://lore.kernel.org/all/f9401718e2be2ab22058b45a6817db912784ef61.camel@rx2.rx-server.de/
Signed-off-by: Philipp Kerling <pkerling@casix.org>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
|
|
The bug only appears when:
- windows 11 copies a file that has an alternate data stream
- streams_xattr is enabled on the share configuration.
Microsoft Edge adds a ZoneIdentifier data stream containing the URL
for files it downloads.
Another way to create a test file:
- open cmd.exe
- echo "hello from default data stream" > hello.txt
- echo "hello again from ads" > hello.txt:ads.txt
If you open the file using notepad, we'll see the first message.
If you run "notepad hello.txt:ads.txt" in cmd.exe, we should see
the second message.
dir /s /r should least all streams for the file.
The truncation happens because the windows 11 client sends
a SetInfo/EndOfFile message on the ADS, but it is instead applied
on the main file, because we don't check fp->stream.
When receiving set/get info file for a stream file, Change to process
requests using stream position and size.
Truncate is unnecessary for stream files, so we skip
set_file_allocation_info and set_end_of_file_info operations.
Reported-by: Marios Makassikis <mmakassikis@freebox.fr>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
|
|
smb2pdu.c: In function ‘smb2_open’:
./include/linux/minmax.h:20:28: warning: comparison of distinct
pointer types lacks a cast
20 | (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
| ^~
./include/linux/minmax.h:26:4: note: in expansion of macro ‘__typecheck’
26 | (__typecheck(x, y) && __no_side_effects(x, y))
| ^~~~~~~~~~~
./include/linux/minmax.h:36:24: note: in expansion of macro ‘__safe_cmp’
36 | __builtin_choose_expr(__safe_cmp(x, y), \
| ^~~~~~~~~~
./include/linux/minmax.h:45:19: note: in expansion of macro ‘__careful_cmp’
45 | #define min(x, y) __careful_cmp(x, y, <)
| ^~~~~~~~~~~~~
/home/linkinjeon/git/smbd_work/ksmbd/smb2pdu.c:3713:27: note: in
expansion of macro ‘min’
3713 | fp->durable_timeout = min(dh_info.timeout,
Fixes: c8efcc786146 ("ksmbd: add support for durable handles v1/v2")
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
|
|
Launch ksmbd-durable-scavenger kernel thread to scan durable fps that
have not been reclaimed by a client within the configured time.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
|
|
The expired durable opens should not be reclaimed by client.
This patch add ->durable_scavenger_timeout to fp and check it in
ksmbd_lookup_durable_fd().
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
|
|
lease break wait for lease break acknowledgment.
rwsem is more suitable than unlock while traversing the list for parent
lease break in ->m_op_list.
Cc: stable@vger.kernel.org
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
|
|
Durable file handles allow reopening a file preserved on a short
network outage and transparent client reconnection within a timeout.
i.e. Durable handles aren't necessarily cleaned up when the opening
process terminates.
This patch add support for durable handle version 1 and 2.
To prove durable handles work on ksmbd, I have tested this patch with
the following smbtorture tests:
smb2.durable-open.open-oplock
smb2.durable-open.open-lease
smb2.durable-open.reopen1
smb2.durable-open.reopen1a
smb2.durable-open.reopen1a-lease
smb2.durable-open.reopen2
smb2.durable-open.reopen2a
smb2.durable-open.reopen2-lease
smb2.durable-open.reopen2-lease-v2
smb2.durable-open.reopen3
smb2.durable-open.reopen4
smb2.durable-open.delete_on_close2
smb2.durable-open.file-position
smb2.durable-open.lease
smb2.durable-open.alloc-size
smb2.durable-open.read-only
smb2.durable-v2-open.create-blob
smb2.durable-v2-open.open-oplock
smb2.durable-v2-open.open-lease
smb2.durable-v2-open.reopen1
smb2.durable-v2-open.reopen1a
smb2.durable-v2-open.reopen1a-lease
smb2.durable-v2-open.reopen2
smb2.durable-v2-open.reopen2b
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
|
|
Don't immediately send directory lease break notification on smb2_write().
Instead, It postpones it until smb2_close().
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
|
|
If client send different parent key, different client guid, or there is
no parent lease key flags in create context v2 lease, ksmbd send lease
break to client.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
|
|
xfstests generic/002 test fail when enabling smb2 leases feature.
This test create hard link file, but removeal failed.
ci has a file open count to count file open through the smb client,
but in the case of hard link files, The allocation of ci per inode
cause incorrectly open count for file deletion. This patch allocate
ci per dentry to counts open counts for hard link.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
|
|
fp can used in each command. If smb2_close command is coming at the
same time, UAF issue can happen by race condition.
Time
+
Thread A | Thread B1 B2 .... B5
smb2_open | smb2_close
|
__open_id |
insert fp to file_table |
|
| atomic_dec_and_test(&fp->refcount)
| if fp->refcount == 0, free fp by kfree.
// UAF! |
use fp |
+
This patch add f_state not to use freed fp is used and not to free fp in
use.
Reported-by: luosili <rootlab@huawei.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
|
|
Move CIFS/SMB3 related client and server files (cifs.ko and ksmbd.ko
and helper modules) to new fs/smb subdirectory:
fs/cifs --> fs/smb/client
fs/ksmbd --> fs/smb/server
fs/smbfs_common --> fs/smb/common
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
|
