| Age | Commit message (Collapse) | Author |
|---|
|
https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
|
|
|
|
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>
|
|
The receive thread queues a request and can read the next PDU before
the negotiation worker finishes. While need_neg is set, the next PDU
can cause ksmbd_init_smb_server() to replace conn->ops. A pipelined
SMB2 request can therefore make the worker handle an earlier, shorter
SMB1 request with SMB2 operations and read beyond its buffer.
An SMB1 multi-protocol negotiate may be followed by an SMB2 NEGOTIATE
that changes conn->ops again. Wait for work queued while the connection
is new or needs negotiation to complete before reading another PDU.
This keeps the operations stable for both stages of negotiation without
changing when need_neg is cleared.
Fixes: dc8289f91238 ("ksmbd: fix slab-out-of-bounds in init_smb2_rsp_hdr")
Reported-by: ihopenre-eng <ihopenre@gmail.com>
Reviewed-by: ihopenre-eng <ihopenre@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
MS-SMB2 3.3.5.2.1.1 requires the Flags/EncryptionAlgorithm field in an
SMB2 transform header to be 0x0001. The server must disconnect the
connection when it has any other value.
smb3_decrypt_req() currently ignores this field. A client with the
session key can therefore send a correctly authenticated encrypted
request with invalid flags and have it processed. Check the field
before looking up the session key or decrypting the request. The caller
aborts the connection when this check fails.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
When an encrypted request comes in, the decryption key is picked by
the SessionId in the transform header, but the request is then
authorized under whatever session the decrypted inner header names.
Nothing ever compares the two, so on a connection with more than
one session a client can get a request decrypted with one session's
key and run it as another session. Encrypted requests are exempt
from signing too, so a valid AEAD tag is the only proof of session
identity, and it is checked against the wrong session.
MS-SMB2 says the server must check that the transform SessionId
matches the one in the decrypted SMB2 header and treat a mismatch
as a protocol error. Add that check in __handle_ksmbd_work() after
decryption, before dispatching: the message has to be at least one
fixed SMB2 header, the first operation must not set
SMB2_FLAGS_RELATED_OPERATIONS and its SessionId must match the
transform SessionId, and every following non-related operation in
a compound chain must carry the same SessionId. NextCommand
offsets must be 8-byte aligned and stay inside the message; the
bounds check is done by subtraction so it cannot wrap.
When the decrypted payload is a compression transform, the
existing decompression step runs first and the decompressed SMB2
message is checked instead, since a compression transform header
carries no SessionId.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Reported-by: Dairui Zhang <zhangdairui@gmail.com>
Assisted-by: LLM
Cc: stable@vger.kernel.org
Signed-off-by: Dairui Zhang <zhangdairui@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
ksmbd_validate_name_reconnect() indexes
ab_pathname[share->path_sz + 1] without checking that the path is
longer than share->path_sz + 1 (d_path() fills the buffer from the
end). A durable fp is not bound to its original share, so a client
can preserve a file from a short-path share and reconnect it on a
long-path share, making strcmp() read past the PATH_MAX buffer -
with a long enough share path this oopses the kernel, and on a
mapped page it also gives a byte-equality oracle on adjacent heap
(the reconnect succeeds exactly when the out-of-bounds string
equals the requested name). A same-share variant with the durable
fp at the share root reads 1 byte past the buffer
(strlen(ab_pathname) == share->path_sz).
Check before comparing: the path must be longer than share->path_sz,
it must start with share->path, and the next character must be '/'.
The share root itself is accepted only for an empty name.
Fixes: c8efcc786146 ("ksmbd: add support for durable handles v1/v2")
Reported-by: Dairui Zhang <zhangdairui@gmail.com>
Assisted-by: LLM
Cc: stable@vger.kernel.org
Signed-off-by: Dairui Zhang <zhangdairui@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
The POSIX extension specifications have now been published:
https://smb3posix.org/
Suggested-by: Paulo Alcantara <pc@manguebit.org>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: ZhangGuoDong <zhangguodong@kylinos.cn>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
smb2_allocate_rsp_buf() uses the MAX_CIFS_SMALL_BUFFER_SIZE
(448-byte) buffer for a single SMB2_CREATE response. That buffer also
holds a 4-byte length field, which only leaves 444 bytes for the SMB2
body.
The AAPL response made this buffer too small. After 8f1b796ff113, a
request with AAPL response contexts may need at least 456 bytes. In
the 456-byte case, create_aapl_rsp_buf() is appended last, clears 128
bytes, and writes 12 bytes past the allocation.
KASAN reports:
BUG: KASAN: slab-out-of-bounds in create_aapl_rsp_buf+0x31/0x6e0
Write of size 128 at addr ffff88800370954c by task kworker/0:0/9
...
create_aapl_rsp_buf+0x31/0x6e0
smb2_open+0x58f9/0xef10
...
smb2_allocate_rsp_buf+0x19d/0x370
...
The buggy address is located 332 bytes inside of
allocated 448-byte region [ffff888003709400, ffff8880037095c0)
Reserve enough space for any fixed CREATE response KSMBD can build.
Store the required allocation size in the per-dialect values table.
This keeps the small buffer for other commands and avoids using the max
transaction buffer for every CREATE.
Fixes: 8f1b796ff113 ("ksmbd: add AAPL kAAPL_SERVER_QUERY create context support")
Assisted-by: Codex:gpt-5
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
Named pipe handles are tracked in sess->rpc_handle_list rather than
sess->file_table.idr. The generic IOCTL file lookup therefore returns
-ENOENT and STATUS_OBJECT_NAME_NOT_FOUND for valid Windows 11
FSCTL_PIPE_TRANSCEIVE requests.
Validate the RPC handle under rpc_lock and bypass the regular file
lookup only when the handle exists. Return STATUS_FILE_CLOSED for
invalid handles. Populate response file IDs before dispatching the
transceive request.
Assisted-by: OpenCode:gpt-6-astra
Signed-off-by: Omkar Chandorkar <gotenksIN@aospa.co>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
FileAlternateNameInformation and FileNormalizedNameInformation
use the same structure.
Link: https://lore.kernel.org/linux-cifs/8360c969-33f5-4d96-a44d-c6fcb7a8a41f@chenxiaosong.com/
Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
See MS-FSA 2.1.5.12.30.
Set AvailableNameLength to
BlockAlignTruncate((OutputBufferSize -
FieldOffset(FILE_NAME_INFORMATION.FileName)), 2).
Link: https://lore.kernel.org/linux-cifs/ebb45fdd-76f0-4d92-9899-13ecdaea555c@chenxiaosong.com/
Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
Reproducer:
1. touch /mnt/file
2. smbinfo filealternatenameinfo /mnt/file
Alternate Name: FILE~9!.
See MS-FSCC 2.1.5.2.1. The base filename MUST be 1-8 characters in length
and MUST NOT contain a "." period character.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
conn->inet_addr and conn->inet6_addr share a union, and three procfs
printers choose between them by testing whether inet_addr is zero. For
an IPv6 peer that field aliases the first four bytes of the address,
which are zero only for :: prefixed addresses, so a client with a
global IPv6 address takes the IPv4 branch and is listed from those four
bytes. A client at 2001:db8::1 is listed as 32.1.13.184.
IPv4 mapped addresses begin with zero bytes, so the listing is correct
until a native IPv6 client connects.
Record the address family where the address is stored and select on
that, in the client listing and in both session listings.
Fixes: fe4dc5987d7d ("ksmbd: expose connection runtime state in procfs")
Signed-off-by: Gael Blivet <gael.blivet@gmail.com>
Assisted-by: Claude:claude-opus-5
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
smb2_set_stream_name_xattr() synthesizes a 60-byte AFP_AfpInfo xattr
when a client opens a nonexistent one on a Time Machine share, so the
probe is answered instead of failing with -EBADF. Its header does not
match what clients write: ksmbd uses 0x00051607 and 0x00020000 for the
first two fields and leaves the third zero.
The user.DosStream.AFP_AfpInfo:$DATA xattr a macOS client writes
begins:
00000000: 4146 5000 0000 0100 0000 0000 0000 0080 AFP.............
Write those sixteen bytes: the signature, the version at offset 4 and
the backup time at offset 12. The 60-byte length is already correct and
the rest stays zeroed: a zero type and creator lets the client use the
file extension for icon and type detection.
Fixes: eaff8e924f60 ("ksmbd: synthesize empty AFP_AfpInfo xattr on first probe")
Signed-off-by: Gael Blivet <gael.blivet@gmail.com>
Assisted-by: Claude:claude-opus-5
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
get_file_stream_info() walks the xattr list from a single
ksmbd_vfs_listxattr() and sizes each stream it finds with
ksmbd_vfs_casexattr_len(), which matches with strncasecmp(). Linux
xattr names are case-sensitive, so two streams differing only in case
report each other's size.
The loop already holds the exact name, so query it directly with
ksmbd_vfs_xattr_len(), which is what casexattr_len() calls once its
search succeeds. Make it non-static and declare it in vfs.h, and drop
the comment above it, which describes a different bug and no longer
matches the code.
This also drops a full listing per stream: casexattr_len() runs its own
listxattr on every call, so enumerating a file with N xattrs cost N
listings instead of one.
Fixes: 689f1eb3719d ("ksmbd: report actual xattr value length in stream enumeration")
Signed-off-by: Gael Blivet <gael.blivet@gmail.com>
Assisted-by: Claude:claude-opus-5
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
smb2_rename() creates the target stream with
ksmbd_vfs_setxattr(..., NULL, 0, ...), so the source stream's content
is not carried over and renaming a named stream produces a zero-length
stream.
Read the source stream and write it under the new name. The lookup is
case-insensitive because smb2_set_stream_name_xattr() stores
fp->stream.name with the client's casing, which need not be the casing
on disk. A rename to the stream the handle already refers to does
nothing; that comparison is case-insensitive too, since every lookup in
this path is. A read failure now fails the rename instead of creating
an empty target, and a write failure reports its own errno rather than
-EINVAL, since writing a value can fail where writing an empty one
could not.
Initialize s_type, which parse_stream_name() assigns only when the name
carries an explicit stream type and which selects the target xattr
name.
parse_stream_name() leaves the stream name empty when the stream part
is, as in "::$DATA" or ":", returning NULL for the first spelling and
"" for the second. Return without creating an xattr in either case,
after the base name check so that a base ending in '/' is covered too.
These previously created xattrs named "(null)" and "".
The source xattr is left in place and fp->stream.name still refers to
it, as before this change: readers of that pointer take no lock, so it
cannot be swapped here.
Signed-off-by: Gael Blivet <gael.blivet@gmail.com>
Assisted-by: Claude:claude-opus-5
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
Add a maximal-access variant of the smb_check_perm_dacl() boundary
test. The in-boundary ACE grants read access, while a trailing ACE
beyond the declared DACL size grants write access.
Verify that maximal-access calculation includes the in-boundary
permission and ignores the trailing permission.
Suggested-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Hang Nan <nanx95726@gmail.com>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
Drive smb_check_perm_dacl() through ksmbd's NTACL xattr path with a
crafted descriptor whose second ACE is beyond the declared DACL size.
Verify that the out-of-boundary ACE is not selected and access remains
denied.
Suggested-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Hang Nan <nanx95726@gmail.com>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
smb_check_perm_dacl() must stop walking ACEs at the DACL declared
size instead of using the enclosing security descriptor length.
Add the ksmbd KUnit test configuration and a semantic harness that
verifies a crafted access-granting ACE beyond the declared DACL size is
ignored.
Suggested-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Suggested-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Hang Nan <nanx95726@gmail.com>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
The previous design registered a synthetic work struct (in_work)
directly on conn->async_requests and deferred the response to a
workqueue -- a bespoke async/cancel implementation duplicating what
setup_async_work(), release_async_work(), and smb2_send_interim_resp()
already provide for smb2_lock()'s pending byte-range lock.
Replace it with that same pattern: setup_async_work() on the calling
work itself, registered on fp->blocked_works, woken by cancel or by
the handle closing via the existing set_close_state_blocked_works().
This removes the synthetic work struct, the notify_pendings list and
its close-time drain, and the deferred workqueue send, leaving
smb2_notify() sharing the same async/cancel machinery as smb2_lock()
instead of its own separate copy.
The worker now blocks on ksmbd_wq for as long as the watch stays
open, instead of returning immediately. This also makes the skeleton
ready for a future event-delivery implementation on the same
blocking wait.
Suggested-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Gael Blivet <gael.blivet@gmail.com>
Assisted-by: Claude:claude-sonnet-5
Tested-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
A Windows client can send a compound request containing:
Create Request, File: <share>; Notify Request
The Notify Request uses FFFF...FFFF as the compound FID.
Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
When the large dacl_offset is provided, dacl_offset + sizeof(struct
smb_acl) overflows on 32-bit builds and the check is passed,
Which makes le16_to_cpu(ppdacl_ptr->size) read out-of-bounds. This
only happens for dacl_offset in range [0xFFFFFFF8, 0xFFFFFFFF]. So
the oob read is max 8 bytes preceding the allocated ppntsd.
unsigned int dacl_offset = le32_to_cpu(ppntsd->dacloffset);
int ppdacl_size, ntacl_size = ppntsd_size - dacl_offset;
if (!dacl_offset ||
(dacl_offset + sizeof(struct smb_acl) > ppntsd_size))
goto out;
Commit beff0bc9d69b ("ksmbd: fix overflow in dacloffset bounds check")
fixes similar checks in other functions. But leaves this one out,
because it is only valid for 32-bit builds.
Reproduced on 32-bit ARM with KASAN, by a client sending SMB2
SET_INFO(SEC) with dacloffset=0xfffffff8, then QUERY_INFO(SEC) on the
same file:
BUG: KASAN: slab-out-of-bounds in build_sec_desc (fs/smb/server/smbacl.c:1105)
Read of size 2 at addr c2a273fa by task kworker/1:1/35
CPU: 1 UID: 0 PID: 35 Comm: kworker/1:1 Not tainted 7.3.0-rc2+ #5
Workqueue: ksmbd-io handle_ksmbd_work
Call trace:
kasan_check_range from build_sec_desc (fs/smb/server/smbacl.c:1105)
build_sec_desc from smb2_query_info (fs/smb/server/smb2pdu.c:7676)
smb2_query_info from handle_ksmbd_work (fs/smb/server/server.c:160)
handle_ksmbd_work from process_one_work (kernel/workqueue.c:3401)
process_one_work from worker_thread (kernel/workqueue.c:3473)
worker_thread from kthread (kernel/kthread.c:436)
Add overflow check.
Fixes: 8f0541186e9a ("ksmbd: fix heap-based overflow in set_ntacl_dacl()")
Cc: stable@vger.kernel.org
Assisted-by: LLM claude
Tested-by: Xiang Mei <xmei5@asu.edu>
Signed-off-by: Giorgi Kobakhia <gkobakhi@asu.edu>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
fsctl_query_iface_info_ioctl() appends an entry before checking that a
usable address was found, so it advertises unspecified-address
placeholders: an entry containing 0.0.0.0 when an in_device exists but
has no primary IPv4 address, and an entry containing :: when an
inet6_dev exists but its address list is empty or every address is
tentative or deprecated.
The loop also couples IPv6 reporting to IPv4 success: when
__in_dev_get_rtnl() returns NULL it continues to the next interface,
skipping the IPv6 pass, so an interface without an in_device is dropped
even if it has a usable IPv6 address.
Query IPv4 and IPv6 independently and append an entry only when an
address is found. Factor out the common entry initialization and query
the link speed once per interface instead of once per address family.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Reported-by: Sashiko <sashiko-bot+sashiko@kernel.org>
Link: https://sashiko.dev/#/patchset/20260910090818.24479-1-15815827059%40163.com
Assisted-by: Codex:gpt-5.6-sol
Signed-off-by: Henry Hu <huhai@kylinos.cn>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
fsctl_query_iface_info_ioctl() assigns nii_rsp to the next output
slot before checking whether the interface has an in_device or
inet6_dev. If either lookup returns NULL, the loop continues without
advancing nbytes, leaving nii_rsp pointing at an uncommitted slot. If
no later entry is committed, clearing nii_rsp->Next updates that slot
instead of the last returned entry, leaving the latter with a nonzero
Next offset.
Fix this by using nbytes to locate the last committed entry and clear
its Next field, but only when the response contains at least one entry.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Reported-by: Sashiko <sashiko-bot+sashiko@kernel.org>
Link: https://sashiko.dev/#/patchset/20260910090818.24479-1-15815827059%40163.com
Signed-off-by: Henry Hu <huhai@kylinos.cn>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
fsctl_query_iface_info_ioctl() walks idev6->addr_list with a plain
list_for_each_entry() while holding only rtnl_lock(). rtnl does not
serialize all writers of this list: the DAD-failure path
(addrconf_dad_failure -> ipv6_add_addr) can insert addresses without
holding RTNL. It updates idev->addr_list with list_add_tail_rcu() under
write_lock_bh(&idev->lock), so it can race with the unlocked walk.
addr_list is RCU managed, so walk it with list_for_each_entry_rcu()
under rcu_read_lock(), matching the other lockless readers of the list.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Reported-by: Sashiko <sashiko-bot+sashiko@kernel.org>
Link: https://sashiko.dev/#/patchset/20260910090818.24479-1-15815827059%40163.com
Co-developed-by: Xuanqiang Luo <luoxuanqiang@kylinos.cn>
Signed-off-by: Xuanqiang Luo <luoxuanqiang@kylinos.cn>
Signed-off-by: Henry Hu <huhai@kylinos.cn>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
fsctl_query_iface_info_ioctl() used to call
netdev->ethtool_ops->get_link_ksettings() directly and never checked the
return value. If the callback fails, cmd.base.speed may be left
uninitialized. Also, SPEED_UNKNOWN (-1) is treated as a numeric link speed
and would be converted into a bogus link speed.
Use __ethtool_get_link_ksettings() to acquire the per-netdev ops lock when
needed. Fall back to 1Gbps when the query fails, or when the reported speed
is 0 or SPEED_UNKNOWN.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Signed-off-by: Henry Hu <huhai@kylinos.cn>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
When get_inode_acl() fails, it returns an ERR_PTR(), but
ksmbd_acls_fattr() stored that pointer in smb_fattr without
checking for errors. Later callers then passed the error pointer
to security descriptor helpers or posix_acl_release(), resulting
in an invalid pointer dereference.
This was reproduced on an XFS-backed share when xfs_get_acl()
returned ERR_PTR(-ENOMEM). Querying the SMB ACL from a CIFS client
triggered the crash:
BUG: unable to handle page fault for address: fffffffffffffff8
RIP: smb_acl_sec_desc_scratch_len+0x58/0x90 [ksmbd]
Call Trace:
smb2_query_info+0x40d/0x17c0 [ksmbd]
handle_ksmbd_work+0x1ca/0x770 [ksmbd]
Fix this by checking the return values from get_inode_acl() in
ksmbd_acls_fattr(). Treat -EOPNOTSUPP as indicating that no ACL is
available, and propagate all other errors to the callers. Update
smb2_open() and smb2_get_info_sec() to handle these errors.
Cc: stable@vger.kernel.org
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Assisted-by: Codex:gpt-5.6-sol
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Henry Hu <huhai@kylinos.cn>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
ksmbd_find_netdev_name_iface_list() walks iface_list for
fsctl_query_iface_info_ioctl(), which holds RTNL. But during server
reset, ksmbd_tcp_destroy() unlinks and frees the entries without RTNL,
and queued SMB requests can still be running at that point. So an
FSCTL_QUERY_NETWORK_INTERFACE_INFO request can therefore race with
interface destruction and dereference a freed entry. With debug-only
instrumentation to make the race deterministic, KASAN reports:
BUG: KASAN: slab-use-after-free in
ksmbd_find_netdev_name_iface_list
ksmbd_find_netdev_name_iface_list
fsctl_query_iface_info_ioctl
smb2_ioctl
handle_ksmbd_work
Freed by:
kfree
ksmbd_tcp_destroy
ksmbd_conn_transport_destroy
kill_server_store
The fix is to detach iface_list to a temporary list under RTNL, then drop
RTNL before cleanup because the following cleanup may block and hold RTNL
for a long time.
Fixes: b2d99376c5d6 ("ksmbd: browse interfaces list on FSCTL_QUERY_INTERFACE_INFO IOCTL")
Reported-by: Sashiko <sashiko-bot+sashiko@kernel.org>
Link: https://sashiko.dev/#/patchset/20260911163516.6403-1-15815827059%40163.com
Assisted-by: LLM
Signed-off-by: Henry Hu <huhai@kylinos.cn>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
Initialize the compatibility byte in request buffers without zeroing
the entire allocation. Keep the one-byte compatibility exception for
SMB2 messages so valid zero-length WRITE requests remain compatible,
and validate the data range in the WRITE handlers against the current
compound request before accessing it.
This prevents stale request-buffer data from being written to files or
sent to IPC pipes.
Fixes: 443d61d1fa9f ("ksmbd: allocate one more byte for implied bcc[0]")
Cc: stable@vger.kernel.org
Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-32436
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
Now that everything takes a mnt_idmap as const store a const pointer in
struct vfsmount, struct mount_kattr and struct kstatmount and return one
from mnt_idmap(), file_mnt_idmap() and ovl_upper_mnt_idmap(). Finally,
also make mnt_idmap_get() return a const pointer. Also convert the
remaining local variables that are initialized from the accessors.
alloc_mnt_idmap() keeps returning a non-const pointer. It is the only
place where an idmapping is actually written to.
Link: https://patch.msgid.link/20260901-work-idmap-const-v1-26-54ccd48e100b@kernel.org
Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Seth Forshee <sforshee@kernel.org>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
Convert to const struct mnt_idmap.
A mount's idmapping is immutable. The only thing that is allowed to be
modified afterwards is the reference count and that is hidden behind
mnt_idmap_get() and mnt_idmap_put(). Everything else only ever reads
from the idmapping. This is the same model that struct cred uses and the
idmapping is also rather sensitive.
So make the idmap argument const wherever we can. The conversion is done
from the bottom up so callers can continue to pass a non-const pointer
to a const parameter until the conversion is finished.
No functional changes.
Link: https://patch.msgid.link/20260901-work-idmap-const-v1-15-54ccd48e100b@kernel.org
Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Seth Forshee <sforshee@kernel.org>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
Convert to const struct mnt_idmap.
A mount's idmapping is immutable. The only thing that is allowed to be
modified afterwards is the reference count and that is hidden behind
mnt_idmap_get() and mnt_idmap_put(). Everything else only ever reads
from the idmapping. This is the same model that struct cred uses and the
idmapping is also rather sensitive.
So make the idmap argument const wherever we can. The conversion is done
from the bottom up so callers can continue to pass a non-const pointer
to a const parameter until the conversion is finished.
No functional changes.
Link: https://patch.msgid.link/20260901-work-idmap-const-v1-13-54ccd48e100b@kernel.org
Acked-by: Paul Moore <paul@paul-moore.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Seth Forshee <sforshee@kernel.org>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
Convert to const struct mnt_idmap.
A mount's idmapping is immutable. The only thing that is allowed to be
modified afterwards is the reference count and that is hidden behind
mnt_idmap_get() and mnt_idmap_put(). Everything else only ever reads
from the idmapping. This is the same model that struct cred uses and the
idmapping is also rather sensitive.
So make the idmap argument const wherever we can. The conversion is done
from the bottom up so callers can continue to pass a non-const pointer
to a const parameter until the conversion is finished.
No functional changes.
Link: https://patch.msgid.link/20260901-work-idmap-const-v1-12-54ccd48e100b@kernel.org
Acked-by: Paul Moore <paul@paul-moore.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Seth Forshee <sforshee@kernel.org>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
Convert to const struct mnt_idmap.
A mount's idmapping is immutable. The only thing that is allowed to be
modified afterwards is the reference count and that is hidden behind
mnt_idmap_get() and mnt_idmap_put(). Everything else only ever reads
from the idmapping. This is the same model that struct cred uses and the
idmapping is also rather sensitive.
So make the idmap argument const wherever we can. The conversion is done
from the bottom up so callers can continue to pass a non-const pointer
to a const parameter until the conversion is finished.
No functional changes.
Link: https://patch.msgid.link/20260901-work-idmap-const-v1-5-54ccd48e100b@kernel.org
Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Seth Forshee <sforshee@kernel.org>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
|
Convert to const struct mnt_idmap.
A mount's idmapping is immutable. The only thing that is allowed to be
modified afterwards is the reference count and that is hidden behind
mnt_idmap_get() and mnt_idmap_put(). Everything else only ever reads
from the idmapping. This is the same model that struct cred uses and the
idmapping is also rather sensitive.
So make the idmap argument const wherever we can. The conversion is done
from the bottom up so callers can continue to pass a non-const pointer
to a const parameter until the conversion is finished.
No functional changes.
Link: https://patch.msgid.link/20260901-work-idmap-const-v1-3-54ccd48e100b@kernel.org
Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Seth Forshee <sforshee@kernel.org>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
|
