SMB3 permits a message to be compressed before it is encrypted. After
decrypting such a request, ksmbd must trim the AEAD tag using
OriginalMessageSize, decompress the nested compression transform, and
validate the resulting SMB2 PDU.
Share the decompression helper between the connection receive path and
the post-decryption work path so unencrypted and encrypted compressed
requests follow the same validation.
Fixes: a08de24c2b ("ksmbd: negotiate and decode SMB2 compression")
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Previous-session replacement marks the old session expired but retains its
SMB3 encryption key. An in-flight encrypted request can still arrive on
that connection. Rejecting the expired session before decryption made ksmbd
treat the request as a key failure and abort the transport, causing
reconnect failures.
Allow key lookup for expired sessions that have encryption enabled. Keep
the session reference during validation so the normal
STATUS_USER_SESSION_DELETED response is encrypted with the old key. The
session remains expired and no command is executed.
Fixes: fa9415d402 ("ksmbd: mark SMB2_SESSION_EXPIRED to session when destroying previous session")
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
MS-SMB2 requires the server to disconnect a connection when an
encrypted transform cannot be associated with a session or fails
authenticated decryption. This includes an encrypted request that
still carries a SessionId invalidated through PreviousSessionId.
Move the connection to EXITING and shut down its transport when
decrypt_req() fails. Add the missing TCP shutdown callback so a receive
blocked in kernel_recvmsg() is released; SMB Direct already provides
the corresponding callback.
Plaintext requests using an invalidated SessionId do not take this
path and continue to receive STATUS_USER_SESSION_DELETED.
Fixes: e2f34481b2 ("cifsd: add server-side procedures for SMB3")
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
The normal response path applies an SMB3 transform when the request was
encrypted. Async interim responses, completed compound prefixes and two
CHANGE_NOTIFY cleanup paths write their synthetic response work directly,
bypassing that encryption step.
A packet capture shows FE SMB2 STATUS_PENDING, CREATE and CHANGE_NOTIFY
responses following FD SMB3 requests. The client resets the connection
immediately after receiving those plaintext responses.
Send synthetic interim work through a common helper that applies the
session encryption transform first. A compound prefix shares the original
work's response iov, which encryption would replace in place, so flatten it
into an independently owned work before encrypting and sending it.
Fixes: 64bfa9d49026 ("smb/server: use MSG_EOR for async interim response")
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
ksmbd_all_conn_set_status() treats every connection whose transient
binding flag is set as belonging to the target SessionId. A logoff or
session replacement can consequently move an unrelated connection to
NEED_RECONNECT or NEED_SETUP.
Pass the target session itself and select connections using either the
connection-local session xarray or the session's permanent channel list.
Use the same association test while waiting for requests to drain.
Serialize session-wide status changes under request_lock and do not
overwrite EXITING or RELEASING. Protect the shutdown transition with the
same lock so a concurrent session update cannot revive a closing
connection.
Fixes: f5a544e3ba ("ksmbd: add support for SMB3 multichannel")
Fixes: abcc506a9a ("ksmbd: fix racy issue from smb2 close and logoff with multichannel")
Fixes: c444139cb7 ("ksmbd: rewrite stop_sessions() with restartable iteration")
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
An SMB3 multichannel binding registers the secondary connection in the
session channel list, but does not insert the session into the secondary
connection's session xarray.
The decryption path only searches the connection-local xarray. As a
result, every encrypted request received on a bound channel fails with
"Could not get decryption key".
Use the channel-aware session lookup for decryption. Also stop using the
temporary conn->binding flag to decide whether the global lookup is
allowed. Validate the permanent channel association under chann_lock
instead.
Fixes: f5a544e3ba ("ksmbd: add support for SMB3 multichannel")
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Add a share flag for requiring SMB3 encryption on an individual share.
Advertise SMB2_SHAREFLAG_ENCRYPT_DATA in TREE_CONNECT responses and
reject both unencrypted TREE_CONNECT attempts and plaintext requests for
shares carrying the flag.
Keep BIT(19) reserved for the existing ksmbd-tools WIDE_LINKS flag and
use BIT(20) for the new netlink ABI flag.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
BVT_DirectoryLeasing_ReadWriteHandleCaching requires a parent directory
lease break notification when another client creates a child in the
leased directory. A child CREATE without a lease context did not notify
the parent lease holders because the notification path expected a
non-NULL lease context.
Allow the parent lease notification helper to handle a NULL child lease
context and notify matching parent leases. Invoke it after a child is
created without a lease context while preserving the existing lease-key
filtering for requests that provide one.
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>
BVT_DurableHandleV1_Reconnect_WithBatchOplock,
BVT_DurableHandleV1_Reconnect_WithLeaseV1,
BVT_DurableHandleV2_Reconnect_WithBatchOplock, and
BVT_DurableHandleV2_Reconnect_WithLeaseV1 fail to reconnect a durable
handle when the request leaves VolatileFileId unset.
A durable reconnect request may omit VolatileFileId by setting it to
zero. Treating zero as an ID makes ksmbd reject the request whenever the
saved volatile ID is nonzero.
Only compare the saved and requested volatile IDs when the request
contains a nonzero value. Explicit mismatches continue to be rejected.
This allows SMB2 durable handle V1 and V2 reconnects that identify the
handle through the persistent ID and reconnect context.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
See the procedure below:
smb2_tree_connect
ksmbd_tree_conn_connect
xa_store(&sess->tree_conns, tree_conn->id, tree_conn)
ksmbd_counter_inc(KSMBD_COUNTER_TREE_CONNS)
ksmbd_share_tree_conn_inc(sc)
ksmbd_iov_pin_rsp // fail
status.ret = KSMBD_TREE_CONN_STATUS_NOMEM
// do not disconnect tree_conn
Disconnect the new tree connection if ksmbd_iov_pin_rsp() fails.
Fixes: e2b76ab8b5 ("ksmbd: add support for read compound")
Signed-off-by: Ze Tan <tanze@kylinos.cn>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
A cancelled SMB2 CHANGE_NOTIFY request is completed from system_wq. The
deferred work keeps a reference to the connection, but it is not included
in the connection's r_count. During connection teardown,
ksmbd_conn_transport_destroy() can therefore finish the connection handler
and destroy session proc entries before the deferred response runs.
Account for the deferred cancellation work in r_count. The connection
handler now waits for the deferred response to finish before it
deregisters sessions and removes their proc entries.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
A file_lock retained by ksmbd for byte-range lock bookkeeping can still
be part of the VFS blocked-request graph. In particular, the VFS can
chain a new waiter below an already blocked request through
flc_blocked_requests. The ksmbd_file reference count does not cover that
graph.
Both __ksmbd_close_fd() and the cross-request unlock path free these
retained file_lock objects directly. If a dependent waiter is still
attached, locks_release_private() hits
BUG_ON(!list_empty(&flc->flc_blocked_requests)). The same lifetime
mismatch can leave a freed ksmbd_lock reachable through its request-local
llist.
Detach the file_lock from the blocked-request graph before freeing it in
the close, cross-request unlock, and rollback paths. locks_delete_block()
also wakes requests chained below the object. Remove llist when a
completed lock is published so a globally visible ksmbd_lock no longer
points into the submitting worker's stack.
Fixes: d63528eb0d ("ksmbd: free ksmbd_lock when file is closed")
Reported-by: Kyenghwan Hwang <obnred@gmail.com>
Tested-by: Kyenghwan Hwang <obnred@gmail.com>
Tested-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
struct preauth_session contains the Preauth_HashValue[] array that
might contain sensitive data. Use kfree_sensitive() to clear it
before returning the memory to the heap.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
struct ksmbd_conn contains an embedded struct ntlmssp_auth with the
ciphertext[] and cryptkey[] arrays, so to avoid leaking this information
via the heap, it should be freed with kfree_sensitive().
While we're at it, also use kfree_sensitive() for freeing preauth_info
in ksmbd_conn_free() to avoid that the Preauth_HashValue[] could leak
via the heap here, too.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
struct ksmbd_session contains some arrays with sensitive information, like
sess_key, smb3encryptionkey, smb3decryptionkey and smb3signingkey. Thus
let's make sure that this information cannot leak via the heap and use
kfree_sensitive() to free it.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Use kfree_sensitive() to free the user->passkey (and the struct
ksmbd_login_response in ksmbd_login_user() that contains the same
information) to avoid that this information could leak somewhere
else via the heap.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Sensitive data like keys that are stored in stack-local arrays could be
leaked via the stack to the calling functions, or via the heap when using
only normal kfree() functions. There is no known vulnaribility for this
right now, but it's good security style to explicitly zeroize this
sensitive matieral as soon as possible to avoid that it could be exploited
together with other bugs later.
In calc_ntlmv2_hash(), the struct hmac_md5_ctx is normally cleared during
hmac_md5_final() already, but in case of errors, this function is skipped
and ctx is never zeroized, so add a memzero_explicit(&ctx, sizeof(ctx))
there to fix the problem.
In ksmbd_krb5_authenticate(), the ksmbd_spnego_authen_response contains
the session key in the payload. It's currently freed with plain kvfree().
Let's better use kvfree_sensitive() instead.
In generate_key(), the prfhash[] array is used to calculate the key,
but it's never cleared, so it leaks on the stack. Thus clear this with
a memzero_explicit(), too.
In ksmbd_crypt_message(), the sign[] and key[] arrays are leaked via
the stack, too. Make sure to clear them via memzero_explicit() at the
end.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
In krb5_authenticate(), out_len is calculated to determine available
headroom in response_buf for the incoming Kerberos AP-REP token. However,
when SMB2_SESSION_SETUP is processed as a non-first element of a compounded
SMB2 request, the calculation omits work->next_smb2_rsp_hdr_off.
This causes out_len to overstate remaining buffer headroom by
the cumulative size of prior responses in the compound chain. Consequently,
the length check in ksmbd_krb5_authenticate()
(*out_len <= resp->spnego_blob_len) passes erroneously, allowing memcpy()
to write the AP-REP blob past the end of response_buf into adjacent kernel
heap memory.
Fix this by subtracting work->next_smb2_rsp_hdr_off when computing out_len,
ensuring it accurately reflects physical remaining buffer space.
Signed-off-by: Ilan Dudnik <ilan.dudnik@safebreach.com>
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: c841bd3d8d ("ksmbd: deny renaming directory with open children")
Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
smb2_ioctl() rejects FSCTL_PIPE_TRANSCEIVE with
STATUS_OBJECT_NAME_NOT_FOUND before fsctl_pipe_transceive() runs. RPC
pipe IDs live in sess->rpc_handle_list, a separate namespace from the
ksmbd_file table the generic ksmbd_lookup_fd_slow() gate searches, so
the lookup always misses.
Found while testing generic SMB browsing (Finder's "Connect to
Server"): every DCE/RPC bind over a named pipe (SRVSVC, WKSSVC, SAMR,
LSARPC) failed right after CREATE. Adding FSCTL_PIPE_TRANSCEIVE to the
same no_fileid_ioctl exemption as FSCTL_PIPE_WAIT fixes it, confirmed
by testing a build with and without the change.
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>
In smb2_lock(), mid-batch granted locks are published to connection-wide
(conn->lock_list) and file-wide (fp->lock_list) lists immediately upon
vfs_lock_file() success, while also remaining tracked on the stack-local
rollback_list.
If a subsequent element in the same SMB2_LOCK request array fails
validation or execution, the thread jumps to out: and walks
rollback_list to undo previously granted locks. However, because the
granted lock was already published to conn->lock_list, a concurrent
UNLOCK request on the same connection can find the lock object and
kfree() it before the rollback loop executes.
When the granting thread subsequently walks rollback_list, it
dereferences and frees the already-freed ksmbd_lock structure, resulting
in a Use-After-Free and Double-Free (on both ksmbd_lock and struct
file_lock).
Fix this by deferring the publication of granted locks to
conn->lock_list and fp->lock_list until after the entire array of lock
elements has been processed without error. Mid-batch grants remain
tracked exclusively on the request-local rollback_list until the whole
batch succeeds, eliminating the race window.
Fixes: e2f34481b2 ("cifsd: add server-side procedures for SMB3")
Signed-off-by: Ilan Dudnik <ilan.dudnik@safebreach.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
smb2_lease_break_noti() selects a connection from a shared lease table,
but reads lease->l_lb without lease_list_lock. Connection teardown can
free the table before the notification takes a reference to the selected
connection.
Select and pin the connection while holding the lock protecting its
lifetime, before the allocations that may sleep. Also protect the owner
connection lookup with ci->m_lock, since session reconnect can clear
opinfo->conn under that lock. Transfer the reference to the notification
work and release it on allocation failures or in the existing work cleanup
path.
Fixes: 2145945feb ("ksmbd: route v2 lease breaks on the client lease channel")
Reported-by: Jinpyo Lee <bint4b13@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
smb2_handle_negotiate() records specific failures such as
STATUS_INVALID_PARAMETER or STATUS_NOT_SUPPORTED.
Fixes: e2b76ab8b5 ("ksmbd: add support for read compound")
Signed-off-by: ZhangGuoDong <zhangguodong@kylinos.cn>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
When a later initializer fails, the unwind chain releases resources
created after procfs and then jumps directly to class_unregister().
Returning an error from module_init() leaves the proc tree and its
per-CPU counters allocated.
Fixes: b38f99c121 ("ksmbd: add procfs interface for runtime monitoring and statistics")
Signed-off-by: ZhangGuoDong <zhangguodong@kylinos.cn>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
ksmbd_server_init() calls ksmbd_proc_init() before creating the
remaining proc entries and server subsystems. ksmbd_proc_init() tears
down partial state on a procfs or percpu_counter allocation failure,
but returns void, so ksmbd_server_init() continues as if the counters
were usable.
Once userspace starts the server, server_ctrl_handle_init() calls
ksmbd_proc_reset(), which reaches percpu_counter_set() with a NULL
per-CPU counters pointer on SMP systems. The later ksmbd_proc_create()
calls also receive a NULL parent and may create entries in the /proc
root; ksmbd_proc_cleanup() cannot remove those entries because
ksmbd_proc_fs is NULL.
Fixes: b38f99c121 ("ksmbd: add procfs interface for runtime monitoring and statistics")
Signed-off-by: ZhangGuoDong <zhangguodong@kylinos.cn>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Three ipc_msg_alloc() calls in transport_ipc.c allocate
sizeof(struct) + payload_len + 1, but the extra byte is
unnecessary. The payload data is binary and copied with
memcpy() to the exact size; no null terminator is needed.
This was present in the original commit that introduced the
file, where the structs already used [0] zero-length arrays,
so the +1 was never correct.
Assisted-by: Opencode:Big-Pickle
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Clients set SMB2_LOCKFLAG_FAIL_IMMEDIATELY when a LOCK request contains
multiple lock elements, and servers reject requests that omit it.
Accepting such a request can leave earlier elements locked while a later
element waits asynchronously, enabling prolonged partial lock ownership
and avoidable deadlocks.
Return STATUS_INVALID_PARAMETER before processing any element when a
multi-element lock request contains a blocking lock. Unlock arrays remain
unaffected.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
When vfs_lock_file() defers a lock, smb2_lock() puts its ksmbd_lock on
rollback_list before allocating and registering the asynchronous work.
If either operation fails, rollback assumes that smb_lock->conn is
initialized and dereferences NULL. The deferred file_lock also remains
linked into the VFS blocked-lock state while it is freed.
Keep the lock off rollback_list until async setup succeeds. On setup
failures, explicitly unblock and wake the deferred lock before freeing it
and its ksmbd wrapper.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
A server returns success without processing a lock request when a valid
LockSequenceArray entry contains the same sequence number. The current
verifier only invalidates mismatched entries, so matching requests are
submitted to the VFS again and recorded as duplicate locks.
Make the verifier report matching sequences and skip lock processing for
those replays. Also correct the field comment to describe the sequence
and index bit layout used by the implementation and the protocol. Use
the capabilities advertised by the server when deciding whether lock
sequence verification applies to a multichannel connection.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
SMB2 describes a byte-range lock using an offset and a length, while
Linux file_lock uses an inclusive end offset. smb2_lock() currently sets
fl_end to start + length and consequently locks one extra byte for every
nonzero-length request.
Translate nonzero lengths to start + length - 1 and reject ranges that
cannot be represented by loff_t instead of silently truncating them at
OFFSET_MAX. Track zero-length locks from the request length so one-byte
ranges are not mistaken for zero-length locks after endpoint conversion.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
close may abort an in-flight oplock break while another breaker already
holds an opinfo reference. Releasing pending_break wakes that waiter, but
without serializing the close transition with bit acquisition it can become
a new break owner through the test_and_set_bit() fast path. It can then
overwrite OPLOCK_CLOSING with OPLOCK_ACK_WAIT and continue a break for
a dying opinfo.
Make OPLOCK_CLOSING terminal once the opinfo is removed from the inode
list. Serialize that transition, pending_break acquisition, and
OPLOCK_ACK_WAIT setup with an opinfo state lock. A breaker which loses
the race releases its ownership and returns -ENOENT. Explicitly wake
pending_break waiters during close so they can observe the terminal state.
Also prevent ACK and timeout paths from replacing OPLOCK_CLOSING with
OPLOCK_STATE_NONE.
Fixes: e2f34481b2 ("cifsd: add server-side procedures for SMB3")
Co-developed-by: Yunseong Kim <yunseong.kim@est.tech>
Signed-off-by: Yunseong Kim <yunseong.kim@est.tech>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
FILE_NO_INTERMEDIATE_BUFFERING is a CreateOptions flag and can be
combined with other flags, such as FILE_NON_DIRECTORY_FILE.
Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Store the expiry time from the Kerberos authentication response in
the session and reject requests after that time with
STATUS_NETWORK_SESSION_EXPIRED.
Allow an expired Kerberos session to be reauthenticated. Keep the old SMB
signing key until its SESSION_SETUP response has been signed, then install
the new session key and regenerate the SMB3 keys.
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>
FSCTL_QUERY_ALLOCATED_RANGES treated every range in a file without the
sparse attribute as allocated. Files can have holes after an ordinary write
beyond EOF, so CIFS FIEMAP reported extents for those holes.
SEEK_DATA and SEEK_HOLE are insufficient because unwritten extents
look like holes. Use zero writes for FSCTL_SET_ZERO_DATA on dense files.
Sparse files still use hole punching, and allocated-range queries can use
SEEK_DATA and SEEK_HOLE for both file types.
When clearing the sparse attribute, materialize holes with zero writes
before updating the attribute. This keeps the file fully allocated without
relying on unwritten extents that SEEK_DATA would still report as holes.
Return STATUS_BUFFER_OVERFLOW when another allocated range does not fit in
the SMB response so the client continues the query.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
ksmbd_reopen_durable_fd() walks the inode's m_op_list and rebinds every
detached oplock to the reconnecting session:
list_for_each_entry_rcu(op, &ci->m_op_list, op_entry,
lockdep_is_held(&ci->m_lock)) {
if (op->conn)
continue;
op->conn = ksmbd_conn_get(fp->conn);
op->sess = work->sess;
}
The only key is op->conn == NULL, which every detached durable handle on
that inode matches, not just the one owned by fp. When two sessions hold
durable handles on the same file and both disconnect, reconnecting one of
them adopts the other session's oplock: op->sess is overwritten with the
reconnecting session without taking a reference on it, while op->conn
pins the connection.
The sibling teardown path, session_fd_check(), keys on the identity of
the connection being torn down (op->conn == conn) rather than on shared
state, and so does not have this problem.
Once the adopting session is destroyed, ksmbd_session_destroy() frees it
while the foreign oplock still points at it. The reader in
ksmbd_close_fd_app_instance_id() validates only opinfo->conn, which is
still live thanks to the reference taken above, and then dereferences the
stale session:
if (!opinfo->conn) {
up_read(&fp->f_ci->m_lock);
goto out;
}
ft = &opinfo->sess->file_table;
write_lock(&ft->lock);
BUG: KASAN: slab-use-after-free in _raw_write_lock+0x74/0xd0
Write of size 4 at addr ffff88810a970528 by task kworker/0:0/9
Workqueue: ksmbd-io handle_ksmbd_work
Call Trace:
_raw_write_lock+0x74/0xd0
ksmbd_close_fd_app_instance_id+0x183/0x410
smb2_open+0x1346/0x4430
handle_ksmbd_work+0x2bb/0x7b0
Reached from an authenticated session against a share with the default
durable-handle and oplock configuration: two sessions open the same file
with a durable-v2 handle and an RH lease under distinct AppInstanceIds,
both log off, one reconnects with DH2C, and a later durable-v2 create
carrying the other AppInstanceId walks into the freed session.
Constrain the loop to the oplock owned by the file being reopened.
Fixes: f363a0fb13 ("ksmbd: fix app-instance durable supersede session UAF")
Cc: stable@vger.kernel.org
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
SMB3.1.1 multichannel binding preserves the preauthentication hash in a
preauth_session between the NTLM negotiate and authenticate requests.
The binding NTLM negotiate allocates this object and returns
STATUS_MORE_PROCESSING_REQUIRED. If the client disconnects before it sends
the authenticate request, neither the authenticate nor error cleanup paths
free the object.
Release any remaining preauthentication sessions when tearing down the
connection. Initialize the list when allocating the connection so that this
cleanup is safe regardless of the negotiated dialect.
Reported-by: Runa Takemoto <takemotoruna223@gmail.com>
Fixes: f5a544e3ba ("ksmbd: add support for SMB3 multichannel")
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
A connection-close scan can miss the synthetic CHANGE_NOTIFY work item
because smb2_notify() registers it directly after setup_async_work()
has returned. Link both regular and synthetic async work through one
helper that checks the connection state under request_lock.
If the connection is already closing, release a newly allocated async
ID or complete the synthetic notify work immediately.
Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Co-developed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
An async request may still be waiting when a connection is closed.
This can stop the connection from closing.
Cancel active async requests before waiting for them to finish.
Suggested-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Some SMB responses keep their data in another buffer. The SMB header
and the data are then in different iovs.
The old code only handled this for SMB2 READ. For other commands, it
signed only the last iov. QUERY_INFO and CHANGE_NOTIFY can also use
another iov for their data. Their SMB header was not signed, so Windows
will client rejected the response.
Find the iov that starts with the current SMB header. Sign this iov and
all iovs after it.
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
ipc_validate_msg() computes the expected message size by reading length
fields out of the response buffer supplied by the userspace ksmbd daemon
(payload_sz, session_key_len, ngroups, ...). Those fields are read before
the buffer is verified to be large enough to contain the struct they belong
to, so a short response makes the read land past the end of the allocation.
handle_response() sizes entry->response purely from the netlink attribute
length (nla_len()) and only guards the leading handle read, so the daemon
can install a response as small as the kmalloc-8 object seen below. When
ipc_msg_send_request() then calls ipc_validate_msg() for a
KSMBD_EVENT_RPC_REQUEST, the cast to struct ksmbd_rpc_command reads
resp->payload_sz at offset 8 of an 8-byte allocation:
[ 3697.841381] ==================================================================
[ 3697.844099] BUG: KASAN: slab-out-of-bounds in ipc_msg_send_request+0x763/0x800
[ 3697.846604] Read of size 4 at addr ffff888105f95910 by task kworker/4:3/20682
[ 3697.849061]
[ 3697.849801] CPU: 4 UID: 0 PID: 20682 Comm: kworker/4:3 Not tainted 7.2.0-rc3-next-20260717-virtme #117 PREEMPT(lazy)
[ 3697.850077] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.17.0-debian-1.17.0-1 04/01/2014
[ 3697.850303] Workqueue: ksmbd-io handle_ksmbd_work
[ 3697.850592] Call Trace:
[ 3697.850794] <TASK>
[ 3697.850952] __dump_stack+0x21/0x60
[ 3697.851239] dump_stack_lvl+0xc2/0x100
[ 3697.851528] print_address_description+0x77/0x200
[ 3697.851816] ? ipc_msg_send_request+0x763/0x800
[ 3697.852024] print_report+0x58/0x70
[ 3697.852316] kasan_report+0x117/0x150
[ 3697.852585] ? down_write+0x146/0x1f0
[ 3697.852809] ? ipc_msg_send_request+0x763/0x800
[ 3697.853082] ipc_msg_send_request+0x763/0x800
[ 3697.853385] ? __pfx_ipc_msg_send_request+0x10/0x10
[ 3697.853604] ? kasan_unpoison+0x48/0x70
[ 3697.853936] ? __pfx___up_read+0x10/0x10
[ 3697.854221] ksmbd_rpc_ioctl+0x380/0x520
[ 3697.854542] ? __pfx_ksmbd_rpc_ioctl+0x10/0x10
[ 3697.854757] ? kasan_unpoison+0x48/0x70
[ 3697.854962] ? copy_from_kernel_nofault+0x32c/0x4e0
[ 3697.855166] ? kasan_unpoison+0x48/0x70
[ 3697.855416] fsctl_pipe_transceive+0x139/0x7a0
[ 3697.855705] ? __pfx_copy_from_kernel_nofault+0x10/0x10
[ 3697.855937] ? __pfx_fsctl_pipe_transceive+0x10/0x10
[ 3697.856388] ? __sanitizer_cov_trace_switch+0x7b/0x140
[ 3697.856620] smb2_ioctl+0x1141/0x3420
[ 3697.856994] ? __pfx_smb2_ioctl+0x10/0x10
[ 3697.857182] ? get_smb2_cmd_val+0xe3/0x1c0
[ 3697.857655] handle_ksmbd_work+0x9ad/0x15e0
[ 3697.858034] ? __pfx_handle_ksmbd_work+0x10/0x10
[ 3697.858251] ? lock_release+0xf7/0x360
[ 3697.858466] ? process_scheduled_works+0x954/0x1600
[ 3697.858698] ? process_scheduled_works+0x954/0x1600
[ 3697.858905] process_scheduled_works+0xc22/0x1600
[ 3697.859368] ? __pfx_process_scheduled_works+0x10/0x10
[ 3697.859637] ? __pfx_assign_work+0x10/0x10
[ 3697.859896] ? lock_is_held_type+0x7b/0x110
[ 3697.860146] worker_thread+0x975/0xee0
[ 3697.860524] ? __pfx_do_raw_spin_lock+0x10/0x10
[ 3697.860830] ? __kthread_parkme+0x21e/0x260
[ 3697.861105] kthread+0x3a6/0x490
[ 3697.861423] ? __pfx_worker_thread+0x10/0x10
[ 3697.861643] ? __pfx_kthread+0x10/0x10
[ 3697.861878] ret_from_fork+0x55a/0xa20
[ 3697.862194] ? __pfx_ret_from_fork+0x10/0x10
[ 3697.862480] ? __pfx_kthread+0x10/0x10
[ 3697.862714] ret_from_fork_asm+0x1a/0x30
[ 3697.862965] </TASK>
[ 3697.863039]
[ 3697.938882] Allocated by task 20761:
[ 3697.940257] kasan_save_track+0x3e/0x80
[ 3697.941782] __kasan_kmalloc+0x72/0x90
[ 3697.943228] __kvmalloc_node_noprof+0x3e9/0x6a0
[ 3697.944948] handle_generic_event+0x59b/0x750
[ 3697.946592] genl_family_rcv_msg_doit+0x3d6/0x560
[ 3697.946977] genl_rcv_msg+0x67c/0x900
[ 3697.947224] netlink_rcv_skb+0x286/0x580
[ 3697.947488] genl_rcv+0x2d/0x80
[ 3697.947706] netlink_unicast+0x937/0xb70
[ 3697.947993] netlink_sendmsg+0x977/0xc10
[ 3697.948268] __sock_sendmsg+0x264/0x2d0
[ 3697.948536] __sys_sendto+0x4de/0x690
[ 3697.948789] __x64_sys_sendto+0x173/0x380
[ 3697.949069] do_syscall_64+0x13d/0x420
[ 3697.949328] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 3697.949662]
[ 3697.949779] The buggy address belongs to the object at ffff888105f95908
[ 3697.949779] which belongs to the cache kmalloc-8 of size 8
[ 3697.950550] The buggy address is located 0 bytes to the right of
[ 3697.950550] allocated 8-byte region [ffff888105f95908, ffff888105f95910)
[ 3697.951455]
[ 3697.951574] The buggy address belongs to the physical page:
[ 3697.951958] page: refcount:0 mapcount:0 mapping:0000000000000000 index:0xffff888105f951b8 pfn:0x105f95
[ 3697.952571] flags: 0x100000000000200(workingset|node=0|zone=2)
[ 3697.952973] page_type: f5(slab)
[ 3697.953198] raw: 0100000000000200 ffff888100042640 ffffea0004063610 ffff888100040588
[ 3697.953707] raw: ffff888105f951b8 00000000001c000e 00000000f5000000 0000000000000000
[ 3697.954240] page dumped because: kasan: bad access detected
[ 3697.954616]
[ 3697.954734] Memory state around the buggy address:
[ 3697.955063] ffff888105f95800: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fa
[ 3697.955534] ffff888105f95880: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 3697.956006] >ffff888105f95900: fc 00 fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 3697.956477] ^
[ 3697.956728] ffff888105f95980: fc fc fc fa fc fc fc fc fc fc fc fc fc fc fc fc
[ 3697.957202] ffff888105f95a00: fc fc fc fc fc fa fc fc fc fc fc fc fc fc fc fc
[ 3697.957671] ==================================================================
The final "entry->msg_sz != msg_sz" comparison cannot help: the offending
read has already happened by the time it runs. Every case in the switch
shares this pattern.
Floor entry->msg_sz against the base struct of each event type before
dereferencing any of its length fields. On failure ipc_msg_send_request()
already frees the response and returns NULL, so callers stay safe.
The malformed message originates from the ksmbd.mountd daemon over genl
netlink rather than a remote SMB client, so triggering it requires a buggy
or compromised daemon; it is still an out-of-bounds read the validator is
meant to prevent.
Fixes: d6a6aa81ea ("ksmbd: validate response sizes in ipc_validate_msg()")
Signed-off-by: Yunseong Kim <yunseong.kim@est.tech>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
smbdirect_socket_destroy() releases the listener's pending/ready child
sockets while still holding the listener's handler lock, the
&id_priv->handler_mutex taken via rdma_lock_handler(), not
sc->listen.lock, and before the listener's own rdma_destroy_id().
That ordering has one real consequence and one cosmetic one.
The real one: smbdirect_socket_release() drops the child's last
reference, which destroys the child's cm_id. Doing that before the
listener's rdma_destroy_id() lets _cma_cancel_listens(), running from
the listener's _destroy_id(), walk an already freed child id_priv,
which KASAN catches as a slab-use-after-free during listener shutdown:
[ 4758.909130] BUG: KASAN: slab-use-after-free in __mutex_lock+0x1469/0x1560
[ 4758.911450] Read of size 1 at addr ffff88821c381db4 by task ksmbd.control/1652
[ 4758.913262] Call Trace:
[ 4758.913267] <TASK>
[ 4758.913299] __mutex_lock+0x1469/0x1560
[ 4758.913408] _cma_cancel_listens+0x312/0x3b0
[ 4758.913413] _destroy_id+0x363/0xee0
[ 4758.913417] smbdirect_socket_destroy_sync+0x17d5/0x2440
[ 4758.913443] smbdirect_socket_release+0x124/0x230
[ 4758.913451] ksmbd_rdma_stop_listening+0x9f/0x190
[ 4758.913457] ksmbd_conn_transport_destroy+0x65/0x3c0
[ 4758.913463] kill_server_store+0x1fb/0x2b0
[ 4758.913501] kernfs_fop_write_iter+0x349/0x4d0
[ 4758.913507] vfs_write+0x5e7/0xc70
[ 4758.913528] ksys_write+0x12a/0x210
[ 4758.913541] do_syscall_64+0x135/0x460
[ 4758.913555] entry_SYSCALL_64_after_hwframe+0x77/0x7f
The cosmetic one: releasing a child recurses into
smbdirect_socket_destroy(), which takes the child's own
rdma_lock_handler() lock nested under the listener's. The listener's
and the child's cm_id are always different instances, so this cannot
deadlock for real; the CM core itself nests a new connection id's
handler_mutex under the listening id's in cma_ib_req_handler(). But
lockdep only sees one lock class, reports possible recursive locking,
and then disables itself, hiding real locking bugs for the rest of the
run:
[ 2424.579653] WARNING: possible recursive locking detected
[ 2424.581180] 7.1.0-next-20260623+ #89 Not tainted
[ 2424.582548] --------------------------------------------
[ 2424.584500] ksmbd.control/8854 is trying to acquire lock:
[ 2424.586817] ffff888102303c20 (&id_priv->handler_mutex){+.+.}-{4:4}, at: smbdirect_socket_destroy_sync+0xc39/0x2440
[ 2424.590590]
[ 2424.590590] but task is already holding lock:
[ 2424.591601] ffff888102046c20 (&id_priv->handler_mutex){+.+.}-{4:4}, at: smbdirect_socket_destroy_sync+0xc39/0x2440
[ 2424.594178]
[ 2424.594178] other info that might help us debug this:
[ 2424.596634] Possible unsafe locking scenario:
[ 2424.596634]
[ 2424.598841] CPU0
[ 2424.599765] ----
[ 2424.600695] lock(&id_priv->handler_mutex);
[ 2424.601836] lock(&id_priv->handler_mutex);
[ 2424.602590]
[ 2424.602590] *** DEADLOCK ***
[ 2424.602590]
[ 2424.604512] May be due to missing lock nesting notation
Splice the pending/ready children onto a local list under the
listener's listen.lock, while the handler lock is held so a concurrent
CM CONNECT_REQUEST cannot add more, but defer the actual
smbdirect_socket_release() calls until after the listener's cm_id has
been destroyed and its handler lock dropped. The children are
independent sockets whose teardown needs neither the listener's
handler lock nor its cm_id.
Found with ksmbdzzer [2], a KSMBD fuzzer that drives libFuzzer with a
kcov-dataflow [1] coverage vector: it folds each instrumented
comparison/argument's runtime operand value together with its PC (the
default arm mixes them as pc⊕val) so that a new operand value at a known
site counts as new coverage.
[1] https://lwn.net/Articles/1077606/
[2] https://github.com/yskzalloc/kcov-dataflow
Fixes: dc691b91ad ("smb: smbdirect: introduce smbdirect_socket_{listen,accept}()")
Signed-off-by: Yunseong Kim <yunseong.kim@est.tech>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
__smbdirect_socket_schedule_cleanup() and smbdirect_socket_cleanup_work()
take sc->listen.lock and walk the listener's pending list, recursing
into smbdirect_socket_schedule_cleanup() for each child, and every
child takes its own listen.lock inside that walk.
This cannot deadlock for real: the nesting is strictly listener into
child, a child never listens, so the outer and the inner lock are
always different instances. lockdep only tracks lock classes, though;
it sees the same class acquired twice and reports a possible recursive
locking deadlock. This change therefore works around a lockdep
limitation rather than fixing a real hang, but the report is still
worth avoiding: lockdep disables itself after the first splat and then
hides real locking bugs for the rest of the run.
Only a socket that was a listener owns a populated listen.ready/pending
list; a child has empty lists and nothing to do in these blocks. Guard
both of them with sc->listen.backlog != -1, the "was a listener" marker
that smbdirect_socket_destroy() already uses: listen.backlog leaves its
initial -1 exactly once, when smbdirect_socket_listen() succeeds. The
alternative !sc->accept.listener test reads as "not a listener" while
meaning the opposite, and it is also true for an accepted child, whose
accept.listener has been cleared on hand-over. With the guard the walk
only runs for a listener and never nests a child's listen.lock under
it; a pending child stays on its listener's list for the free path
(smbdirect_socket_destroy) to reap.
[ 741.705044] WARNING: possible recursive locking detected
[ 741.705403] 7.1.0-next-20260623+ #75 Not tainted
[ 741.705695] --------------------------------------------
[ 741.706022] ksmbd.control/18502 is trying to acquire lock:
[ 741.706379] ffff888108d612f8 (&sc->listen.lock){....}-{3:3}, at: __smbdirect_socket_schedule_cleanup+0x719/0xd70
[ 741.707008]
[ 741.707008] but task is already holding lock:
[ 741.707396] ffff8881087642f8 (&sc->listen.lock){....}-{3:3}, at: __smbdirect_socket_schedule_cleanup+0x719/0xd70
[ 741.708025]
[ 741.708025] other info that might help us debug this:
[ 741.708448] Possible unsafe locking scenario:
[ 741.708448]
[ 741.708845] CPU0
[ 741.709016] ----
[ 741.709186] lock(&sc->listen.lock);
[ 741.709453] lock(&sc->listen.lock);
[ 741.709705]
[ 741.709705] *** DEADLOCK ***
[ 741.709705]
[ 741.710095] May be due to missing lock nesting notation
[ 741.710095]
[ 741.710663] 6 locks held by ksmbd.control/18502:
[ 741.710975] #0: ffff888109e51420 (sb_writers#7){.+.+}-{0:0}, at: vfs_write+0x1e7/0xc70
[ 741.711561] #1: ffff888126ec3880 (&of->mutex){+.+.}-{4:4}, at: kernfs_fop_write_iter+0x1be/0x4d0
[ 741.712147] #2: ffff888102af17b0 (kn->active#45){.+.+}-{0:0}, at: kernfs_fop_write_iter+0x205/0x4d0
[ 741.712803] #3: ffffffff85ad1e00 (ctrl_lock){+.+.}-{4:4}, at: kill_server_store+0x1e0/0x2b0
[ 741.713381] #4: ffffffff85ad41a0 (init_lock){+.+.}-{4:4}, at: ksmbd_conn_transport_destroy+0x5b/0x3c0
[ 741.713995] #5: ffff8881087642f8 (&sc->listen.lock){....}-{3:3}, at: __smbdirect_socket_schedule_cleanup+0x719/0xd70
[ 741.714736]
[ 741.714736] stack backtrace:
[ 741.715038] CPU: 4 UID: 0 PID: 18502 Comm: ksmbd.control Not tainted 7.1.0-next-20260623+ #75 PREEMPT(lazy)
[ 741.715043] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.17.0-debian-1.17.0-1 04/01/2014
[ 741.715046] Call Trace:
[ 741.715049] <TASK>
[ 741.715052] dump_stack_lvl+0x77/0xa0
[ 741.715058] print_deadlock_bug+0x279/0x290
[ 741.715065] __lock_acquire+0x272a/0x2e30
[ 741.715070] ? stack_trace_save+0xae/0x100
[ 741.715075] ? smb_direct_logging_vaprintf+0x1a0/0x230
[ 741.715079] ? __pfx_smb_direct_logging_vaprintf+0x10/0x10
[ 741.715082] ? __timer_delete+0x58/0x320
[ 741.715087] lock_acquire+0xd3/0x270
[ 741.715091] ? __smbdirect_socket_schedule_cleanup+0x719/0xd70
[ 741.715095] ? entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 741.715099] _raw_spin_lock_irqsave+0x42/0x60
[ 741.715105] ? __smbdirect_socket_schedule_cleanup+0x719/0xd70
Note the two addresses above: ffff888108d612f8 is the child's lock,
ffff8881087642f8 the listener's, always distinct objects.
Found with ksmbdzzer [2], a KSMBD fuzzer that drives libFuzzer with a
kcov-dataflow [1] coverage vector: it folds each instrumented
comparison/argument's runtime operand value together with its PC (the
default arm mixes them as pc⊕val) so that a new operand value at a known
site counts as new coverage.
[1] https://lwn.net/Articles/1077606/
[2] https://github.com/yskzalloc/kcov-dataflow
Fixes: dc691b91ad ("smb: smbdirect: introduce smbdirect_socket_{listen,accept}()")
Signed-off-by: Yunseong Kim <yunseong.kim@est.tech>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
