| Age | Commit message (Collapse) | Author |
|---|
|
The pending setup limit only counts InProgress sessions. A client can
complete authentication repeatedly and retain an unbounded number of Valid
sessions on one connection.
Reserve a connection slot before allocating a session and retain a
connection reference while authentication is in progress. The first SMB3
channel takes over that reservation; binding to another connection reserves
one slot there. Reauthentication and authentication continuation reuse the
existing reservation. Return slots on allocation or insertion failure,
channel removal, and final session destruction.
Accept max_sessions_per_connection in reserved startup-request space and
default to 64 when older tools leave it zero. Keep the request size and
existing field offsets unchanged. Expose the configured limit in procfs.
The existing connection limits bound the aggregate number of sessions
without a separate global session allocation limit.
Fixes: d7fd1f98607f ("ksmbd: follow SMB2 session expiration semantics")
Cc: stable@vger.kernel.org
Reported-by: Abdifatah Suruur <suruurism@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
ksmbd_session_unregister() removes a session from the global and per-
connection tables before dropping the table reference. A concurrent
reference holder can keep the session and its channel list alive after
those removals.
Connection cleanup can no longer find the unpublished session to remove its
channels. Since a channel only stores a raw conn pointer, the connection
can be freed while another session reference holder is still accessing it,
for example during SMB3.1.1 binding or a procfs read.
Take a connection reference when registering a channel and release it on
insertion failure, channel removal, or final session destruction. Keep the
channels and signing keys until the session is destroyed so a response can
still be signed after the session has been unregistered.
Fixes: d7fd1f98607f ("ksmbd: follow SMB2 session expiration semantics")
Cc: stable@vger.kernel.org
Reported-by: Abdifatah Suruur <suruurism@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
The expiration worker holds conn_list_lock for reading while checking
for authenticated sessions. Without this change, the check can wait for
conn->session_lock or sessions_table_lock.
In destroy_previous_session(), the status updates after
ksmbd_conn_wait_idle_sess() run with both session locks held for
writing. Both the KSMBD_SESS_GOOD error path and the
KSMBD_SESS_NEED_SETUP success path call ksmbd_all_conn_set_status(),
which takes conn_list_lock for reading.
A writer queued on conn_list_lock can block these new read acquisitions.
The worker then waits for a session lock held by
destroy_previous_session(), which waits behind the writer. The writer
cannot proceed until the worker releases conn_list_lock.
Try both session locks instead of waiting in the scavenging check. Keep
the connection when either lock is busy and let the periodic worker
retry later.
Fixes: d7fd1f98607f ("ksmbd: follow SMB2 session expiration semantics")
Cc: stable@vger.kernel.org
Reported-by: Abdifatah Suruur <suruurism@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
ksmbd-tools releases before 3.5.2 do not have the payload_sz field in
struct ksmbd_share_config_response; the field occupies the last word of
their reserved[] area, so the kernel always sees payload_sz == 0 from
them. ipc_validate_msg() and share_config_request() used to treat
payload_sz == 0 as "legacy userspace" and skip the payload size checks.
That case was removed by
commit f25e93768fcc ("ksmbd: prevent out-of-bounds reads in share config
responses").
With payload_sz == 0:
- the IPC$ (pipe) response fails the final message size comparison:
these tools always send a one-byte payload, so msg_sz is
sizeof(*resp) + 1, but the expected size is now computed as
sizeof(*resp) + 0;
- every other share fails "payload_sz <= veto_list_sz".
In addition, ksmbd-tools before 3.5.3 copy a global "veto files" setting
into the IPC$ response, so with such a configuration IPC$ also fails the
now unconditional "veto_list_sz > payload_sz" check, even though pipe
shares never use a veto list.
ksmbd_ipc_share_config_request() then returns NULL and every tree
connect fails with STATUS_BAD_NETWORK_NAME, so ksmbd cannot serve any
share when used with these ksmbd-tools releases, for example the 3.5.1
package shipped by Ubuntu 24.04. Login still works, which makes the
failure look like a configuration problem:
CIFS: VFS: BAD_NETWORK_NAME: \\127.0.0.1\IPC$
CIFS: VFS: \\127.0.0.1 failed to connect to IPC (rc=-2)
When payload_sz is zero, derive it from the size of the message actually
received and store it back in the response, so that the checks in
ipc_validate_msg() and the path and veto list parsing in
share_config_request() all use the same bound. The response is the
kernel's own copy made by handle_response(), and msg_sz has already been
checked to be at least sizeof(*resp), so this cannot underflow. All
checks added by that commit then apply unchanged, bounded by the bytes
that really arrived, and newer ksmbd-tools that fill in payload_sz are
unaffected. Also skip the veto list size check for pipe shares:
share_config_request() reads neither the path nor the veto list for
them.
Tested in QEMU with ksmbd-tools 3.5.1 and 3.5.7 and the in-kernel SMB
client over loopback, with and without a global "veto files" setting.
Fixes: f25e93768fcc ("ksmbd: prevent out-of-bounds reads in share config responses")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Michail Andreas Michalopoulos <mike.michalo@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
SetAllocation rounds requests to 512-byte blocks, although
FS_SIZE_INFORMATION advertises the filesystem's f_bsize as its
allocation unit. For example, on a filesystem with a 4 KiB unit, a
1000-byte request can truncate an 8 KiB file to 1024 bytes instead of
4 KiB.
Round the request to the advertised unit and use the same result for
the block-count decision, fallocate, truncate, and the size stored on
the open handle. Validate the unit and rounded size before either
operation. Use DIV_ROUND_UP_ULL() so the calculation works on 32-bit
kernels.
This does not change the existing block-count decision for whether to
call fallocate. In particular, success does not yet guarantee
reservation of holes within the requested range on a sparse file.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
When SetAllocation requests a size below the allocated block count but
above EOF, ksmbd truncates the file to that size and restores only the
in-memory i_size. The larger EOF remains on disk and reappears after a
reboot. A 3-byte file preallocated to 64 KiB can acquire a 4 KiB EOF
after reducing its allocation to 4 KiB. Extending EOF this way is not
a reliable way to reclaim the preallocation beyond the old EOF either.
Only use this truncate path when the requested allocation is at or below
the observed EOF. A truncate to the existing EOF still lets filesystems
release preallocated blocks beyond EOF. Remove the in-memory-only i_size
restoration. The size check still precedes vfs_truncate() taking the
inode lock, so a concurrent shrink can make the truncate extend EOF;
that race also existed in the original path.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Cc: stable@vger.kernel.org
Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
MS-SMB2 specifies that subsequent related operations use the FileId from
the preceding operation. Track FileIds from handle-bearing requests so
related requests that use the all-ones placeholder resolve to the
inherited handle.
Return STATUS_INVALID_HANDLE when a related request refers to a FileId
but the preceding operation neither contained nor generated one.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Reported-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
smb_strtoUTF16() always writes a terminating zero UTF-16 unit after the
converted characters, but get_file_stream_info() budgets only
sizeof(struct smb2_file_stream_info) + streamlen * 2 for a stream entry
and sizeof(struct smb2_file_stream_info) + 7 * 2 for the trailing
"::$DATA" entry. When an entry exactly fills the 448-byte small response
buffer, the terminator is written two bytes past the allocation.
Reserve one extra UTF-16 unit in both budget checks. The terminator is
not part of the wire format, so the reported lengths are unchanged.
Cc: stable@vger.kernel.org
Signed-off-by: Abdifatah Suruur <suruurism@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
An authenticated client with a writable tree connection can open a
directory with FILE_WRITE_DATA and send FSCTL_SET_ZERO_DATA on that
handle. The handler only checks fp->daccess & FILE_WRITE_DATA_LE, but on
a directory FILE_WRITE_DATA is the legitimate add-file right, so the
check passes. ksmbd_vfs_zero_data() then calls kernel_write() on the
directory file, which has no FMODE_WRITE, and __kernel_write_iter()
hits WARN_ON_ONCE(!(file->f_mode & FMODE_WRITE)), panicking a
panic_on_warn kernel.
Before commit 25e414db7033 ("ksmbd: report holes in allocated range
queries") the dense-file path used vfs_fallocate(), which rejects
directories with -EISDIR. The kernel_write() loop that replaced it has
no such check. Reject a directory handle in ksmbd_vfs_zero_data() before
writing, as ksmbd_vfs_write() already does for SMB2_WRITE.
WARNING: fs/read_write.c:608 at __kernel_write_iter+0x74b/0x8c0
Workqueue: ksmbd-io handle_ksmbd_work
Call Trace:
kernel_write+0x1ef/0x4d0
ksmbd_vfs_zero_data+0x413/0x590
smb2_ioctl+0x21fa/0x66a0
handle_ksmbd_work+0x61b/0x1ff0
Fixes: 25e414db7033 ("ksmbd: report holes in allocated range queries")
Signed-off-by: Lee Seong Hyeon <ihopenre@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
POSIX filesystem information uses f_bsize for BlockSize even when the
filesystem reports a distinct fragment size for its block counts.
Use f_frsize when available, falling back to f_bsize. Keep f_bsize as
the optimal transfer size and derive the response length from the
information structure instead of a hard-coded value.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
POSIX filesystem queries use the share root for statfs and do not
validate the requested file handle. This can report the wrong filesystem
for files below a mount point within the share.
Require SMB3.1.1 POSIX extensions and a valid POSIX open. Resolve the
requested handle, including compound file IDs, and use its referenced
path for the filesystem query.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
POSIX file information queries only check the tree's extension flag,
while directory queries do not verify that the directory was opened with
a POSIX create context.
Require SMB3.1.1, negotiated POSIX tree extensions and a POSIX open for
both file and directory POSIX information requests. Return
STATUS_INVALID_INFO_CLASS when these conditions are not met.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
The POSIX file query response ends after the owner and group SIDs,
omitting the filename length and filename.
Append the path relative to the share in UTF-16, without a leading
separator, and include its byte length in the response. Check both the
fixed fields and the converted filename against the available output
buffer before writing the response.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
POSIX responses omit special permission bits, and the create response
reports a file identifier as its reparse tag without encoding the file
type. Directory and file queries also report rdev as the filesystem
device identifier.
Report the POSIX mode and file type in create responses, include
special permission bits in all three response paths, and use the
special-file reparse tag consistently. Set the corresponding DOS
attribute in directory and file queries, report the filesystem device
identifier and initialize reserved fields.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
Explicit modes carried by NFS mode ACEs can be overwritten by the DACL
permission fallback, and security updates discard special mode bits.
Track modes supplied by ACCESS_ALLOWED NFS mode ACEs and bypass the
permission fallback for them. Apply both permission and special bits
when setting an explicit POSIX mode.
For SET_INFO, require a POSIX open for explicit mode changes and apply
the mode only when DACL security information is requested.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
The share mode helpers treat a requested POSIX mode of zero as an
unspecified mode, replacing it with the share's default permissions.
Use the requested mode, including zero, before applying the share masks
and forced modes. Pass an explicit default-mode sentinel for non-POSIX
creates to keep their existing mode selection.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
Using the tree's POSIX extension flag to skip byte-range lock checks
also skips them for ordinary opens on a POSIX-enabled tree.
Base the read and write lock checks on each file's POSIX create context
instead.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
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>
|
|
POSIX create contexts are only examined when the tree has POSIX
extensions enabled, and IPC requests bypass this validation.
Examine POSIX contexts before selecting the IPC create path. Reject
duplicate contexts and require an SMB3.1.1 connection, a POSIX-enabled
tree and a non-IPC share for POSIX creates.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
The POSIX negotiate context currently enables extensions without
checking its payload, and duplicate contexts are accepted.
Reject duplicate contexts and invalid payload lengths. Enable POSIX
extensions only for the published tag, and use the same tag constant
when building the negotiate response.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
Arm a lease break acknowledgment timer once the common notification path
accepts an ACK-required notification for transmission. Keep the deadline
on the lease across channel retries and scan every lease in the global
lease tables when the timer expires. Set timed-out leases and all their
opens to NONE, then rearm for the next deadline.
Serialize client acknowledgments, notification state and timer expiry
under the lease lock, taking each open's state lock after it. Update every
open before publishing break completion so a late acknowledgment cannot
restore an expired lease. Wake blocked opens on timeout, send failure and
global lease-table teardown, and cancel the timer at teardown.
Keep the existing connection-independent lease-table lifetime and use the
notifier's opinfo reference to retain the lease. Stop after selecting one
open during lease-key lookup.
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>
|
|
Disconnecting one channel destroys the lease table for its ClientGuid,
even when another channel still uses the same opens. A lease break ACK
on the surviving channel can no longer find the lease. Detached durable
opens also lose their lease lookup entries before reconnect or expiry.
Tie a client table to its lease entries instead of the connection which
created it. Remove the table when its last lease is removed, and release
any remaining entries at server shutdown. Do not destroy client tables
from connection teardown or keep a connection reference in each table.
For a detached lease, collect break candidates using the table's
ClientGuid directly. The existing notification retry path takes its own
references on matching live connections.
Fixes: 0626e6641f6b ("cifsd: add server handler for central processing and tranport layers")
Reported-by: Pan Guihua <pan.guihua@datatom.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
An oplock or lease break belongs to an open that can outlive its original
channel. A draining channel or stalled TCP writer can otherwise leave the
break pending until timeout or invalidate a durable open.
Snapshot the owner's live channels and try eligible connections with the
same notification. Track TCP delivery separately from the SMB ACK, retry
when a channel stops progressing, and share one break deadline. Run
ordinary break waits on a dedicated workqueue so blocked request
responses do not stall breaks.
Publish NONE before sending a level II break, which requires no ACK, so
an unsolicited ACK observes the completed level while TCP delivery is
pending.
Keep the first rename lease notification in the request context and
enqueue the interim writer only after the notification is accepted. Join
that work before returning so a blocked interim response does not stop
notification retries or release the request early.
Reported-by: Pan Guihua <pan.guihua@datatom.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
A nonblocking break send must distinguish a PDU accepted by TCP from one
still queued in ksmbd. Retrying an accepted PDU can send duplicates, while
abandoning a partial send can corrupt the stream.
Queue owned PDU bytes when TCP cannot accept the full message. Track the
stream position through socket delivery and TCP ACK, and expose it to break
retries. Bound retained data and stop queued writes during teardown.
Reported-by: Pan Guihua <pan.guihua@datatom.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
ACK handling updates lease or oplock levels and wakes break waiters.
Without state_lock across validation and publication, timeout or break
completion can observe a partially updated state.
Serialize ACK validation and state changes under state_lock. Publish NONE
after lease levels are updated, then wake waiters after unlocking.
Reported-by: Pan Guihua <pan.guihua@datatom.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
Previous-session teardown waits for requests on the old channels. Holding
srv_mutex during that wait can block their responses and prevent the drain
from completing.
Record the previous session ID during authentication, finish using the
request's authentication state, and release srv_mutex while the old
session drains. Keep the response binding decision in the work item because
signing and preauthentication hashing run after the drain.
Reported-by: Pan Guihua <pan.guihua@datatom.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
parse_sec_desc() constructs SID and DACL pointers by adding untrusted
32-bit offsets to the security descriptor before checking their bounds.
The existing checks reject offsets into the descriptor header, but do
not reject offsets beyond the descriptor's length.
On a 32-bit kernel, a large offset can wrap the pointer before the
descriptor. The end-only checks in parse_sid() and parse_dacl() can then
accept it, allowing an authenticated SMB2 SET_INFO request to access
memory outside the security descriptor and fault the kernel.
Check each nonzero offset against acl_len before constructing its
pointer. Initialize the optional owner and group SID pointers to NULL
so an omitted SID remains absent when parsing the DACL.
Fixes: 0626e6641f6b ("cifsd: add server handler for central processing and tranport layers")
Cc: stable@vger.kernel.org
Reported-by: Ratnaker Gautam <ratnaker.gautam@kuleuven.be>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
The receive completion handlers smbdirect_connection_recv_io_done(),
smbdirect_accept_negotiate_recv_done() and
smbdirect_connect_negotiate_recv_done() verify that the completion is a
regular receive with WARN_ON_ONCE(wc->opcode != IB_WC_RECV).
But wc->opcode is controlled by the remote peer: a posted receive work
request is consumed not only by an incoming SEND, but also by an RDMA
Write with Immediate, which completes as IB_WC_RECV_RDMA_WITH_IMM. An
unauthenticated peer can transmit such a message, even before the
negotiate completes, so it reaches the accept/connect negotiate
handlers as well. With panic_on_warn set this is a remote denial of
service, and otherwise it still taints the kernel and spams the log for
something a peer can trigger at will.
The opcode check itself was inherited from the original client and
server recv_done() handlers (fs/smb/client/smbdirect.c and
fs/smb/server/transport_rdma.c), where an unexpected opcode was simply
logged and the connection was torn down. Only the WARN_ON_ONCE() was
added on top during the socketization rework. Drop the WARN_ON_ONCE()
and keep the existing graceful error path, which logs the unexpected
opcode (unless the status is IB_WC_WR_FLUSH_ERR) and tears the
connection down.
Fixes: ea4151222a93 ("smb: smbdirect: introduce smbdirect_connection_recv_io_done()")
Fixes: eb3ed1e9048c ("smb: smbdirect: introduce smbdirect_accept_connect_request()")
Fixes: d1f187656797 ("smb: smbdirect: introduce smbdirect_connect[_sync]()")
Reported-by: Sashiko <sashiko-bot+sashiko@kernel.org>
Closes: https://sashiko.dev/#/bug/linux-5341a0c0-7b00-4eef-915c-91e099649d3e
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Paulo Alcantara <pc@manguebit.org>
Cc: Tom Talpey <tom@talpey.com>
Cc: linux-cifs@vger.kernel.org
Cc: samba-technical@lists.samba.org
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
smbdirect_socket_accept()
A socket on the ready list is in SMBDIRECT_SOCKET_NEGOTIATE_RUNNING,
but it may fail before smbdirect_socket_accept() picks it up, e.g.
RDMA_CM_EVENT_DISCONNECTED already moved it to
SMBDIRECT_SOCKET_DISCONNECTED.
smbdirect_socket_accept() unconditionally overwrote the status with
SMBDIRECT_SOCKET_CONNECTED and handed out the already disconnected
socket. smbdirect_socket_cleanup_work() then called rdma_disconnect()
on it again, and before the recent commit
smbdirect_socket_destroy_sync() waited forever for an
RDMA_CM_EVENT_DISCONNECTED that already happened.
Now we check first_error and change the status to
SMBDIRECT_SOCKET_CONNECTED only via cmpxchg() from
SMBDIRECT_SOCKET_NEGOTIATE_RUNNING, while we still hold listen.lock.
If the socket already failed, we move it to the orphaned list and
queue purge_orphaned_work, like smbdirect_listen_orphan_socket() does,
and try the next one. If we only found failed sockets, we wait for
the next one.
Fixes: dc691b91ad16 ("smb: smbdirect: introduce smbdirect_socket_{listen,accept}()")
Reported-by: Sashiko <sashiko-bot+sashiko@kernel.org>
Closes: https://sashiko.dev/#/patchset/cover.1791224972.git.metze%40samba.org
Closes: https://sashiko.dev/#/bug/linux-f9dbca09-30a7-4145-9bdb-4676759982c8
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Paulo Alcantara <pc@manguebit.org>
Cc: Tom Talpey <tom@talpey.com>
Cc: linux-cifs@vger.kernel.org
Cc: samba-technical@lists.samba.org
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
A child of a listener only left the listen.pending or listen.ready
list via smbdirect_socket_accept() or when the listener was
destroyed. So every connection that failed before it was accepted
(negotiate timeout, peer disconnect, invalid negotiate request, ...)
stayed on the list and counted against the backlog until the listener
was destroyed. After about backlog (10 for ksmbd) such connections
smbdirect_listen_connect_request() rejected every new connection with
-EBUSY, so an unauthenticated peer was able to permanently stop
ksmbd from accepting SMB-Direct connections.
Now a failed child moves itself to a new listen.orphaned list at the
end of smbdirect_socket_cleanup_work(), after the disconnect was
started, and queues listen.purge_orphaned_work, which releases it
with smbdirect_socket_release(), without holding the listener's
rdma_lock_handler() lock, in the same way smbdirect_socket_destroy()
does for the remaining children. Orphaned sockets still count against
the backlog until they are released.
sc->accept.listener is now only changed under listener->listen.lock
and whoever clears it is responsible for releasing the socket:
smbdirect_socket_accept(), purge_orphaned_work or the destroy of the
listener.
In order to make that work:
- a struct smbdirect_socket that is a listener is now freed via
kfree_rcu(), so that a child can dereference sc->accept.listener
under rcu_read_lock(), even if the listener is released concurrently.
- smbdirect_listen_connect_request() adds nsc to the listen.pending
list and sets nsc->accept.listener before it calls
smbdirect_accept_connect_request(), and always returns 0 so the
rdma_cm core keeps the connection id and nsc owns it. On a non-zero
return it schedules nsc's teardown with
smbdirect_socket_schedule_cleanup(), which orphans it off the
listener so that purge_orphaned_work releases it; nsc destroys its
own connection id with rdma_destroy_id() in that teardown. The
teardown has to be deferred because the rdma_cm core holds nsc's
id_priv->handler_mutex across the handler, so nsc->rdma.cm_id must
not be destroyed synchronously from here. That same handler_mutex
(and the listener's) also guarantees nsc can't go away under us,
so setting the listener before the call is safe and lets a failure
orphan nsc.
- smbdirect_accept_negotiate_recv_work() no longer moves a socket to
the ready list if it already failed or no longer belongs to the
listener. Every accepting socket has a listener, so if
sc->accept.listener is already cleared, it no longer sends a
negotiate response, as the socket is about to be released.
- smbdirect_socket_destroy() of the listener disables
purge_orphaned_work and also releases the orphaned sockets.
Fixes: dc691b91ad16 ("smb: smbdirect: introduce smbdirect_socket_{listen,accept}()")
Reported-by: Lee Seong Hyeon <ihopenre@gmail.com>
Closes: https://lore.kernel.org/linux-cifs/CANTrAmxL5sWU8Sn29JAGZvSq5PBB2OA+VKA0MHsZJMesagtfzA@mail.gmail.com/
Reported-by: Sashiko <sashiko-bot+sashiko@kernel.org>
Closes: https://sashiko.dev/#/patchset/cover.1791224972.git.metze%40samba.org
Closes: https://sashiko.dev/#/patchset/cover.1791229120.git.metze%40samba.org
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Paulo Alcantara <pc@manguebit.org>
Cc: Tom Talpey <tom@talpey.com>
Cc: linux-cifs@vger.kernel.org
Cc: samba-technical@lists.samba.org
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
smbdirect_socket_destroy_sync()
smbdirect_socket_destroy_sync() waited for RDMA_CM_EVENT_DISCONNECTED
after smbdirect_socket_cleanup_work() called rdma_disconnect(). That
can take very long, e.g. if the peer just disappeared, until the
rdma cm gives up, or forever if the event already happened, but the
status was overwritten afterwards.
This is not needed: smbdirect_socket_destroy() drains the qp under
rdma_lock_handler(), destroys it and calls rdma_destroy_id(), which
only waits for a currently running event handler and makes sure no
further events are delivered. The rest of the disconnect protocol
(DREQ/DREP and timewait for IB, abrupt close for iWarp) is handled
by the rdma core asynchronously. drivers/nvme/host/rdma.c also just
calls rdma_disconnect() and ib_drain_qp() before rdma_destroy_id().
So smbdirect_socket_destroy() now also accepts
SMBDIRECT_SOCKET_DISCONNECTING and changes the status to
SMBDIRECT_SOCKET_DISCONNECTED itself under rdma_lock_handler().
We could still get RDMA_CM_EVENT_DISCONNECTED in the small
windows between rdma_unlock_handler() and rdma_destroy_id(),
but in that case smbdirect_connection_rdma_event_handler()
is basically a no-op.
Fixes: 422a2436697d ("smb: smbdirect: introduce smbdirect_socket_destroy[_sync]()")
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Paulo Alcantara <pc@manguebit.org>
Cc: Tom Talpey <tom@talpey.com>
Cc: linux-cifs@vger.kernel.org
Cc: samba-technical@lists.samba.org
Cc: linux-rdma@vger.kernel.org
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
A write-only client can open an existing named stream with FILE_OPEN_IF
without permission to read the underlying file. The case-insensitive
lookup finds the xattr name, but querying its value length then fails
with -EACCES. smb2_set_stream_name_xattr() treats every negative result
as a missing stream and replaces the existing xattr with an empty value.
A failed xattr list is not proof that the stream is absent either:
for example, -ENOMEM can lead to the same empty replacement. Preserve
listing errors and create a stream only for -ENOENT or -ENODATA. The
latter can occur if the xattr is removed between listing its name and
querying its value. Return other lookup errors without changing the
stream.
A write-only FILE_OPEN now reports access denied instead of name not
found. FILE_OVERWRITE_IF and FILE_SUPERSEDE on an unreadable stream
also fail instead of appearing to replace it. Stream writes and EOF
changes already need to read the old value, so such handles cannot
update the stream after opening either.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Cc: stable@vger.kernel.org
Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
SMB stream names are case-insensitive, but ksmbd stores each named
stream in an xattr whose name is case-sensitive. Opening an existing
stream finds its xattr with a case-insensitive match, yet the handle
keeps the name as the client spelled it, and every later operation that
needs an exact name uses that spelling:
- a WRITE or an end-of-file change creates a second xattr, so a stream
created as "Foo" and written through "foo" ends up stored twice, and
which value a later lookup returns depends on xattr list order;
- delete-on-close and delete-pending removal can fail with -ENODATA,
or remove only a case-variant copy and leave the original;
- the share mode check compares stream names with strcmp(), so two
opens of the same stream that differ only in case do not conflict.
Have ksmbd_vfs_casexattr_len() copy the matching xattr's name into the
handle when the stream is opened, so later operations use the name
already on disk. The match must have the same length as the name it
replaces; the stream lookup passes a length that includes the
terminating NUL, which already guarantees that, and the check makes the
copy safe for any caller. A stream that does not exist yet keeps the
client's spelling, as before.
smb2_rename() compares and looks up fp->stream.name case-insensitively,
so it behaves the same with either spelling.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Cc: stable@vger.kernel.org
Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
A READ on a non-empty named stream at or beyond its end fails with
STATUS_INVALID_PARAMETER. ksmbd_vfs_stream_read() returns -EINVAL when
the offset is not below the stream length, and smb2_read() maps -EINVAL
to that status.
smb2_read() already turns a zero-byte read into STATUS_END_OF_FILE, the
status a regular file returns at its end, and an empty stream already
takes that path. Return 0 instead of -EINVAL so that non-empty streams
do the same. Clients that read a stream until end of file otherwise get
an error once they have read all of its data.
The buffered copychunk path converts a zero-byte source read to -EIO,
as it does for a regular file that shrinks during copying.
Fixes: 2ae1a6cc4302 ("cifsd: fix potential read overflow in ksmbd_vfs_stream_read()")
Cc: stable@vger.kernel.org
Signed-off-by: DaeMyung Kang <charsyam@gmail.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>
|
|
An SMB WRITE to an existing named stream can silently discard data after
its written range. ksmbd stores streams as xattr values, but the write
path passes offset + count as the length of the replacement xattr even
when the old stream is longer. Writing 26 bytes at offset 0 to a 60-byte
stream reports 26 bytes written while discarding the remaining 34 bytes.
Keep the existing xattr length when an in-place write ends before EOF.
Reject writes that cross XATTR_SIZE_MAX rather than storing only the
portion that fits while reporting the full requested count.
Commit 4ea0bb8aaedf ("ksmbd: handle set/get info file for streamed file")
intentionally skipped ordinary inode truncation for stream handles: an
EOF SetInfo on an ADS had truncated the base file. That fixed the base
file corruption but left stream EOF changes reporting success without
resizing the stream. Handle these requests by resizing only the stream
xattr; the base file size remains untouched. Zero-fill stream extensions.
This is needed alongside the write fix: a short write's accidental
truncation previously masked an ignored explicit EOF change. The stream
resize helper returns xattr errors directly rather than using the
base-file truncate path's EBADF conversion.
Fixes: f44158485826 ("cifsd: add file operations")
Fixes: 4ea0bb8aaedf ("ksmbd: handle set/get info file for streamed file")
Cc: stable@vger.kernel.org
Signed-off-by: DaeMyung Kang <charsyam@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
ksmbd_build_ntlmssp_challenge_blob() only advertises extended session
security when conn->use_spnego is true. As a result, raw NTLMSSP
negotiation drops NTLMSSP_NEGOTIATE_EXTENDED_SEC from the challenge even
when the client requested it.
A Windows capture shows an initial SPNEGO authentication succeeding,
followed by a raw NTLMSSP negotiation on the same TCP connection. The raw
challenge omits extended session security, and the client does not send
an AUTHENTICATE message. The unfinished session then occupies the pending
session slot, causing subsequent SESSION_SETUP requests to fail with
STATUS_INSUFFICIENT_RESOURCES.
Set NTLMSSP_NEGOTIATE_EXTENDED_SEC whenever the client requests it,
independently of the SPNEGO wrapper, as required by [MS-NLMP].
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Cc: stable@vger.kernel.org
Reported-by: Tea La Forgia <tealaforgia+alias@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
When a share exports "/", ksmbd_validate_name_reconnect() expects
another slash after the share path. For a file at /foo, this rejects
the valid reconnect name "foo" even though the file is in the share.
Compare the requested name with the rendered file path after its
leading slash when the resolved share root is "/". Keep the
exact-root/empty-name check and reject rendered paths without an
initial slash.
Fixes: c8efcc786146 ("ksmbd: add support for durable handles v1/v2")
Reported-by: Dairui Zhang <zhangdairui@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
A share configured as /a/link/b, with /a/link pointing to /real, is
opened relative to share->vfs_path at /real/b. However,
ksmbd_validate_name_reconnect() compares the file's d_path() result
against the original /a/link/b string and rejects a valid reconnect.
Render share->vfs_path with d_path() after the existing kern_path()
lookup and cache a copy in real_path along with its length. Use the
resolved string for durable reconnect name validation. Keep path and
path_sz as the configured spelling for existing consumers, including
the volume serial calculation and path lookups.
Fixes: c8efcc786146 ("ksmbd: add support for durable handles v1/v2")
Reported-by: Dairui Zhang <zhangdairui@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
The SMB2 oplock level is an unsigned 8-bit wire value, but
smb20_oplock_break_ack() stores it in a plain char. On architectures where
plain char is signed, the value 0xff becomes -1 and cannot compare equal to
SMB2_OPLOCK_LEVEL_LEASE (0xff). The acknowledgment is consequently rejected
with STATUS_INVALID_OPLOCK_PROTOCOL instead of the intended
STATUS_INVALID_PARAMETER.
Use u8 for the request and response oplock-level variables. This preserves
all wire values in their original representation.
Fixes: 80a56d4a826c ("ksmbd: align SMB2 oplock break ack handling")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
Handle FileIdAllExtdDirectoryInformation (0x50) in SMB2 QUERY_DIRECTORY.
Add the packed wire layout specified by MS-FSCC 2.4.21 and support
this class in entry reservation, filename lookup and response encoding.
Use the VFS inode number for the 64-bit file ID and retain the existing
Apple extension behavior of returning a zero ID. Zero the 128-bit file
ID because the VFS provides only a 64-bit inode number, as permitted by
the specification. Report reparse tags separately from the EA size, set
the reparse flag without changing other file attributes, and initialize
entry padding.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
Handle FileId64ExtdBothDirectoryInformation (0x4f) in SMB2 QUERY_DIRECTORY.
Add the packed wire layout specified by MS-FSCC 2.4.18 and support
this class in entry reservation, filename lookup and response encoding.
Use the VFS inode number for the 64-bit file ID and retain the existing
Apple extension behavior of returning a zero ID. Return an empty short
name and zero its storage and the reserved byte. Report reparse tags
separately from the EA size, set the reparse flag without changing other
file attributes, and initialize entry padding.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
Handle FileId64ExtdDirectoryInformation (0x4e) in SMB2 QUERY_DIRECTORY.
Add the packed wire layout specified by MS-FSCC 2.4.19 and support
this class in entry reservation, filename lookup and response encoding.
Use the VFS inode number for the 64-bit file ID and retain the existing
Apple extension behavior of returning a zero ID. Report reparse tags
separately from the EA size, set the reparse flag without changing other
file attributes, and initialize entry padding.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
Handle FileIdExtdDirectoryInformation (0x3c) in SMB2 QUERY_DIRECTORY.
Add the packed wire layout specified by MS-FSCC 2.4.23 and support
this class in entry reservation, filename lookup and response encoding.
Return reparse tags separately from the EA size and set the reparse flag
without changing other file attributes. Zero the 128-bit file ID because
the VFS provides only a 64-bit inode number, as permitted by the
specification, and initialize entry padding.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
Windows SMB clients can request FileIdAllExtdBothDirectoryInformation
(0x51) when enumerating a directory. ksmbd does not implement this
format, requiring the client to retry with an older information class.
Add the packed FILE_ID_ALL_EXTD_BOTH_DIR_INFORMATION wire layout from
MS-FSCC section 2.4.20 and support it throughout directory entry
reservation, name retrieval and response encoding. Include the
reparse tag separately from EaSize and retain the file attributes
when marking special files as reparse points.
Use the inode number for the 64-bit file ID, as for existing directory
classes. Set the 128-bit ID to zero because the VFS stat data does not
provide one, as required by MS-FSCC. Return no short name and zero
reserved fields and alignment padding.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
clear_and_wake_up_bit() was introduced by commit 8236b0ae31c8 ("bdi:
wake up concurrent wb_shutdown() callers.") to replace the open-coded
clear_bit_unlock() + smp_mb__after_atomic() + wake_up_bit() sequence.
Replace the sequence and drop the now-trivial wake_up_oplock_break()
wrapper, calling the helper directly at each call site.
Suggested-by: ChenXiaoSong <chenxiaosong@chenxiaosong.com>
Signed-off-by: Daijiamu <3058704216@qq.com>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
