| Age | Commit message (Collapse) | Author |
|---|
|
git://git.kernel.org/pub/scm/linux/kernel/git/linkinjeon/smb
Pull smb server fixes from Namjae Jeon:
- Fix compound CREATE response buffer sizing and reserve space for
final response padding to prevent buffer overflows and out-of-bounds
reads
- Allocate sufficient space for UTF-16 directory-name conversion,
including the terminating NUL
- Fix an RDMA transfer length underflow that can cause out-of-bounds
reads or writes when processing multiple remote buffer descriptors
- Fix use-after-free bugs during failed durable reconnect cleanup and
AppInstanceId supersede racing with session teardown
- Keep retry timeout clamp bounds constant to fix a MIPS build failure
with CONFIG_HZ=1024
- Fix a session reference leak in compounded SESSION_SETUP and reject
compounded session binding to avoid mixing session and tree state
after a failed binding
- Serialize operations on each named pipe and preserve unread RPC reply
data for subsequent READ requests when the client's output buffer is
too small
- Return complete, eight-byte-aligned stream information entries and
report insufficient output space correctly
- Use the resolved share root path so shares rooted at mount points
list their contents correctly with crossmnt disabled
- Ignore network-device events outside the initial network namespace,
preventing container interfaces from affecting ksmbd listeners
* tag 'ksmbd-for-7.4-rc1-part2' of git://git.kernel.org/pub/scm/linux/kernel/git/linkinjeon/smb:
ksmbd: return complete and aligned stream information
ksmbd: fix session reference leak on compounded SESSION_SETUP
ksmbd: fix named-pipe reply loss with small output buffers
ksmbd: avoid races between named-pipe operations
ksmbd: fix use-after-free after a failed durable reconnect
ksmbd: fix out-of-bounds read when padding the final compound response
ksmbd: fix SMB2 CREATE response buffer overflow in compound requests
ksmbd: use the resolved share path for the share root
ksmbd: keep retry timeout clamp bounds constant
smb: server: size directory name conversion buffer correctly
ksmbd: prevent session binding in compound requests
ksmbd: ignore network devices of other namespaces
ksmbd: fix app-instance non-durable supersede session UAF
smb: smbdirect: fix buffer length underflow in RDMA transfers
|
|
Pull smb client fixes from Paulo Alcantara:
- Validate the offsets and lengths a server puts in its responses
before using them, so that a malicious or broken server can no longer
make the client read past the received data:
- POSIX lock data (which leaked kernel bytes through F_GETLK)
- NT transaction responses and SMB1 EA lists (both wrapping their
pointer checks on 32-bit)
- short SESSION_SETUP responses (which sent stale buffer content
back to the server in the NTLMv2 AUTHENTICATE message)
- SMB2 READ data areas
- empty truncated fiemap responses
- Fix a heap overflow in SMB2_tcon() with UNC paths longer than
MAX_SHARENAME_LENGTH
- Fix a slab-out-of-bounds write when chmod() rewrites a security
descriptor that needs more room than was reserved
- Fix a use-after-free of the server info between the cifsd thread and
the release of the last session reference (reported by syzbot)
- Fix a use-after-free in the demultiplex thread when a receive error
reconnects and frees the read request still being processed
- Fix a double teardown of a tcon that the cached directory laundromat
took a reference on while it was being freed
- Fix a NULL pointer dereference when /proc/fs/cifs/DebugData is read
while the module is still initializing
* tag 'cifs-fixes-7.4-rc1' of https://git.manguebit.org/linux:
smb: client: validate SMB1 EA list bounds before parsing
smb: client: stop processing a read response after a receive error
smb: client: don't resurrect an exiting tcon from cached dir workers
smb: client: fix early procfs exposure race
smb: client: fix slab-out-of-bounds write in id_mode_to_cifs_acl()
smb: client: hold server info until cifsd exits
smb: client: fix fiemap wild read on empty truncated response
smb: client: validate the data area in SMB2_read()
smb: client: reject short SESSION_SETUP responses
smb: client: fix buffer overflow in SMB2_tcon()
smb: client: validate NT transaction response layout and offsets
smb: client: bound POSIX lock data area by response length
|
|
FILE_STREAM_INFORMATION responses are assembled without aligning entries
and truncated to the client buffer length after the fact. A short query
can therefore return STATUS_BUFFER_OVERFLOW with an incomplete entry and
a nonzero NextEntryOffset, which clients may reject as an invalid
network response. Standalone queries can also exhaust the server
response buffer and silently omit streams while reporting success.
Allocate the same large response buffer used by other variable-length
QUERY_INFO classes for stream queries. Pack only complete entries within
the requested output length and available response space, aligning each
entry to eight bytes and sizing names after UTF-16 conversion. Return
STATUS_BUFFER_OVERFLOW when more entries remain, using an SMB2 error
response if no entry fits.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Fixes: 6b8b79226bc3 ("ksmbd: fix partial file information responses")
Link: https://github.com/namjaejeon/ksmbd/issues/522#issuecomment-6391152923
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
A compound request can place SESSION_SETUP after another command. That
earlier command leaves a reference to its session in work->sess.
smb2_sess_setup() then overwrites work->sess with the session it looks up
or creates, and the dispatcher later puts work->sess only once. The
inherited reference is never dropped. That leaks the session, together
with its setup connection and socket, and every repeat of the compound
leaks another.
unreferenced object (size 512):
__kmalloc_cache_noprof+0x32c/0x3a0
ksmbd_smb2_session_create+0xf6/0x9c0 [ksmbd]
smb2_sess_setup+0xda4/0x6280 [ksmbd]
handle_ksmbd_work+0x88c/0x1cb0 [ksmbd]
Save the inherited session on entry. After the request has been
processed, release it once, but only when smb2_sess_setup() has replaced
work->sess with its own session or cleared it. Do this after
ksmbd_conn_unlock() so the put does not run under srv_mutex. An error path
may instead leave the inherited session in work->sess. Keep it there, so
the dispatcher still puts it and the error response is still signed or
encrypted with it as before.
Fixes: 7aa8804c0b67 ("ksmbd: fix user-after-free from session log off")
Cc: stable@vger.kernel.org
Signed-off-by: Ian Bridges <icb@fastmail.org>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
FSCTL_PIPE_TRANSCEIVE copies at most MaxOutputResponse bytes from an RPC
reply and reports STATUS_BUFFER_OVERFLOW when it does not fit. It then
frees the complete IPC reply, including the bytes that were not returned.
The client cannot recover those bytes with a subsequent READ, so an RPC
call can fail even though userspace produced a complete reply. Pipe READ
also ignores the requested Length when copying an IPC reply.
In the reproduced Windows NetShareEnum case, a 2632-byte reply is limited
to 1024 bytes of SMB output. The remaining 1608 bytes are discarded
instead of being available to the follow-up READ. Increasing the
application-level enumeration budget does not change that SMB output
limit.
Keep unread reply data on the pipe and respect each operation's output
limit. Preserve it until successfully included in a response: response
preparation failures and later input must not discard output that the
client has yet to receive. Treat a failed IPC exchange as an I/O error,
rather than reporting an empty read or acknowledging a write without
an IPC reply. Additional RPC reply queueing remains userspace's
responsibility.
Reject TRANSCEIVE with STATUS_PIPE_BUSY while cached reply data remains.
Do this before contacting userspace, without accepting new input or
consuming cached output. The client can drain the reply with READ and
explicitly resubmit the transaction. A successful positive-length READ
that leaves a suffix of the same cached reply reports
STATUS_BUFFER_OVERFLOW.
Keep cached bytes on failed requests, and consume them only after the
response is pinned successfully. Release the original reply
when drained or when its pipe is closed, logged off or disconnected.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Link: https://bugzilla.kernel.org/show_bug.cgi?id=222196
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202610260610.5prVfJss-lkp@intel.com/
Signed-off-by: Wang Zhaolong <wangzhaolong3@outlook.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
RPC IPC replies are matched by pipe handle. READ, WRITE, TRANSCEIVE and
CLOSE on the same pipe all use that handle. The session RPC lock protects
the handle table, but is dropped before a pipe I/O request waits for its
reply. Another operation, including CLOSE, can issue a request with the
same handle while that wait is still in progress.
These requests have no separate IPC correlation ID, so overlapping waits
can interfere with reply delivery. CLOSE can also release the handle
before an earlier operation finishes. Protecting table access alone does
not provide the serialization or lifetime needed by an active pipe.
Serialize operations per pipe and keep its state and handle alive until
all users finish. Avoid holding the session table lock across IPC waits,
so a blocked pipe does not hold up unrelated pipes. This also provides
the lifetime needed to retain unread replies in the following patch.
Recheck cancellation, connection release and session teardown after taking
the pipe lock, and mark a removed pipe closed before waiting for its active
operation. Close all session RPC pipes on logoff after the other requests
have drained; keeping the TCP connection open must not retain old handles.
Use standard SMB2 error replies for failed pipe operations while preserving
the selected status.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=222197
Signed-off-by: Wang Zhaolong <wangzhaolong3@outlook.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
ksmbd_reopen_durable_fd() republishes a detached durable handle in the
reconnecting session's file table and takes no reference for that entry.
If smb2_open() then fails, cleanup relies on the final ksmbd_fd_put() to
remove the volatile id.
Two reconnects of the same handle can both take a lookup reference before
either attaches it. If the one that reopens first then fails and drops
its references first, the other request makes the final put through
ksmbd_put_durable_fd(). That frees the handle without removing the first
session's table entry, so tearing that session down writes to freed
memory.
BUG: KASAN: slab-use-after-free in __close_file_table_ids+0x14d/0x740 [ksmbd]
Call Trace:
__close_file_table_ids+0x14d/0x740 [ksmbd]
ksmbd_destroy_file_table+0x76/0x100 [ksmbd]
ksmbd_session_destroy+0xf8/0x480 [ksmbd]
ksmbd_dispose_sessions+0x13e/0x200 [ksmbd]
ksmbd_conn_sessions_cleanup+0x4c3/0x6d0 [ksmbd]
Freed by task 1030:
__ksmbd_close_fd+0x97d/0x1000 [ksmbd]
smb2_open+0x14eb/0xd9f0 [ksmbd]
Commit 3515503322f4 ("ksmbd: fix durable reconnect error path file
lifetime") handles a final put made by the reconnecting request, but not
one made by another holder.
Remove the volatile id under the file-table lock on this error path,
before the references are dropped, as ksmbd_close_fd() does on the
request_open path just above. Removal then no longer depends on which
holder makes the final put. The stale comment in the reconnect cleanup
is updated to match. A successful reconnect is unchanged.
Fixes: c8efcc786146 ("ksmbd: add support for durable handles v1/v2")
Cc: stable@vger.kernel.org
Signed-off-by: Ian Bridges <icb@fastmail.org>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
CIFSSMBQAllEAs() checks the server-provided EA list length by adding it
to the response pointer. On 32-bit systems, a positive length can wrap
the pointer and pass the comparison. The parser then walks outside the
received response while reading the next FEA header.
A response with a list length of 0x40000000 reproduces this as a native
i386 page fault and kernel panic from listxattr() against an SMB1
server.
Check that the EA list starts within the SMB byte area before reading
its header. Keep its length unsigned and compare it with the bytes left
in the response without pointer addition.
SMB1 is not negotiated by default and requires an explicit vers=1.0
mount.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Ratnaker Gautam <ratnaker.gautam@kuleuven.be>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
cifs_read_iter_from_socket() may call cifs_reconnect() before returning an
error. The reconnect path aborts pending mids and invokes their callbacks.
The callback can then release the cifs_io_subrequest containing rdata.
cifs_readv_receive() nevertheless continues after a negative return and may
dereference rdata in cifs_readv_discard(). KASAN reports the resulting
UAF:
BUG: KASAN: slab-use-after-free in cifs_readv_receive
Read of size 4 by task cifsd
cifs_readv_receive (fs/smb/client/transport.c:1284)
cifs_demultiplex_thread (fs/smb/client/connect.c:1355)
kthread (kernel/kthread.c:436)
ret_from_fork (arch/x86/kernel/process.c:158)
Freed by task 148:
netfs_put_subrequest (fs/netfs/objects.c:267)
netfs_read_collection (fs/netfs/read_collect.c:460)
netfs_wait_for_in_progress (fs/netfs/misc.c:462)
Return the socket error immediately, as this path did before the iterator
conversion. This also avoids adding a negative length to total_read.
Cc: stable@vger.kernel.org
Fixes: d08089f649a0 ("cifs: Change the I/O paths to use an iterator rather than a page list")
Reported-by: co+24c7c12d69e63c6d@bugs.sh
Assisted-by: LLM
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
cfids_laundromat_worker() bumps tcon->tc_count for every expired open
cfid and queues cached_dir_offload_close() without checking whether the
tcon is already being torn down. Racing with the final cifs_put_tcon(),
the tcon comes back from zero and gets torn down twice:
T1 T2
cifs_put_tcon()
--tc_count == 0, TID_EXITING
tconInfoFree()
free_cached_dirs()
cancel_delayed_work_sync() cfids_laundromat_worker()
++tc_count == 1
queue_work(serverclose_wq)
kfree(tcon->origin_fullpath)
kfree(tcon)
cached_dir_offload_close()
close_cached_dir() [UAF]
cifs_put_tcon()
tconInfoFree() [double free]
cached_dir_lease_break() also bumps tc_count without tc_lock, racing with
the laundromat's locked update.
Take the tcon reference through a helper that refuses once the tcon is
TID_EXITING or its refcount has dropped to zero. Queued put_work and
close_work then always hold a tcon reference, so tconInfoFree() can't
run before they finish and free_cached_dirs() doesn't need to flush any
workqueue. If the laundromat can't get a reference, put the cfid back on
the dying list for free_cached_dirs() to free; its handle goes away with
the tree connection.
The lease break path finds the tcon on ses->tcon_list under
cifs_tcp_ses_lock, so it can't see TID_EXITING, but it now takes tc_lock.
Fixes: 3fa640d035e5 ("smb: During unmount, ensure all cached dir instances drop their dentry")
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Cc: David Howells <dhowells@redhat.com>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
Cc: stable@vger.kernel.org
|
|
Compound response handling pads the last response of a compound to an
8-byte boundary by growing the final iov and the RFC1002 length.
smb2_allocate_rsp_buf() allocates exactly response_sz. A command can
fill the response buffer to within 7 bytes of its end, for example a
QUERY_INFO whose output is bounded only by the space left. The padding
can then run past the allocation, and ksmbd_tcp_writev() reads those
bytes when it sends the response.
BUG: KASAN: slab-out-of-bounds in _copy_from_iter+0x744/0x10e0
Call Trace:
_copy_from_iter+0x744/0x10e0
tcp_sendmsg_locked+0x2290/0x3f80
ksmbd_tcp_writev+0x10c7/0x1ab0 [ksmbd]
Allocate room for the padding when the request is a compound, as
smb2_read() and smb2_read_pipe() do for their payloads. response_sz is
unchanged, so commands still fill only up to it, and kvzalloc() zeroes
the padding.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Cc: stable@vger.kernel.org
Signed-off-by: Ian Bridges <icb@fastmail.org>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
A CREATE response that carries an AAPL create context can reach
create_rsp_size (572 bytes for SMB 3.1.1). Commit 713c31a3a225
("ksmbd: fix SMB2 CREATE response buffer overflow") makes
smb2_allocate_rsp_buf() reserve that much, but only for the first
command of a compound. A CREATE later in the compound is guarded by
is_chained_smb2_message() instead, which reserves only
MAX_CIFS_SMALL_BUFFER_SIZE (448 bytes) for the next response. smb2_open()
then appends the create contexts without checking the space left.
That check also uses the unpadded length of the response so far.
init_chained_smb2_rsp() pads the previous response to 8 bytes before the
next command runs, so the next response can start up to 7 bytes later
than the check assumes.
When a CREATE follows a command that fills the response buffer, such as
QUERY_DIRECTORY, it writes its create contexts past the end of the
buffer. The AAPL context is written last.
BUG: KASAN: slab-out-of-bounds in create_aapl_rsp_buf+0x31/0x6e0 [ksmbd]
Call Trace:
create_aapl_rsp_buf+0x31/0x6e0 [ksmbd]
smb2_open+0x4fa8/0xd900 [ksmbd]
handle_ksmbd_work+0x88c/0x1cb0 [ksmbd]
When the next command is a CREATE, reserve create_rsp_size, as
smb2_allocate_rsp_buf() does for a single CREATE, and check the reserve
against the 8-byte aligned offset where the next response starts.
Fixes: 8f1b796ff113 ("ksmbd: add AAPL kAAPL_SERVER_QUERY create context support")
Signed-off-by: Ian Bridges <icb@fastmail.org>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
If the share path is a mount point and crossmnt is not set (crossmnt = no,
or ksmbd-tools that predate the option), the share root looks empty to
clients: QUERY_DIRECTORY on it returns only "." and "..". Files created
in the root go to the mounted filesystem, but a directory query cannot
find them afterwards, so e.g. "rm" of a file that was just uploaded
fails with STATUS_NO_SUCH_FILE.
An empty name refers to the share root. For it, ksmbd_vfs_path_lookup()
looks up share_conf->path again starting from its parent, and the mount
on the last component is not followed, which gives the directory covered
by the mount. All other names are looked up relative to
share_conf->vfs_path, which was resolved through the mount when the
share was set up.
share_conf->vfs_path already refers to the share root, so return a
reference to it instead of looking the path up again.
Fixes: 74d7970febf7 ("ksmbd: fix racy issue from using ->d_parent and ->d_name")
Signed-off-by: Denis Kulikov <dar1knessx@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
With CONFIG_HZ=1024, msecs_to_jiffies(1200) includes an overflow check
that calls jiffies_to_msecs(MAX_JIFFY_OFFSET). Its saturation branch
returns MAX_JIFFY_OFFSET, which exceeds the six-second upper bound.
GCC 10.5 carries that branch into clamp_t()'s bounds check and fails to
build ksmbd_tcp_writev() on MIPS.
Compute the 1200 ms lower bound with DIV_ROUND_UP() instead, so both
bounds are compile-time constants. The lower bound still rounds up to
the next jiffy, and the retry interval remains between 1.2 and 6 seconds.
Fixes: c0632d2010ff ("ksmbd: track delivery of queued TCP writes")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202610251743.jztr17Ib-lkp@intel.com/
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
A 2048-byte ASCII directory name needs 4098 bytes when converted to
UTF-16, including the terminator. ksmbd_convert_dir_info_name() caps
its allocation at PATH_MAX and writes past the end of that buffer.
Allocate space for one UTF-16 code unit per input byte plus the
terminator. Use kmalloc_array() to check the size calculation.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Cc: stable@vger.kernel.org
Assisted-by: gpt 6 sol with daybreak blue
Signed-off-by: VeroFess <coderzeng@hotmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
A failed multichannel binding can retain its target session on the work
item so that the error response can be signed. Subsequent compound
commands can then use that session together with a tree connection left
over from an earlier command.
Limit binding SESSION_SETUP requests to standalone messages during
message validation, before smb2_sess_setup() can replace work->sess.
Check both NextCommand and the current request offset to cover binding
in the first, middle and last positions of a compound. The existing
validation error path returns STATUS_INVALID_PARAMETER and stops
processing the remaining commands.
This intentionally limits support for compounded binding. It is not a
protocol requirement that binding be standalone. Standalone binding and
non-binding compounds retain their existing behavior, without changing
session, tree or response encryption handling.
Fixes: 4b706360ffb7 ("ksmbd: fix multichannel binding and enforce channel limit")
Cc: stable@vger.kernel.org
Reported-by: Zeng Adolph <coderzeng@hotmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
With no interface list configured, ksmbd binds every interface that
comes up. ksmbd_netdev_event() does not look at the device's namespace,
and create_socket() opens the listener in the namespace of the task
raising NETDEV_UP. A container with its own address on a bridge brings
up its interface and finds port 445 in its namespace already taken by
ksmbd, so an SMB server running inside the container fails with
"Address already in use".
Interfaces are also matched by name only, so a container's eth0 going
down would stop the listener on the host's eth0.
Only handle netdev events for devices in init_net.
Fixes: 0626e6641f6b ("cifsd: add server handler for central processing and tranport layers")
Cc: stable@vger.kernel.org
Signed-off-by: Jan Koester <j.koester@act-marketing.de>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
ksmbd_close_fd_app_instance_id() supersedes a prior open that has the
same AppInstanceId. It reaches that open's session through opinfo->sess.
This pointer is not refcounted.
Session teardown detaches durable handles before it frees the session.
It does not detach a non-durable open. That open still points at the
session through opinfo->sess after the session is freed. Another session
can supersede the open at the same time. The supersede then takes a
write lock through the stale pointer, in the freed session's file table.
BUG: KASAN: slab-use-after-free in _raw_write_lock+0x71/0xd0
Call Trace:
_raw_write_lock+0x71/0xd0
ksmbd_close_fd_app_instance_id+0xd9/0x670 [ksmbd]
smb2_open+0x3e10/0xd910 [ksmbd]
Freed by task 212:
ksmbd_session_destroy+0x38d/0x480 [ksmbd]
Fix this by looking the session up by opinfo->session_id. The lookup
returns a reference, as smb2_break_collect_channels() does. Recheck that
opinfo->sess still points at it before using it.
Do the lookup without m_lock held. destroy_previous_session() takes
m_lock under the session table lock, and taking them in the other order
here would deadlock.
Fixes: 16c30649709d ("ksmbd: handle durable v2 app instance id")
Cc: stable@vger.kernel.org # e0386cd6498b: ksmbd: validate AppInstanceId takeover target
Signed-off-by: Ian Bridges <icb@fastmail.org>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
When a remote buffer descriptor is larger than the remaining local
buffer, smbdirect_connection_rdma_xmit() clamps its length and sets
buf_len to zero. The common accounting code then subtracts the clamped
length from buf_len, causing it to underflow. If another descriptor
follows, the loop processes it with a wrapped remaining length and
includes memory beyond the local buffer in the RDMA transfer.
A client can trigger an out-of-bounds kernel write with an SMB WRITE
request, or an out-of-bounds kernel read with an SMB READ request, by
supplying multiple RDMA buffer descriptors with an oversized descriptor
before the last one.
Remove the premature assignment to buf_len. Subtracting the clamped length
already consumes the remaining buffer, so the next iteration stops and
only descriptors within the local buffer are used to build work requests.
Fixes: e628bf939aaf ("ksmbd: reduce descriptor size if remaining bytes is less than request size")
Cc: stable@vger.kernel.org
Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-33549
Signed-off-by: Stefan Metzmacher <metze@samba.org>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/linkinjeon/smb
Pull smb server updates from Namjae Jeon:
- Add extended SMB2 directory information support.
Implement FileIdExtdDirectoryInformation, FileId64ExtdDirectoryInformation,
FileId64ExtdBothDirectoryInformation, FileIdAllExtdDirectoryInformation,
and FileIdAllExtdBothDirectoryInformation, including reparse tags, file
attributes, and initialized response padding.
- Track unsigned 64-bit lock ranges in the server lock index, with
conflict detection, blocking waits, cancellation, rollback, and
cleanup for ranges beyond the VFS signed-offset limit.
- Add configurable per-connection session limits.
Reserve slots for new sessions and multichannel bindings, release
them during failure and cleanup, and expose the configured limit
through procfs.
- Validate POSIX negotiate and CREATE contexts, including payload
lengths, extension tags, duplicate contexts, dialect
requirements, and share eligibility. Require POSIX opens for
POSIX file, directory, and filesystem information queries.
- Apply requested POSIX create modes, including mode zero, and NFS
mode ACEs with special permission bits. Enforce append-only
writes and base byte-range lock checks on each open's POSIX
state.
- Complete POSIX metadata responses with filenames, file types,
permission bits, and filesystem device identifiers. Query
filesystem information through the requested open and report the
filesystem fragment size and response length.
- Validate SMB3 encryption transform flags and ensure decrypted
compound requests use the session authenticated by the transform,
including requests containing compressed payloads.
- Serialize negotiation with receive processing, size CREATE
responses for their contexts, reserve UTF-16 terminator space in
stream information, and validate WRITE data against the active
compound request to prevent buffer overruns and stale-data
writes.
- Validate security descriptor offsets before pointer arithmetic,
handle DACL offset overflow on 32-bit systems, and propagate
inode ACL lookup failures safely.
- Deny access through empty DACLs while retaining the owner's
implicit security-management rights. Add KUnit regression
coverage for DACL walk boundaries and maximal-access
calculations.
- Validate RPC pipe ownership for IPC$ reads and writes, and fix
FSCTL_PIPE_TRANSCEIVE handle lookup and response file IDs.
- Fix durable reconnect path validation for symlinked share paths,
shares exporting "/", and handles opened at the share root. Check
path boundaries before comparisons to prevent out-of-bounds reads
and cross-share confusion.
- Rebind completed durable CREATE replays to surviving channels
without draining unrelated requests on the old channel. Transfer
open accounting with the binding and avoid consuming additional
open-file budget during durable reconnects.
- Scope AppInstanceId takeovers to the matching share, path,
stream, and client identity. Check access and AppInstanceVersion
before closing an existing open.
- Retry oplock and lease break notifications through other live
channels, track queued TCP delivery, serialize acknowledgment
updates, and expire unacknowledged breaks while waking blocked
requests.
- Keep lease tables and byte-range lock indexes independent of
their original connection so surviving channels and detached
durable opens retain access to lease and lock state.
- Reclaim ended sessions after logoff and expiration, hold
connection references for session channels, and drain procfs
readers before releasing session table references. Avoid
deadlocks during previous-session teardown and connection
scavenging.
- Prevent file publication after tree disconnect and retain tree
references until final file release. Take parent dentry
references during directory enumeration and serialize directory
emptiness checks with readdir.
- Prevent share-root removal through empty removal paths and return
appropriate errors for directory zero-data requests and temporary
directory attributes.
- Fix named-stream writes, EOF changes, end-of-file reads, and
content copying into rename targets. Reuse existing xattr names,
report sizes by exact xattr name, and avoid emptying streams
after lookup or permission failures.
- Round allocation requests to the advertised allocation unit and
avoid allocation changes that leave on-disk EOF inconsistent with
the in-memory size.
- Inherit FileIds across related compound requests, including
CHANGE_NOTIFY, and use the existing async request machinery for
notify waiting and cancellation.
- Restore share configuration and Kerberos authentication
compatibility with released ksmbd-tools. Advertise extended
session security for raw NTLMSSP when requested by the client.
- Improve Windows chained-compression compatibility, synthesized
AFP_AfpInfo headers, generated short names, and UTF-16 alignment
in partial normalized-name responses.
- Fix interface-query locking, response chaining, link-speed
reporting, and independent IPv4/IPv6 address enumeration. Display
native IPv6 client addresses accurately in procfs.
- Improve SMB Direct socket cleanup by reclaiming failed pending
connections, skipping failed sockets during accept, avoiding
indefinite disconnect waits, and handling unexpected receive
opcodes without triggering kernel warnings.
- Select CRYPTO_AES for SMB3 encryption, replace deprecated system
workqueues, and update feature status documentation and POSIX
specification references.
* tag 'ksmbd-for-7.4-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/linkinjeon/smb: (95 commits)
ksmbd: select CRYPTO_AES
ksmbd: prevent share root removal through empty paths
ksmbd: serialize directory emptiness checks with readdir
ksmbd: fix parent dentry lifetime in directory enumeration
smb/server: prevent file publication after tree disconnect
ksmbd: restore Kerberos logins with released ksmbd-tools
smb/server: replace deprecated system workqueues
ksmbd: reject FILE_ATTRIBUTE_TEMPORARY on directories
ksmbd: deny access for an empty DACL instead of granting the request
ksmbd: reject foreign RPC pipe handles on IPC$ read and write
ksmbd: fix fd_limit accounting across durable reconnects
ksmbd: drain session procfs readers before dropping table references
ksmbd: reclaim ended sessions on logoff and expiration
ksmbd: limit sessions on each connection
ksmbd: hold a connection reference for each session channel
ksmbd: defer connection scavenging while session locks are busy
ksmbd: accept share config responses from ksmbd-tools before 3.5.3
ksmbd: round SetAllocation to the advertised allocation unit
ksmbd: avoid restoring only in-memory EOF after allocation change
ksmbd: inherit FileIds in related compound requests
...
|
|
Currently, init_cifs() calls cifs_proc_init() near the beginning of
the function, before INIT_LIST_HEAD, leading to a momentarily
uninitialized, but exposed, procfs. If a userspace process reads this
empty procfs, iterating over a zeroed list_head leads to a NULL
pointer dereference and a kernel oops/panic.
For example, if a delay is added between cifs_proc_init() and
INIT_LIST_HEAD, and a userspace process is polling
/proc/fs/cifs/DebugData, the following oops occurs:
BUG: kernel NULL pointer dereference, address: 0000000000000608
Oops: Oops: 0000 [#1] SMP PTI
CPU: 0 UID: 0 PID: 3477 Comm: cat Not tainted
7.3.0-02212-g4df4d2a577db-dirty #6 PREEMPT(lazy)
RIP: 0010:cifs_debug_data_proc_show+0x103/0xd80 [cifs]
Call Trace:
<TASK>
? seq_read_iter+0x86/0x400
? seq_read_iter+0x86/0x400
seq_read_iter+0x183/0x400
proc_reg_read_iter+0x62/0x90
copy_splice_read+0x215/0x310
splice_file_to_pipe+0xec/0x1c0
do_splice+0x4b8/0x780
__se_sys_splice+0x18f/0x250
do_syscall_64+0x109/0x330
entry_SYSCALL_64_after_hwframe+0x77/0x7f
</TASK>
Fix this by moving cifs_proc_init() down in init_cifs(), ensuring it runs
only after all internal lists, locks, counters, and workqueues are fully
ready, but before registering the filesystem. Update the error cleanup
ladder and exit_cifs() to maintain proper LIFO order.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Fredric Cover <fredric.cover.lkernel@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
In id_mode_to_cifs_acl(), while the chown branch reserves
sizeof(struct smb_ntsd) + 2 * sizeof(struct smb_sid) (plus
sizeof(struct smb_acl) and the worst-case ACE space after
commit 0ee150794c75 ("smb: client: fix heap overflow in DACL owner/group
rewrite")), the chmod branch only adds 2 * sizeof(struct smb_ace) or
5 * sizeof(struct smb_ace) to secdesclen (or 1 * sizeof(struct smb_ace)
on posix).
However, the chmod branch also invokes build_sec_desc() ->
copy_sec_desc(), which unconditionally writes the Owner SID at
pnntsd + sidsoffset and the Group SID at
pnntsd + sidsoffset + sizeof(struct smb_sid) (2 * 68 = 136 bytes total),
and set_chmod_dacl() always writes a struct smb_acl header (8 bytes)
even when the source descriptor has dacloffset == 0. When a security
descriptor has compact or overlapping Owner/Group SIDs (or
dacloffset == 0), calling chmod() under-allocates pnntsd and writes past
the end of the slab buffer.
Reserve sizeof(struct smb_acl) + 2 * sizeof(struct smb_sid) (plus
sizeof(struct smb_ntsd) on the posix path) in the chmod branch of
id_mode_to_cifs_acl().
Fixes: 7ee0b4c635c0 ("cifs: fix hardcoded default security descriptor length")
Fixes: f5065508897a ("cifs: Retain old ACEs when converting between mode bits and ACL.")
Assisted-by: ARTIST LLM
Signed-off-by: Dominik Maier <dmnk@google.com>
Cc: stable@vger.kernel.org
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
The last session reference can be dropped while the cifsd demultiplex
thread is still running clean_demultiplex_info(). That thread frees
TCP_Server_Info there, racing with cifs_put_tcp_session() as it releases
the server crypto state. syzbot reported a KASAN use-after-free in
cifs_crypto_secmech_release().
Keep an object reference for the cifsd thread. Defer final crypto,
session-key, network namespace, string, and structure cleanup until both
the last session reference and the thread reference are dropped. Drop the
thread reference after clean_demultiplex_info() has finished using the
server.
Build-tested with ARCH=arm64 and W=1 for
fs/smb/client/connect.o. The KASAN reproducer was not run on the patched
kernel.
Fixes: a5c3e1c725af ("Revert "cifs: No need to send SIGKILL to demux_thread during umount"")
Reported-by: syzbot+5c3ed2a0de3270efc921@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=5c3ed2a0de3270efc921
Reported-by: syzbot+c2ec61adfbcb24e5ec20@syzkaller.appspotmail.com
Closes: https://syzbot.org/bug?extid=c2ec61adfbcb24e5ec20
Assisted-by: LLM OpenAI Codex
Signed-off-by: Eduard Zateev <hackerowskiy@gmail.com>
Cc: stable@vger.kernel.org
Tested-by: Michail Andreas Michalopoulos <mike.michalo@gmail.com>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
smb3_fiemap() treats -E2BIG from SMB2_ioctl() as a truncated range blob
and loops for the next chunk. If the server reports
STATUS_BUFFER_OVERFLOW with OutputCount == 0, SMB2_ioctl() returns
-E2BIG with out_data == NULL and out_data_len == 0, and the existing
length checks accept the empty result. The loop then evaluates
next = out_data[num - 1].file_offset + ...
with num == 0, reading 16 bytes before NULL.
Reject a truncated range blob that contains no range, so the resume
offset is never computed from an empty response. Also require the last
range in a truncated blob to advance the resume offset without
overflowing, and to stay within the requested range; a malicious or
broken server could otherwise make no progress or underflow len and loop
forever. Free the previous response buffer before retrying.
Reproduced on 6.6.87.2 with a malicious SMB2 server that answers
FSCTL_QUERY_ALLOCATED_RANGES with STATUS_BUFFER_OVERFLOW and
OutputCount == 0; an FS_IOC_FIEMAP ioctl on the mount oopses in
cifs_fiemap() with CR2 = fffffffffffffff8. The code path is unchanged
on mainline.
Fixes: 2f3ebaba13ce ("cifs: add fiemap support")
Cc: stable@vger.kernel.org
Signed-off-by: Abdifatah Suruur <suruurism@gmail.com>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
SMB2_read() copies le32_to_cpu(rsp->DataLength) bytes from
(char *)rsp + rsp->DataOffset without checking that the range lies
within the response actually received. A READ response with
DataOffset == 0 makes smb2_get_data_area_len() zero the data length, so
smb2_check_message() accepts a frame as short as the fixed response,
while SMB2_read() still uses the server-supplied DataLength, capped only
by CIFS_MAX_MSGSIZE and the caller's length.
A malicious server can answer the synchronous read used by the
mfsymlinks (or SFU) symlink parser with an 81-byte READ response
carrying DataOffset 0 and DataLength 1067, making the client copy 1067
bytes from a 448-byte receive object (KASAN slab-out-of-bounds; up to
16 KiB in the general case).
Clamp a zero (or header-overlapping) DataOffset to read_rsp_size, as
handle_read_data() does for the asynchronous path, and reject the
response unless DataOffset + DataLength fits within rsp_iov.iov_len.
The SMB1 helper was fixed the same way in 5be5bdda5863.
Fixes: d8e050398d23 ("CIFS: Add readpage support for SMB2")
Cc: stable@vger.kernel.org
Signed-off-by: Abdifatah Suruur <suruurism@gmail.com>
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
smb2_check_message() accepts frames shorter than struct smb2_pdu when
the SMB2 status is non-zero, before the minimum-PDU check added by
b4694f269e66d. A 64-byte STATUS_MORE_PROCESSING_REQUIRED SESSION_SETUP
response is therefore accepted even though it cannot contain the
SecurityBufferOffset and SecurityBufferLength fields.
The parser then reads those fields from stale bytes left in the reused
receive buffer by an earlier frame. With a long stale
SecurityBufferLength, decode_ntlmssp_challenge() copies up to about
64 KB from beyond the receive object and sends the data back to the
server in the NTLMv2 AUTHENTICATE target info.
Reject short MPR SESSION_SETUP responses before the short-error path,
and validate SecurityBufferOffset + SecurityBufferLength against the
received response length before decoding the challenge.
The ordering bypass is not new: the short-error early return predates
b4694f269e66d, and the unvalidated SecurityBufferLength read predates
the 2016 raw NTLMSSP split in 166cea4dc3a4, so all supported stable
trees are affected. Reproduced on 6.6.87.2; the path is unchanged on
mainline.
Fixes: 5478f9ba9a34 ("CIFS: Add session setup/logoff capability for SMB2")
Cc: stable@vger.kernel.org
Signed-off-by: Abdifatah Suruur <suruurism@gmail.com>
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
ksmbd allocates "gcm(aes)" and "ccm(aes)" AEAD transforms for SMB3
encryption, but SMB_SERVER only selects CRYPTO_GCM and CRYPTO_CCM, not
an AES cipher. With a configuration in which nothing else enables
CRYPTO_AES, ksmbd builds but cannot allocate the transforms, and every
session that uses encryption fails:
ksmbd: Failed to alloc encrypt aead : -2
ksmbd: crypto alloc failed
Select CRYPTO_AES, as the SMB client already does.
Fixes: a848c4f15ab6 ("cifsd: add Kconfig and Makefile")
Assisted-by: LLM
Signed-off-by: Michail Andreas Michalopoulos <mike.michalo@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
A FileLinkInformation request can supply "/" as the new link name.
smb2_get_name() strips the trailing slash and passes an empty string to
ksmbd_vfs_kern_path_start_removing().
ksmbd_vfs_path_lookup() resolves an empty pathname as the configured share
path, without LOOKUP_BENEATH. With ReplaceIfExists set, smb2_create_link()
can therefore remove the share root from its parent before link creation
fails. An authenticated client with a writable share and DELETE access to
a file can trigger this if the share root is empty and the mapped user
has write and search permission on its parent directory.
Return -EINVAL for empty pathnames when for_remove is set, before
resolving the share path or acquiring removal resources. Ordinary lookups
continue to accept empty pathnames for opening the share root.
Fixes: 74d7970febf7 ("ksmbd: fix racy issue from using ->d_parent and ->d_name")
Cc: stable@vger.kernel.org
Reported-by: Zachariah Mithani (AploSec) <zachariah@aplosai.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
SMB2 QUERY_DIRECTORY holds fp->readdir_lock while it calls iterate_dir()
on fp->filp. An SMB2 SET_INFO request with FILE_DISPOSITION_INFORMATION
can resolve the same FileId and call ksmbd_vfs_empty_dir() on the same
filp without that lock.
iterate_dir()'s inode read lock permits concurrent readers and does not
serialize per-file iterator state. ext4 stores its dx readdir cursor and
fname RB tree in file->private_data. Concurrent reset or refill can
therefore free a fname node while the other worker still traverses it or
retains its RB-tree link, resulting in use-after-free accesses.
An authenticated SMB client with list and delete rights on a directory in
a writable KSMBD share backed by ext4 can trigger this when the directory
takes the dx readdir path. The client overlaps an SMB2 QUERY_DIRECTORY
request with an SMB2 SET_INFO request carrying FILE_DISPOSITION_INFORMATION
on one live FileId.
In one x86_64 SMP KASAN multi-shot guest using ksmbd-for-next at
171df27077fb, the unmodified kernel reported a cross-worker use-after-free
before a panic. The first report's access and free stacks show the two
request paths directly (trimmed):
BUG: KASAN: slab-use-after-free in ext4_htree_store_dirent+0x488/0x550
Read of size 8 by task kworker/2:0/29
Workqueue: ksmbd-io handle_ksmbd_work
Call Trace:
ext4_htree_store_dirent+0x488/0x550
htree_dirblock_to_tree+0x535/0x990
ext4_htree_fill_tree+0x319/0xba0
ext4_readdir+0x175a/0x2f40
iterate_dir+0x1b6/0x630
smb2_query_dir+0x1003/0x1cc0
Freed by task 279:
free_rb_tree_fname+0x67/0xc0
ext4_readdir+0x13a9/0x2f40
iterate_dir+0x1b6/0x630
ksmbd_vfs_empty_dir+0xa2/0x100
smb2_set_info+0x2ffa/0x45d0
Take fp->readdir_lock across the entire emptiness check. This places both
iterators over that handle's filp in the same lock domain while
preserving the disposition status handling. The exact code change
completed a warning-free x86_64 KASAN bzImage build. Tracing three request
pairs showed that the two iterate_dir() calls no longer overlapped and the
expected statuses were preserved. A further 1,600 reproducer iterations
completed with expected statuses and no KASAN report, Oops, or panic.
The reproducer and complete guest configuration are available privately
to maintainers on request.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Yongjun Gwon <d3bugee@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
ksmbd_populate_dot_dotdot_entries() builds the ".." entry from
the open directory's d_parent and passes that pointer to
ksmbd_vfs_fill_dentry_attrs() without taking a reference. That pointer
is only valid while the directory stays under this parent. If a
concurrent rename moves the directory elsewhere, the old parent loses
its last reference; a following rmdir then turns it into a negative
dentry with d_inode cleared, and after the RCU grace period the dentry
itself is freed. The worker still holds the raw pointer and
dereferences d_inode through vfs_getattr() and vfs_getxattr().
A remote SMB client, authenticated or guest, triggers this by listing
a directory while that directory is moved under a new parent and the
old parent is deleted. It can be done by the client itself with write
access or by other concurrent writers, triggering null-ptr-deref and
slab-use-after-free on a KASAN build.
KASAN: null-ptr-deref in range
[0x0000000000000000-0x0000000000000007]
Workqueue: ksmbd-io handle_ksmbd_work
RIP: 0010:xattr_permission+0x9d/0x160
Call Trace:
vfs_getxattr+0x9c/0x1b0
ksmbd_vfs_getxattr+0x82/0x100
ksmbd_vfs_get_dos_attrib_xattr+0x7f/0x130
ksmbd_vfs_fill_dentry_attrs+0x435/0x650
ksmbd_populate_dot_dotdot_entries+0x25e/0x630
smb2_query_dir+0xcf9/0x1a20
handle_ksmbd_work+0x5f1/0x1c80
process_one_work+0x90b/0x19d0
Fix by using the dentry reference counting APIs: take the parent with
dget_parent(), take a matching dget() on ".", and dput() the dentry
once ksmbd_vfs_fill_dentry_attrs() has returned or the pattern does
not match.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Cc: stable@vger.kernel.org
Cc: AutonomousCodeSecurity@microsoft.com
Signed-off-by: Cen Zhang (Microsoft) <cenzhang@linux.microsoft.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
TREE_DISCONNECT closes the tree's published file handles before marking and
removing the tree connection. A concurrent CREATE that already holds a tree
reference can therefore publish its file after the close pass. When CREATE
drops its reference, the tree can be freed while the published file still
points to it.
Mark and remove the tree connection before closing its files, and serialize
both new-open and durable-reconnect volatile ID publication with that state
transition. An opener is then either published in time for the close pass
or rejected after disconnect with STATUS_NETWORK_NAME_DELETED.
Keep a tree reference while a file carries its tree pointer. This protects
asynchronous users that can defer the file's final destruction past tree
disconnect.
The close pass can race the successful FP_NEW to FP_INITED transition.
Take the opening request's file reference atomically with that transition.
This keeps the file alive until request completion even if teardown removes
the ID immediately.
BUG: KASAN: slab-use-after-free in proc_show_files (fs/smb/server/vfs_cache.c:126)
Read of size 4 at addr ff11000101540f80 by task poc/153
Call Trace:
proc_show_files (fs/smb/server/vfs_cache.c:126)
seq_read_iter (fs/seq_file.c:231)
proc_reg_read_iter (fs/proc/inode.c:299)
vfs_read (fs/read_write.c:574)
ksys_read (fs/read_write.c:716)
do_syscall_64 (arch/x86/entry/syscall_64.c:84)
entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121)
Allocated by:
ksmbd_tree_conn_connect (fs/smb/server/mgmt/tree_connect.c:35)
smb2_tree_connect (fs/smb/server/smb2pdu.c:2819)
handle_ksmbd_work (fs/smb/server/server.c:368)
Freed by:
kfree (mm/slub.c:6801)
handle_ksmbd_work (fs/smb/server/server.c:332)
Fixes: f44158485826 ("cifsd: add file operations")
Cc: stable@vger.kernel.org
Reported-by: <co+d531129f2f9fbf88@bugs.sh>
Assisted-by: LLM
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
Commit a47634cd729b ("ksmbd: expire SMB sessions when Kerberos tickets
expire") added a __u64 session_expiry field to struct
ksmbd_spnego_authen_response, growing it from 156 to 168 bytes.
ipc_validate_msg() requires the response to have exactly that size, so
the 156-byte response of every released ksmbd-tools, up to 3.5.7, is
dropped and Kerberos session setup fails with STATUS_LOGON_FAILURE.
The other way round, kernels without that commit, including the stable
trees, drop the 168-byte response of current ksmbd-tools master, and
ksmbd-tools has no way to tell which layout a kernel expects.
Pass the expiry in the reserved area of the embedded struct
ksmbd_login_response instead. ksmbd-tools releases with that area
(3.4.4 and later) allocate the response zeroed and never write to it,
so they send an expiry of 0, which ksmbd already treats as no expiry.
The response is 156 bytes again, so it works with those releases, and
every kernel also accepts a response that sets the expiry. Use two
__u32 halves, as the reserved area is only 4-byte aligned.
Fixes: a47634cd729b ("ksmbd: expire SMB sessions when Kerberos tickets expire")
Cc: stable@vger.kernel.org
Closes: https://lore.kernel.org/all/CAO+rqLhpPvY0NLrAnZjFXz0gOXnA6zsXxK8xCgtA42XUkSsAeg@mail.gmail.com/
Assisted-by: LLM
Signed-off-by: Michail Andreas Michalopoulos <mike.michalo@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
In SMB2_tcon(), unc_path is allocated with a fixed buffer size of
MAX_SHARENAME_LENGTH * 2 bytes (672 bytes). However, the user-controlled
UNC path (tree) length is passed to cifs_strtoUTF16(), which assumes the
destination buffer is large enough and writes strlen(tree) + 1 UTF-16 code
units.
Commit 62a1a439e0fd ("cifs: clean up handling of unc= option") removed the
length check on UNC mount options, allowing mount paths to exceed
MAX_SHARENAME_LENGTH while retaining the fixed-size buffer allocation in
SMB2_tcon(). Consequently, an oversized UNC string causes cifs_strtoUTF16()
to write past the end of the heap object, resulting in a slab-out-of-bounds
write. Furthermore, if a tree exceeds MAX_TREE_SIZE characters, strscpy()
truncates it in cifs_tcon::tree_name, causing subsequent reconnects or
failovers to fail.
Fix this by first verifying that the length of tree does not exceed
MAX_TREE_SIZE, returning -EINVAL if it does. Then, dynamically allocate
unc_path based on the length of tree so that it safely accommodates all
UTF-16 code units for paths within MAX_TREE_SIZE.
Fixes: 62a1a439e0fd ("cifs: clean up handling of unc= option")
Cc: stable@vger.kernel.org
Reported-by: Xin Liu <bird@lzu.edu.cn>
Closes: https://bugtracker.nebusec.ai/f/3292
Assisted-by: LLM
Suggested-by: Paulo Alcantara <pc@manguebit.org>
Signed-off-by: Luxiao Xu <rakukuip@gmail.com>
Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
[pc: remove unused MAX_SHARENAME_LENGTH]
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
validate_ntransact() assumes that the response uses the fixed 18-word NT
transaction layout. A different WordCount makes get_bcc() locate the byte
count at a different offset from pSMBr->ByteCount, allowing the subsequent
bounds checks to use inconsistent response boundaries.
The function also turns the 32-bit ParameterOffset and DataOffset fields
into pointers before checking their bounds. On a 32-bit client, a large
offset can wrap the pointer below end_of_smb and pass the one-sided pointer
comparison. The caller then dereferences the parameter pointer or copies
from the data pointer.
Require the expected WordCount before reading the byte count. Check each
offset and count against the received SMB length as integers before forming
either pointer. Use subtraction after validating the offset so that the
range checks cannot overflow.
Reported-by: Ratnaker Gautam <ratnaker.gautam@kuleuven.be>
Fixes: 0a4b92c05ed0 ("[CIFS] Add worker function for Get ACL cifs style")
Cc: stable@vger.kernel.org
Signed-off-by: Ratnaker Gautam <ratnaker.gautam@kuleuven.be>
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
CIFSSMBPosixLock() receives a TRANS2 response whose server-provided
DataOffset and DataCount are not required by validate_t2() to describe an
interval inside the response. A server can therefore make the client read a
cifs_posix_lock beyond the response buffer and return those kernel bytes
through F_GETLK.
Use smbCalcSize() to obtain the full response length after any multipart
TRANS2 reassembly, then reject data areas that extend past it before
dereferencing them.
Fixes: fc94cdb94462 ("[CIFS] Fix new POSIX Locking for setting lock_type correctly on unlock")
Reported-by: Ratnaker Gautam <ratnaker.gautam@kuleuven.be>
Cc: stable@vger.kernel.org
Signed-off-by: Ratnaker Gautam <ratnaker.gautam@kuleuven.be>
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
Queueing the lease break timer on system_wq triggers the following warning:
workqueue: work func lease_break_timer [ksmbd] enqueued on deprecated
workqueue. Use system_{percpu|dfl}_wq instead.
The oplock break timer and TCP transmit work also use deprecated system
queues.
Signed-off-by: ZhangGuoDong <zhangguodong@kylinos.cn>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
The danger of doing the merge window while on vacation and traveling
with just the laptop: I only do full builds a couple of times a day
because they are so slow on my poor little laptop.
As a result, I missed the conflict between the newly added smb client
fileattr_set function (commit 273913b7c33a: "smb: client: implement
fileattr_set for compression flags") and the vfs layer having in the
meantime made the idmap argument 'const' (commit 025756ee7b47: "fs: port
mnt_idmap() and file_mnt_idmap() to const mnt_idmap")
Mea culpa.
Fixes: 32543b09681a ("Merge tag 'cifs-for-7.4' of https://git.manguebit.org/linux")
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
|
|
Pull smb client updates from Paulo Alcantara:
- Implement ->fileattr_set so chattr(1) compression changes (chattr
+c/-c) go through the generic VFS fileattr path and its permission
and serialization checks instead of the raw FS_IOC_SETFLAGS fallback
and remove the now-unreachable file flags ioctl handlers
- Require stable pages when the connection is signed or SMB compression
is enabled, so a buffered write can no longer modify a folio that is
still in flight and corrupt its signature (surfaced as -EIO) or, with
compression, silently corrupt the data stored on the server
- Stop breaking a file's just-granted lease while probing it for a
Minshall+French symlink, by reusing an already open handle or opening
with the inode's lease key
- Fix several out-of-bounds reads and use-after-free bug in SMB1
- Fix an use-after-free bug in cifs_ses_add_channel()
- Fix a NULL pointer dereference in the demultiplex thread when an
encrypted read response matches no outstanding request and reclaim
the credits granted on such responses to avoid starving the
connection of credits
- Fail sends fast when the connection needs to reconnect
- Drop the redundant nls_table argument threaded through the session
setup, tree connect and path-based call paths, allocate the session
setup state on the stack and remove the internal CIFS version number
along with a batch of write-only fields and commented-out code
* tag 'cifs-for-7.4' of https://git.manguebit.org/linux: (61 commits)
smb: client: reclaim credits for decrypted reads with no matching mid
smb: client: do not pass a NULL buf to is_oplock_break()
smb: client: require stable pages for SMB compression
smb: client: allocate struct SMB2_sess_data on the stack
smb: client: allocate struct sess_data on the stack
smb: client: drop redundant nls_table arg from CIFSSMBRenameOpenFile()
smb: client: drop redundant nls_table arg from SMBLegacyOpen()
smb: client: drop redundant nls_table arg from CIFSPOSIXCreate()
smb: client: drop redundant nls_table arg from CIFSPOSIXDelFile()
smb: client: drop redundant nls_table arg from ->tree_connect()
smb: client: drop redundant nls_table arg from cifs_setup_session()
smb: client: drop redundant nls_table arg from ->sess_setup()
smb: client: drop unused nls_table arg from decode_ascii_ssetup()
smb: client: drop redundant nls_table arg from decode_unicode_ssetup()
smb: client: drop unused nls_table arg from ascii_ssetup_strings()
smb: client: drop redundant nls_table arg from unicode_ssetup_strings()
smb: client: drop unused nls_table arg from ascii_domain_string()
smb: client: drop redundant nls_table arg from unicode_domain_string()
smb: client: drop unused nls_table arg from ascii_oslm_strings()
smb: client: drop redundant nls_table arg from build_ntlmssp_auth_blob()
...
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux
Pull crypto library updates from Eric Biggers:
- Migrate the x86 and riscv optimized implementations of various AES
modes (ECB, CBC, CBC-CTS, CTR, XCTR, XTS) into lib/crypto/.
This makes the library APIs be properly accelerated on these
architectures, while still accelerating crypto_skcipher as well.
It removes a lot of redundant glue code, since the crypto API
boilerplate no longer needs to be duplicated for these architectures.
It also fixes the longstanding issue where optimizations for these
AES modes were disabled by default on these architectures.
- Optimize AES-XTS on SPARC64 using the SPARC crypto opcodes
- Move the SipHash code into lib/crypto/
- Update the ChaCha and Poly1305 functions to take size_t lengths,
resolving issues where lengths over INT_MAX were mishandled
- Make frame pointer unwinding work properly in some of the x86
assembly functions, by following the standard %rbp convention
- Add helper functions for zeroizing structs
- Make a few documentation improvements
* tag 'libcrypto-for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux: (49 commits)
lib/crypto: md5: remove unused header
lib/crypto: mips/chacha: Handle lengths of 2 GiB or more
lib/crypto: arm/chacha: Handle lengths of 2 GiB or more in scalar code
lib/crypto: chacha: Use size_t length
lib/crypto: poly1305: Use size_t length
lib/crypto: x86/sha512: Follow the normal %rbp convention
lib/crypto: x86/sha256: Follow the normal %rbp convention
lib/crypto: x86/sha1: Follow the normal %rbp convention
lib/crypto: chacha20poly1305: zeroize state in __chacha20poly1305_decrypt
lib/crypto: sparc/aes-xts: Add optimization using the AES opcodes
lib: Move SipHash into lib/crypto/
lib/crypto: x86/aes-ctr: Remove some unreachable code
crypto: x86/aesni-intel - Add transitional selections
crypto: aes - Boost priority of encryption modes further
lib/crypto: x86/aes: Set RNDKEY in aes_cbc_encrypt_aesni()
lib/crypto: riscv/aes-xts: Migrate optimized code into library
lib/crypto: riscv/aes-ctr: Migrate optimized code into library
lib/crypto: riscv/aes-cbc: Migrate optimized code into library
lib/crypto: riscv/aes-ecb: Migrate optimized code into library
lib/crypto: riscv/aes: Pass key struct to assembly code
...
|
|
Setting FILE_ATTRIBUTE_TEMPORARY on a directory with SMB2 SET_INFO
(FileBasicInformation) succeeds: ksmbd keeps the attribute and, with
"store dos attributes" enabled, also writes it to the DOS attribute
xattr.
MS-FSA (FileBasicInformation) says the request must fail with
STATUS_INVALID_PARAMETER when FILE_ATTRIBUTE_TEMPORARY is set on a
directory, and Samba rejects it too. smbtorture's
smb2.create.dosattr_tmp_dir checks this and fails against ksmbd.
Return -EINVAL, which is sent as STATUS_INVALID_PARAMETER, when a
directory is asked to become TEMPORARY. Do the check before any
timestamp is changed, so a rejected request leaves the open file and
the inode unchanged.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Assisted-by: LLM
Signed-off-by: Michail Andreas Michalopoulos <mike.michalo@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
smb_check_perm_dacl() returns the access mask it computes to the SMB2
CREATE handler, which uses it as the granted access for the open. For a
security descriptor whose DACL is present but contains no ACEs, the
function is supposed to deny access: an empty (non-NULL) DACL grants no
access to anyone except the object owner's implicit READ_CONTROL and
WRITE_DAC (MS-DTYP 2.4.5).
The empty-DACL branch only denies when both
- the DACL has no trailing bytes (pdacl_size == sizeof(struct smb_acl)),
and
- the request contains bits beyond READ_CONTROL / WRITE_DAC.
In every other case it takes the shared "goto err_out" with rc still 0,
which returns success without modifying *pdaccess, so the caller grants
the full requested access. Three problems follow:
- The READ_CONTROL / WRITE_DAC carve-out models the owner's implicit
rights, but the check runs before is_owner is computed, so those
rights are handed to *every* caller, not just the owner. For
READ_CONTROL the SMB2 CREATE path does not fall back to
inode_permission(), so any user can read the security descriptor of
an object whose empty DACL is meant to deny them.
- A request of exactly READ_CONTROL and/or WRITE_DAC is granted to any
caller. WRITE_DAC lets the handle rewrite the DACL.
- A DACL that declares zero ACEs but carries trailing bytes
(pdacl_size > sizeof(struct smb_acl)) skips the deny entirely and is
granted whatever was requested.
On shares where the NT ACL is the authoritative access control -- e.g.
"force user" shares, where every client maps to one POSIX identity so
POSIX cannot distinguish users -- this is an access-control bypass.
Compute is_owner before the empty-DACL branch and deny access from an
empty DACL unless the caller is the owner requesting no more than
READ_CONTROL / WRITE_DAC.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Aamir Ahmed <elb12345@hotmail.co.uk>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
RPC pipe handles on IPC$ are allocated from a single server-global IDA,
and ksmbd.mountd looks a pipe up by handle alone, with no session
information. smb2_read_pipe() and smb2_write_pipe() take the handle
straight from the client request (VolatileFileId) and pass it to the
daemon without checking that the requesting session opened it. An
authenticated user can therefore issue an SMB2 READ or WRITE on IPC$
with another session's handle and receive that session's in-flight
DCERPC response, or write attacker-chosen bytes into its pipe stream.
A tree connect to IPC$, which every RPC client already has, is the only
requirement.
Validate ownership before the transfer, the way the sibling pipe paths
already do: under sess->rpc_lock, ksmbd_session_rpc_method() returns 0
for a handle the session did not open. smb2_get_info_file_pipe() and
the FSCTL_PIPE_TRANSCEIVE ioctl already reject such handles; read and
write were the remaining gap. Reject them with STATUS_FILE_CLOSED, as
the ioctl path does.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Assisted-by: LLM
Signed-off-by: Mohammed Alquraini <mohammed.saad.alquraini@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
A disconnected durable handle keeps its ksmbd_file and the fd_limit
reservation taken at its initial open. Reconnecting allocates another
volatile ID and takes another reservation for the same file. Final close
returns only one reservation, so repeated reconnects permanently reduce
the server-wide open-file budget.
Move reservation into ksmbd_open_fd() and release it on every creation
failure. Keep the reservation until final close, including while a durable
handle is disconnected. Make ID allocation independent of fd_limit so
reconnects neither consume nor return slots. This also avoids returning an
unreserved slot when persistent ID allocation fails.
New opens continue to return -EMFILE when the budget is exhausted, keeping
the existing STATUS_INSUFFICIENT_RESOURCES mapping.
Fixes: c8efcc786146 ("ksmbd: add support for durable handles v1/v2")
Reported-by: Mohammed <mohammed.saad.alquraini@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
Session registration failure and connection cleanup can drop the table
reference while the per-session procfs entry is still accessible. If a
reader holds the last remaining reference, its final put calls
proc_remove() from its own callback and waits for that callback to
finish.
Remove the procfs entry before dropping the table reference in the
registration failure path. Queue sessions removed during connection
cleanup and drain their procfs readers after releasing both session
locks, then return their table references. Keep the global hash node
unhashed while a session is queued so a concurrent unregister cannot
drop the reference again.
Previous-session replacement also destroys the file table before the
session is finally freed. Detach the idr under the lock used by procfs
file counters, and report zero files after it has been detached. This
prevents a reader from accessing the idr during or after its
destruction.
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>
|
|
LOGOFF and previous-session replacement leave sessions Expired with a
zero credential expiry. Ended sessions can retain their memory and
connection slots when no further setup arrives on the owning connection.
Unpublish a logged-off session while the request reference keeps its
response signing and encryption keys alive. Remove its procfs entry
before dropping the table reference so a reader cannot remove its own
entry on the last put. Clear proc_entry after removal to avoid removing
it twice.
Reclaim ended sessions from the global table in the existing expiration
worker before it acquires conn_list_lock. Only select Expired sessions
with zero credential expiry and no references beyond the table
reference. Keep sessions whose Kerberos credentials expired available
for reauthentication.
Unlink the session from its setup connection and all channel
connections, so SMB2 sessions and sessions with only secondary SMB3
channels are covered. The session already holds connection references,
and sessions_table_lock prevents concurrent channel removal. Queue
sessions on a separate disposal list, leaving their hash nodes unhashed,
and remove procfs entries and drop table references outside the locks.
Keep the pending setup check before allocating a new session so
abandoned InProgress sessions can release their slots even at the
connection limit. Retain the timeout and reference checks and dispose of
those sessions outside the locks. Leave ended Expired sessions to the
expiration worker.
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 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>
|
