-----BEGIN PGP SIGNATURE-----
iQGzBAABCgAdFiEE6fsu8pdIjtWE/DpLiiy9cAdyT1EFAmpRLz8ACgkQiiy9cAdy
T1GalgwAqtaIFVMj7T1CDZcV/S0RymYPaiI2RNaaEcdn1T59mpsjaf5UVHwXN04K
SZjQcAdGOgWvjZlPYvzah4pw0/T6EHRMYe7d2MRanV5epx6PJqfmBS3/gJ+I1NvN
jckWo3RPnrixYWpIH/f9S0aIksyr19UUsxkDpfUHhQPerWXwIZNXR0+k/VbiHUDn
OnyhpasP6/okrNqZBZ8GjkCx7p5PENckd1p5hD+NeIRzQybHuWPos7kGIExp1973
BvXXzK76OXvB4aOUS/ofP1MxbmCvMaXUeM3JecPSspjOiu02z2qmRCnJVsCXztPC
uHe63kKHWSlunSAVH1wJHTG3BsI0hTK9Kmkyi7jbExpqYydpXwjLSEeQeggnoCBj
bQuVNslAYRzPFddjhxGyTkIyry8Hh2drlCD7FF1cxnlIz4x9ADOxiyEKHS7m9Xv+
ZJiEIuhs5IUJnU7xGXqSx4Ve4AU8rW7aRXYvXRS1LrdWj8EZpbIW70leFyv6ocAI
hG/TeGmG
=RhsK
-----END PGP SIGNATURE-----
Merge tag 'v7.2-rc2-smb3-client-fixes' of git://git.samba.org/sfrench/cifs-2.6
Pull smb client fixes from Steve French:
- DFS cache allocation fix
- DFS referral bounds check fix
- Fix absolute symlinks when mounting with POSIX extensions
- Fixes for incorrect nlink returned by fstat
- Fix atime in read completion
- Fix busy dentry on umount
- ioctl_query_info buffer overflow fix
- Two fixes for creating special files with SFU
- Fix mode mask in parse_dacl
- SMB1 is_path_accessible wildcard fix and minor SMB1 cleanup
- smb2_check_message fix
- Debug message improvement
- Minor cleanup
* tag 'v7.2-rc2-smb3-client-fixes' of git://git.samba.org/sfrench/cifs-2.6:
cifs: Remove CIFSSMBSetPathInfoFB() fallback function
cifs: Fix and improve cifs_is_path_accessible() function
smb: client: mask server-provided mode to 07777 in modefromsid
cifs: Show reason why autodisabling serverino support
smb/client: fix incorrect nlink returned by fstat()
smb/client: zero-initialize stack-allocated cifs_open_info_data
smb/client: pass cifs_open_info_data to SMB2_open()
smb/client: use stack-allocated smb2_file_all_info in smb3_query_mf_symlink()
smb: client: fix overflow in passthrough ioctl bounds check
smb: client: fix busy dentry warning on unmount after DIO
cifs: Fix support for creating SFU fifo
cifs: Fix support for creating SFU socket
smb: client: fix atime clamp check in read completion
cifs: validate DFS referral string offsets
smb: client: use GFP_KERNEL for DFS cache allocations
smb: client: restrict implied bcc[0] exemption to responses without data area
smb: client: preserve leading slash for POSIX absolute symlink targets
smb: client: refactor cifs_revalidate_mapping() to use clear_and_wake_up_bit()
This fallback function CIFSSMBSetPathInfoFB() is called only from
CIFSSMBSetPathInfo() function. CIFSSMBSetPathInfo() is used in
smb_set_file_info() which contains all required fallback code, including
fallback via filehandle, since commit f122121796 ("cifs: Fix changing
times and read-only attr over SMB1 smb_set_file_info() function") and
commit 92210ccd87 ("cifs: Add fallback code path for cifs_mkdir_setinfo()").
So the CIFSSMBSetPathInfoFB() is just code duplication, which is not needed
anymore. Therefore remove it.
Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Do not call SMBQueryInformation() command for path with SMB wildcard
characters on non-UNICODE connection because server expands wildcards.
Function cifs_is_path_accessible() needs to check if the real path exists
and must not expand wildcard characters.
Do not dynamically allocate memory for small FILE_ALL_INFO structure and
instead allocate it on the stack. This structure is allocated on stack by
all other functions.
When CAP_NT_SMBS was not negotiated then do not issue CIFSSMBQPathInfo()
command. This command returns failure by non-NT Win9x SMB servers, so there
is no need try it. The purpose of cifs_is_path_accessible() function is
just to check if the path is accessible, so SMBQueryInformation() for old
servers is enough.
Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
When modefromsid is active, parse_dacl() applies the server-provided
sub_auth[2] value from the NFS mode SID to cf_mode without masking to
07777. Apply the correct masking, same as in the read path.
Fixes: e2f8fbfb8d ("cifs: get mode bits from special sid on stat")
Signed-off-by: Norbert Manthey <nmanthey@amazon.de>
Assisted-by: Kiro:claude-opus-4.6
Cc: stable@vger.kernel.org
Signed-off-by: Steve French <stfrench@microsoft.com>
Extend cifs_autodisable_serverino() function to print also text message why
the function was called.
The text message is printed just once for mount then autodisabling
serverino support. Once the serverino support is disabled for mount it will
not be re-enabled. So those text messages do not cause flooding logs.
This change allows to debug issues why cifs.ko decide to turn off server
inode number support and hence disable support for detection of hardlinks.
Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Stack-allocated cifs_open_info_data may contain random data.
This can make some fields have wrong value if they are not set later.
Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Steve French <stfrench@microsoft.com>
Let SMB2_open() fill the smb2_file_all_info embedded in cifs_open_info_data
directly. This removes the temporary smb2_file_all_info copy in
smb2_open_file().
Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Steve French <stfrench@microsoft.com>
SMB2_open() only fills the fixed fields, so a stack-allocated
smb2_file_all_info is sufficient here.
Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Steve French <stfrench@microsoft.com>
smb2_ioctl_query_info() validates the PASSTHRU_FSCTL response payload
before copying it to userspace.
The payload offset and length both come from 32-bit fields. The bounds
check currently adds OutputOffset and qi.input_buffer_length directly, so
the addition can wrap in 32-bit arithmetic before the result is compared
against the response buffer length.
A malicious server can use a large OutputOffset and a small OutputCount
to make the wrapped sum pass the bounds check. The later copy_to_user()
then reads from io_rsp + OutputOffset, outside the response buffer.
Use size_add() for the offset plus length check so overflow is treated as
out of bounds.
Fixes: 2b1116bbe8 ("CIFS: Use common error handling code in smb2_ioctl_query_info()")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
Commit c68337442f ("cifs: Fix busy dentry used after unmounting") fixed
the issue in cifs where deferred close of a file led to a dentry reference
count not being released in umount, by flushing deferredclose_wq in
cifs_kill_sb() to solve it.
However, the cifs DIO path suffers from the same busy-dentry problem caused
by a delayed dentry reference-count release:
[dio] [cifsd] [close + umount]
netfs_unbuffered_write_iter_locked
...
cifs_demultiplex_thread
netfs_unbuffered_write
cifs_issue_write
netfs_wait_for_in_progress_stream [1]
...
netfs_write_subrequest_terminated
netfs_subreq_clear_in_progress
netfs_wake_collector // wake [1]
netfs_put_subrequest
netfs_put_request
queue_work(system_dfl_wq, xxx) [2]
// dio write return cifs_close
_cifsFileInfo_put
// cfile->count 2->1
--cfile->count [3]
// umount
cifs_kill_sb
kill_anon_super
// warning triggered!
shrink_dcache_for_umount [4]
[system_dfl_wq] [5]
netfs_free_request
...
_cifsFileInfo_put
// cfile->count 1->0
--cfile->count
queue_work(fileinfo_put_wq, xxx)
[fileinfo_put_wq] [6]
cifsFileInfo_put_work
cifsFileInfo_put_final
dput
If the umount path is triggered before [5], it results warning:
BUG: Dentry 00000000eab1f070{i=9a917b66ae404fec,n=test} still in use (1)
[unmount of cifs cifs]
The existing per-inode ictx->io_count wait in cifs_evict_inode() does not
help: it lives in the inode eviction path, which runs after
shrink_dcache_for_umount() has already warned about the busy dentries.
Fix it by adding a per-superblock outstanding-rreq counter that is
incremented in cifs_init_request() and decremented in cifs_free_request().
In cifs_kill_sb(), before kill_anon_super(), wait for this counter to reach
0 - which guarantees that all cleanup_work for this sb have run and thus
all relevant cfile puts are queued on fileinfo_put_wq or serverclose_wq.
Then drain the workqueue so the dentry refs are dropped.
This is a targeted wait, not a flush of the system-wide system_dfl_wq.
Fixes: 340cea84f6 ("cifs: open files should not hold ref on superblock")
Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
SFU fifos are natively supported (created and recognized) at least by:
- Microsoft POSIX subsystem
- OpenNT/Interix subsystem
- Microsoft SFU (Windows Services for UNIX)
- Microsoft SUA (Subsystem for UNIX-based Applications)
- Windows NFS server (up to the Windows Server 2008 R2)
Windows NFS server since Windows Server 2012 uses new reparse point format
for storing new fifos, but still can recognize this old format (also in the
latest Windows Server 2022 version).
SFU-style fifo is empty regular file which has system attribute set.
These SFU-style fifos are already recognized by Linux SMB client.
But Linux SMB client is currently creating new SFU fifos in different
format which is not compatible with all those SFU-style consumers. Fix this
by creating new fifos in correct SFU format which would be recognized by
all those applications and also by existing Linux SMB clients.
This change affects only creating new fifos when mount option -o sfu is used.
Signed-off-by: Pali Rohár <pali@kernel.org>
Acked-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
SFU sockets are natively supported by Interix 3.0 subsystem and also by
later versions. It is part of Microsoft SFU (Windows Services for UNIX) and
Microsoft SUA (Subsystem for UNIX-based Applications). They can be created
and existing (stored on local disk or remote SMB share) can be recognized.
SFU sockets are recognized also by NFS server included in Windows Server.
Windows NFS server versions since Windows Server 2012 uses new reparse
point format for storing new sockets, but still can recognize this old
format (also in the latest Windows Server 2022 version).
SFU-style socket is a regular file which has system attribute set and
content of the file is one zero byte.
These SFU-style sockets are already recognized by Linux SMB client.
But Linux SMB client is currently creating new SFU socket in different
format which is not compatible with all those SFU applications. Fix this by
creating new sockets in correct SFU format which would be recognized by all
SFU, SUA, NFS and existing Linux SMB clients.
This change affects only creating new sockets when mount option -o sfu is used.
Signed-off-by: Pali Rohár <pali@kernel.org>
Acked-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
cifs_rreq_done() updates the inode atime to current_time(inode) after a
netfs read. It then preserves the CIFS rule that atime should not be
older than mtime, because some applications break if atime is less than
mtime. That rule only requires clamping when atime < mtime.
The current check uses the raw non-zero result of timespec64_compare().
It therefore takes the clamp path for both atime < mtime and
atime > mtime. The latter is the normal case when reading an older file:
the newly recorded atime is newer than the file mtime. The completion
handler then immediately moves atime back to mtime, losing the access
time that was just recorded. Userspace tools that rely on atime, such as
stat, find -atime, backup tools or cold-data classifiers, can therefore
see a recently read CIFS file as not recently accessed.
This is easy to miss because the bug is silent: read I/O still succeeds,
no error is reported, and many systems either do not check atime after
reads or mount with policies such as relatime/noatime. It becomes
visible when a CIFS file has an mtime older than the current time, the
file is read, and the local inode atime is inspected before a later
revalidation replaces the cached timestamps.
Clamp only when atime is actually older than mtime. This matches the
same atime/mtime rule used when applying CIFS inode attributes.
Fixes: 69c3c023af ("cifs: Implement netfslib hooks")
Cc: stable@vger.kernel.org
Signed-off-by: Xu Rao <raoxu@uniontech.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
parse_dfs_referrals() validates that the response header and referral
array fit in the received buffer, but each referral also contains string
offsets supplied by the server.
Those offsets are used to compute the DfsPath and NetworkAddress string
pointers without checking whether they still point inside the response
buffer. A malformed referral can therefore make the computed pointer
exceed the end of the buffer. The resulting negative max_len is then
passed to cifs_strndup_from_utf16(), and the non-Unicode path forwards it
to kstrndup() as a size_t, allowing strnlen() to read out of bounds.
Validate each string offset before deriving the string pointer.
Fixes: 4ecce920e1 ("CIFS: move DFS response parsing out of SMB1 code")
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
In dfs_cache.c, the helper functions alloc_target(), setup_referral(),
and update_cache_entry_locked() currently utilize GFP_ATOMIC to
allocate memory.
However, all of these functions are executed in sleepable conditions.
Use GFP_KERNEL instead, to reduce the risk of allocation failure and
stop putting unnecessary pressure on emergency memory pools in
low-memory scenarios.
Signed-off-by: Fredric Cover <fredric.cover.lkernel@gmail.com>
Acked-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
smb2_check_message() has a long-standing quirk that accepts a response
whose calculated length is one byte larger than the bytes actually
received ("server can return one byte more due to implied bcc[0]").
This was introduced to accommodate servers that omit the trailing bcc[0]
overlap byte when no data area is present.
However, the exemption is applied unconditionally, regardless of whether
the command actually carries a data area (has_smb2_data_area[]). When a
response with a data area is subject to the +1 exemption, the reported
data can extend one byte beyond the bytes actually received, yet
smb2_check_message() still accepts it. The subsequent decoder then reads
past the end of the receive buffer. This is reachable during NEGOTIATE
and SESSION_SETUP, before the session is established.
The resulting out-of-bounds reads are visible under KASAN when mounting
against a non-conforming server; both the SPNEGO/negTokenInit and the
NTLMSSP challenge decoders are affected:
BUG: KASAN: slab-out-of-bounds in asn1_ber_decoder+0x16a7/0x1b00
Read of size 1 at addr ffff8880084d67c0 by task mount.cifs/81
CPU: 1 UID: 0 PID: 81 Comm: mount.cifs Not tainted 7.1.0-rc6 #1
Call Trace:
<TASK>
dump_stack_lvl+0x4e/0x70
print_report+0x157/0x4c9
kasan_report+0xce/0x100
asn1_ber_decoder+0x16a7/0x1b00
decode_negTokenInit+0x19/0x30
SMB2_negotiate+0x31d9/0x4c90
cifs_negotiate_protocol+0x1f2/0x3f0
cifs_get_smb_ses+0x93f/0x17e0
cifs_mount_get_session+0x7f/0x3a0
cifs_mount+0xb4/0xcf0
cifs_smb3_do_mount+0x23a/0x1500
smb3_get_tree+0x3b0/0x630
vfs_get_tree+0x82/0x2d0
fc_mount+0x10/0x1b0
path_mount+0x50d/0x1de0
__x64_sys_mount+0x20b/0x270
do_syscall_64+0xee/0x590
entry_SYSCALL_64_after_hwframe+0x77/0x7f
</TASK>
Allocated by task 85:
kmem_cache_alloc_noprof+0x106/0x380
mempool_alloc_noprof+0x116/0x1e0
cifs_small_buf_get+0x31/0x80
allocate_buffers+0x10d/0x2b0
cifs_demultiplex_thread+0x1d5/0x1d50
kthread+0x2c6/0x390
ret_from_fork+0x36e/0x5a0
ret_from_fork_asm+0x1a/0x30
The buggy address is located 0 bytes to the right of
allocated 448-byte region [ffff8880084d6600, ffff8880084d67c0)
which belongs to the cache cifs_small_rq of size 448
BUG: KASAN: slab-out-of-bounds in kmemdup_noprof+0x36/0x50
Read of size 329 at addr ffff88800726c678 by task mount.cifs/89
CPU: 0 UID: 0 PID: 89 Comm: mount.cifs Tainted: G B 7.1.0-rc6 #1
Call Trace:
<TASK>
dump_stack_lvl+0x4e/0x70
print_report+0x157/0x4c9
kasan_report+0xce/0x100
kasan_check_range+0x10f/0x1e0
__asan_memcpy+0x23/0x60
kmemdup_noprof+0x36/0x50
decode_ntlmssp_challenge+0x457/0x680
SMB2_sess_auth_rawntlmssp_negotiate+0x6f0/0xcb0
SMB2_sess_setup+0x219/0x4f0
cifs_setup_session+0x248/0xaf0
cifs_get_smb_ses+0xf79/0x17e0
cifs_mount_get_session+0x7f/0x3a0
cifs_mount+0xb4/0xcf0
cifs_smb3_do_mount+0x23a/0x1500
smb3_get_tree+0x3b0/0x630
vfs_get_tree+0x82/0x2d0
fc_mount+0x10/0x1b0
path_mount+0x50d/0x1de0
__x64_sys_mount+0x20b/0x270
do_syscall_64+0xee/0x590
entry_SYSCALL_64_after_hwframe+0x77/0x7f
</TASK>
Allocated by task 93:
kmem_cache_alloc_noprof+0x106/0x380
mempool_alloc_noprof+0x116/0x1e0
cifs_small_buf_get+0x31/0x80
allocate_buffers+0x10d/0x2b0
cifs_demultiplex_thread+0x1d5/0x1d50
kthread+0x2c6/0x390
ret_from_fork+0x36e/0x5a0
ret_from_fork_asm+0x1a/0x30
The buggy address is located 120 bytes inside of
allocated 448-byte region [ffff88800726c600, ffff88800726c7c0)
which belongs to the cache cifs_small_rq of size 448
Restrict the +1 exemption to responses that have no data area, so that
it still covers the bcc[0] omission it was meant for. When a data area
is present, the +1 discrepancy instead means the reported data length
overruns the received buffer, so the response must be rejected.
Fixes: 093b2bdad3 ("CIFS: Make demultiplex_thread work with SMB2 code")
Cc: stable@vger.kernel.org
Signed-off-by: Shoichiro Miyamoto <shoichiro.miyamoto@gmail.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
When creating a native SMB symbolic link (CIFS_SYMLINK_TYPE_NATIVE) whose
target is an absolute path on a mount that uses POSIX paths, the leading
path separator was silently dropped from the stored symlink target.
create_native_symlink() converted the target to UTF-16 with
cifs_convert_path_to_utf16(). That helper was intended for share-relative
SMB paths and therefore unconditionally strips a leading path separator.
For an absolute POSIX symlink target the leading '/' is significant, so a
target of "/foo/bar" was stored and read back as "foo/bar", even
though the reparse point was still flagged as absolute
(SYMLINK_FLAG_RELATIVE cleared).
On a POSIX paths mount the symlink target is stored verbatim, so convert
it directly with cifs_strndup_to_utf16() instead. This preserves the
leading separator, avoids the leading-backslash stripping that
cifs_convert_path_to_utf16() also performs (a backslash is a valid POSIX
filename character), and uses NO_MAP_UNI_RSVD to match the readback path
in smb2_parse_native_symlink(), which always converts the target with
cifs_strndup_from_utf16() / NO_MAP_UNI_RSVD. This mirrors how the NFS and
WSL reparse symlink creators convert their targets.
The NT-style absolute symlink handling, which needs the "\??\" prefix and
drive-letter colon preserved, continues to use cifs_convert_path_to_utf16()
together with the existing masking of those bytes.
Fixes: 12b466eb52 ("cifs: Fix creating and resolving absolute NT-style symlinks")
Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
Acked-by: Ralph Boehme <slow@samba.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
When an SMB3 session is referenced by a binding request on an SMB2.1
connection, the request is signed with the existing session's SMB3 signing
algorithm. ksmbd instead verifies it with the new connection's SMB2.1 HMAC
algorithm, so verification fails and the client receives
STATUS_ACCESS_DENIED instead of STATUS_REQUEST_NOT_ACCEPTED.
Select the signing verifier from the referenced session dialect. Permit a
signed SESSION_SETUP without an established channel to use the SMB3 session
signing key for verification. This is limited to SESSION_SETUP so other
unbound requests remain rejected.
The rejected response must use the same existing session algorithm. When an
SMB3 session is referenced on an SMB2.1 connection, sign the SESSION_SETUP
response with the SMB3 signing path rather than the connection's SMB2.1
path.
This fixes smb2.session.bind_negative_smb3to2s.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Binding an SMB 2.1 session to an SMB 3.x connection is invalid because the
dialects do not match. ksmbd returns STATUS_INVALID_PARAMETER. The check
fails before attaching the referenced session to the request, so the error
response lacks SMB2_FLAGS_SIGNED.
A client requiring signing checks this flag before handling the status and
reports STATUS_ACCESS_DENIED instead of STATUS_INVALID_PARAMETER. Preserve
the signed flag for a signed binding request rejected with
STATUS_INVALID_PARAMETER. The client can then apply the special error path
without attempting to validate a response using incompatible signing
algorithms.
This fixes smb2.session.bind_negative_smb2to3s.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
SMB2_SESSION_REQ_FLAG_BINDING is not supported before SMB 3.0. ksmbd maps
such a request to STATUS_REQUEST_NOT_ACCEPTED, but it rejects the request
without looking up the referenced session. The response is then sent
unsigned. A client requiring signing reports
STATUS_ACCESS_DENIED instead of the server status.
Look up the referenced session and verify the binding request with its
signing key. Keep the session reference only after successful verification
so the rejected response can be signed without providing a signing oracle.
A signed SESSION_SETUP without the binding flag can reference a session
that does not belong to the connection. Preserve SMB2_FLAGS_SIGNED on the
STATUS_USER_SESSION_DELETED response. Clients skip signature verification
for this status but still require the signed flag before propagating it.
Also restrict failed binding preauthentication cleanup to SMB 3.1.1, the
only dialect that initializes and uses that context.
This fixes smb2.session.bind_negative_smb210s.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
When an authenticated user tries to bind a channel to a session owned by a
different user, ksmbd returns STATUS_LOGON_FAILURE. Windows instead rejects
this attempt with STATUS_ACCESS_DENIED. The supplied credentials are valid
but cannot be used with the existing session.
Use a distinct internal error for a user mismatch in both NTLM and Kerberos
authentication and map it to STATUS_ACCESS_DENIED during SESSION_SETUP.
Keep ordinary authentication failures mapped to STATUS_LOGON_FAILURE.
A failed SMB 3.1.1 binding also leaves its preauthentication context on the
connection. A subsequent binding attempt for the same session reuses the
stale hash and derives an incorrect channel signing key. Remove the binding
preauthentication context on failure so a valid retry starts with a fresh
hash.
This fixes smb2.session.bind_different_user.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
A session bound to an additional connection is stored in the session
channel list, but it is not added to that connection's local session table.
After the binding exchange completes, conn->binding is cleared.
A later SESSION_SETUP reauthentication on the bound channel only searches
the local session table. It fails to find the session and returns
STATUS_USER_SESSION_DELETED instead of processing authentication and
returning STATUS_LOGON_FAILURE for invalid credentials.
If the local lookup fails, look up the session globally and accept it only
when the current connection is registered in its channel list. This keeps
unbound connections from using the session while allowing reauthentication
on an established channel.
This fixes smb2.session.bind_invalid_auth.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
When a signed request uses a session that is not registered on the
connection, ksmbd returns STATUS_USER_SESSION_DELETED before reaching the
normal response signing path. The response therefore lacks
SMB2_FLAGS_SIGNED.
Clients that require signing check this flag before handling
STATUS_USER_SESSION_DELETED and replace the server status with
STATUS_ACCESS_DENIED when it is absent. The protocol permits this error
response to skip signature verification because the connection has no
matching session key.
Preserve SMB2_FLAGS_SIGNED on the early error response when the request was
signed. This lets the client propagate STATUS_USER_SESSION_DELETED.
It fixes smb2.session.bind2.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
FILE_ALLOCATION_INFORMATION can call vfs_fallocate(). If the allocation
cannot be satisfied, vfs_fallocate() returns -ENOSPC.
smb2_set_info() did not map -ENOSPC, so ksmbd returned a generic SMB error
and the client reported EIO instead of ENOSPC. This makes the ENOSPC step
in xfstests generic/213 fail.
Map -ENOSPC and -EFBIG to STATUS_DISK_FULL in the SET_INFO error path.
Tested with xfstests generic/213 on ksmbd.
Signed-off-by: Huiwen He <hehuiwen@kylinos.cn>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Windows reports automatic write-time updates with a resolution of roughly
15 milliseconds. If a file is written and closed within that interval, a
close response requesting full information can report the write time from
the open rather than the filesystem's finer-grained mtime update.
ksmbd currently converts the filesystem mtime directly in SMB2 CLOSE, so
even a sub-millisecond write is visible to the client. This makes
smb2.timestamp_resolution.resolution1 fail because the immediate write
changes LastWriteTime.
Save the write time returned by SMB2 CREATE in the file handle. When CLOSE
requests post-query attributes, coalesce a positive mtime change smaller
than 15 milliseconds to that saved value. Larger changes remain visible,
including the test's write after a 20 millisecond delay.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
A signed multichannel SESSION_SETUP binding request can require multiple
authentication rounds. ksmbd excludes SESSION_SETUP from the signed
request check and tries to sign every binding response with the channel
signing key. The channel does not exist for
STATUS_MORE_PROCESSING_REQUIRED, so that response is sent unsigned.
Clients reject it with STATUS_ACCESS_DENIED.
The final channel signing key also needs the key exported by the binding
authentication context. Keep that key in the channel instead of
overwriting the established session key, and use the session signing key
for intermediate and failed binding responses. Retain the binding session
reference until an error response has been signed and sent.
Limit a session to 32 channels while holding the channel lock. Return
STATUS_INSUFFICIENT_RESOURCES for an additional binding, matching the
server limit expected by clients.
This fixes smb2.multichannel.generic.num_channels, which previously
failed the first binding with STATUS_ACCESS_DENIED and returned the same
status instead of STATUS_INSUFFICIENT_RESOURCES for channel 33.
Fixes: f5a544e3ba ("ksmbd: add support for SMB3 multichannel")
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
sid_to_id() currently treats the last subauthority of any owner or group
SID as a Unix uid or gid. For example, this maps Everyone (S-1-1-0) to
uid 0 and BUILTIN\Users (S-1-5-32-545) to gid 545.
When an SMB2 CREATE security descriptor contains those SIDs, ksmbd
attempts to change the newly created file to the bogus Unix ownership.
notify_change() then returns -EPERM, which makes smb2.create.aclfile fail
with NT_STATUS_SHARING_VIOLATION.
Validate the SID prefix before extracting its RID. Only server-domain
owner SIDs and S-1-22-2 Unix group SIDs have local ID representations.
Treat other valid Windows SIDs as unmapped so their original values can
still be preserved in the NT ACL xattr.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
In the `skip_invalidate:` path under `cifs_revalidate_mapping()`, the
sequence of calls:
clear_bit_unlock();
smp_mb__after_atomic();
wake_up_bit();
can be replaced exactly by `clear_and_wake_up_bit()`.
The `clear_and_wake_up_bit()` helper function was introduced in
'commit 8236b0ae31 ("bdi: wake up concurrent wb_shutdown()
callers.")' to replace equivalent instances of this sequence of
operations. This substitution has been applied in multiple subsystems.
Compile-tested with CONFIG_CIFS=y on x86_64, no new warnings present.
Suggested-by: Agatha Isabelle Moreira <code@agatha.dev>
Link: https://kernelnewbies.org/Beginner%20Cleanup%20and%20Refactor%20Tasks%20by%20Agatha%20Isabelle%20Moreira#task_010
Cc: Agatha Isabelle Moreira <code@agatha.dev>
Signed-off-by: Mehdi Hassan <mehdi.h.business@pm.me>
Signed-off-by: Steve French <stfrench@microsoft.com>
-----BEGIN PGP SIGNATURE-----
iQGzBAABCgAdFiEE6fsu8pdIjtWE/DpLiiy9cAdyT1EFAmpIIEQACgkQiiy9cAdy
T1F7/Qv+JNKEjHIO4ckW44iuX9FC435P9A0hNI41ILqPwAkPFr3nbJFlFGiHQXRr
zCwsxOUDBNtvleJczata8b6hcHR+6d00w6k5MKTZn/h59MJx21I7ukZd6RLy9saV
KtndptFrfUw1ZR+BWohVUknpEFzt8lU9Ai2V1Rq8zvb9GtckkArR5sapF38Y3Cbr
F7bu3uDlyXM9uGBDNoU58o0qA7en7FfISR6S3YC7ToZPsOUZUFMmu/7LtbUFv1WH
EMKaCdFKKKqJbBrqDC0AnLQW8N6js5yOJ2KXc1+eG1t1sBQizYf8x3yy94YECVkI
sXppz7s9aDYoWMAMD9ofDq+fBQQhI9zyRULNZIVCtLy09WDoXYoibbTWzID7nrd/
ufW7H91s9t4s+yTFKigN3vzY3A8JEqEA04TscJw7rLi/LaovcVvgLvm33A5mrxOS
kjcjoha7Rz2kac7A8KxAIHK4CCLMS7bG028sLS0vY2lBW2KrDjzKiANJqMeSuB1m
9IM0u7eK
=waBr
-----END PGP SIGNATURE-----
Merge tag 'v7.2-rc1-smb3-server-fixes' of git://git.samba.org/ksmbd
Pull smb server fixes from Steve French:
- Fix several use-after-free races in durable handle reconnect,
supersede, and oplock handling
- Avoid holding the inode oplock lock while waiting for a lease break
acknowledgement. This removes delays of up to 35 seconds when cifs.ko
closes a deferred handle in response to a lease break
- Fix malformed security descriptor handling, including an undersized
DACL allocation issue and an out-of-bounds ACE SID read
- Fix memory leaks in security descriptor and DOS attribute xattr
encoding/decoding error paths
- Fix outstanding SMB2 credit leaks on aborted requests and correct the
QUERY_INFO credit charge calculation
- Fix hard-link creation without replacement being incorrectly rejected
when the handle lacks DELETE access
- Avoid unnecessary zeroing of large SMB2 read buffers
- Add an oplock list lockdep annotation and update the documented
support status for durable handles and SMB3.1.1 compression
- Durable handle fixes to address ownership and lifetime races during
reconnect, session teardown, oplock handling, and superseding opens,
preventing stale session and file references from being used by
concurrent operations
* tag 'v7.2-rc1-smb3-server-fixes' of git://git.samba.org/ksmbd:
ksmbd: fix app-instance durable supersede session UAF
ksmbd: snapshot previous oplock state before durable checks
ksmbd: close superseded durable handles through refcount handoff
ksmbd: fix use-after-free of fp->owner.name in durable handle owner check
smb/server: do not require delete access for non-replacing links
ksmbd: don't hold ci->m_lock while waiting for a lease break ack
ksmbd: doc: update feature support status for durable handles and compression
ksmbd: annotate oplock list traversals under m_lock
ksmbd: fix outstanding credit leak on abort and error paths
ksmbd: fix credit charge calculation for SMB2 QUERY_INFO
ksmbd: avoid zeroing the read buffer in smb2_read()
ksmbd: validate num_subauth when copying ACE in set_ntacl_dacl
ksmbd: reject undersized DACLs before parsing ACEs
ksmbd: fix n.data memory leak in ksmbd_vfs_set_dos_attrib_xattr
ksmbd: Fix acl.sd_buf memory leak and invalid sd_size error handling
ksmbd: fix sd_ndr.data memory leak in ksmbd_vfs_set_sd_xattr
The server controls create-context DataOffset, so the POSIX context data
pointer may be misaligned on strict-alignment architectures. Use
get_unaligned_le32() when reading nlink, reparse_tag, and mode.
Fixes: 69dda3059e ("cifs: add SMB2_open() arg to return POSIX data")
Cc: stable@vger.kernel.org
Signed-off-by: Zihan Xi <xizh2024@lzu.edu.cn>
Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
Signed-off-by: Steve French <stfrench@microsoft.com>
posix_info_sid_size() reads sid[1] to obtain the subauthority count,
but its existing boundary check still accepts buffers with only one
remaining byte. Require two bytes before reading sid[1] so all client
paths that reuse the helper reject truncated POSIX SIDs safely.
Fixes: 349e13ad30 ("cifs: add smb2 POSIX info level")
Cc: stable@vger.kernel.org
Reported-by: Yuan Tan <yuantan098@gmail.com>
Reported-by: Yifan Wu <yifanwucs@gmail.com>
Reported-by: Juefei Pu <tomapufckgml@gmail.com>
Reported-by: Xin Liu <bird@lzu.edu.cn>
Assisted-by: Codex:gpt-5.4
Signed-off-by: Zihan Xi <xizh2024@lzu.edu.cn>
Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
Signed-off-by: Steve French <stfrench@microsoft.com>
ksmbd_close_fd_app_instance_id() looks up a prior durable handle by
AppInstanceId and closes it through opinfo->sess->file_table. This is
unsafe after the original session has been torn down. session_fd_check()
preserves reconnectable durable handles in the global table and clears
opinfo->conn/fp->conn, but opinfo->sess can still point to the freed
ksmbd_session.
Use opinfo->conn as the orphan sentinel, but make the check reliable by
serializing it with session_fd_check(). That path clears opinfo->conn
under fp->f_ci->m_lock, so hold the same lock while testing opinfo->conn
and while dereferencing opinfo->sess->file_table. Also avoid closing
through the session file table if the volatile id has already been
unpublished by session teardown.
Durable reconnect must keep the two fields consistent. Rebinding only
opinfo->conn leaves opinfo->sess pointing at the old freed session, so
a later app-instance supersede can pass the conn check and write-lock the
freed session's file table. Clear opinfo->sess when preserving a durable
handle during session teardown, and set it to the reconnecting session
when opinfo->conn is rebound in ksmbd_reopen_durable_fd().
Fixes: 16c3064970 ("ksmbd: handle durable v2 app instance id")
Reported-by: Gil Portnoy <dddhkts1@gmail.com>
Co-developed-by: Gil Portnoy <dddhkts1@gmail.com>
Signed-off-by: Gil Portnoy <dddhkts1@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
smb_grant_oplock() checks the previous oplock holder's o_fp to decide
whether a durable handle should be invalidated when the oplock break
cannot be delivered. prev_opinfo is obtained with opinfo_get_list(),
which pins only the oplock_info. It does not pin the ksmbd_file stored
in opinfo->o_fp.
A concurrent last close can unlink the opinfo from ci->m_op_list under
ci->m_lock and then free the ksmbd_file. The oplock_info can still be
kept alive by the refcount taken by opinfo_get_list(), but o_fp may
already point at freed memory by the time smb_grant_oplock() reads
is_durable, conn, or tcon.
Snapshot the previous holder's durable state while ci->m_lock is held,
then use only the copied values after dropping the lock. This keeps the
o_fp lifetime tied to the inode lock without taking an extra ksmbd_file
reference. Taking such a reference is unsafe here because smb_grant_oplock()
does not necessarily have the previous holder's session work, and dropping
the temporary reference can otherwise become the final putter.
Fixes: 26fa88dc87 ("ksmbd: invalidate durable handles on oplock break")
Reported-by: Gil Portnoy <dddhkts1@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
ksmbd_close_disconnected_durable_delete_on_close() collects disconnected
durable handles for a name being superseded by a new delete-on-close
open, drops ci->m_lock, then closes each collected handle directly with
__ksmbd_close_fd().
That bypasses the FP_CLOSED and refcount handoff used by the other close
paths. If a durable reconnect or the durable scavenger already took a
reference to the same fp, the direct __ksmbd_close_fd() can free the
ksmbd_file while that other holder still owns a live reference.
Claim the disconnected durable handle before unlinking it from m_fp_list.
While holding ci->m_lock and global_ft.lock, only take ownership when the
durable lifetime reference is the only remaining reference. Then take a
transient reference, remove the fp from global_ft, mark it FP_CLOSED, and
move it to the local dispose list. If another holder already has a
reference, leave the fp linked and let that holder complete its path.
The dispose loop then drops both references owned by the claim. This keeps
the force-close path in the same refcount handoff model as the durable
scavenger and avoids leaving a live reconnected fp detached from
m_fp_list.
Fixes: 166e4c0702 ("ksmbd: supersede disconnected delete-on-close durable handle")
Signed-off-by: Gil Portnoy <dddhkts1@gmail.com>
Co-developed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Two concurrent SMB2 durable reconnects (DH2C/DHnC) on the same
persistent_id race the fp->owner.name compare-read in
ksmbd_vfs_compare_durable_owner() against the kfree() in
ksmbd_reopen_durable_fd()'s reopen-success path. fp->owner.name is a
standalone kstrdup() buffer whose lifetime is independent of the fp
refcount, and the two sites share no lock: the compare reads the buffer
while the reopen frees it, so the strcmp() can dereference freed memory.
Commit 7ce4fc4001 ("ksmbd: fix durable reconnect double-bind race in
ksmbd_reopen_durable_fd") made the fp->conn claim atomic under
global_ft.lock (closing the owner.name double-free and the ksmbd_file
write-UAF), but the compare-read versus reopen-free pair was left
unserialized.
BUG: KASAN: slab-use-after-free in strcmp+0x2c/0x80
Read of size 1 by task kworker
strcmp
ksmbd_vfs_compare_durable_owner
smb2_check_durable_oplock
smb2_open
Freed by task kworker:
kfree
ksmbd_reopen_durable_fd
smb2_open
Allocated by task kworker:
kstrdup
session_fd_check
smb2_session_logoff
The buggy address belongs to the cache kmalloc-8
Serialize both sides of the race with fp->f_lock. The global durable
file-table lock still protects the durable reconnect claim, but
fp->owner.name is per-open state and does not need to block unrelated
durable table lookups or reconnects. The teardown is left at its
existing location after the reopen-success point so that an __open_id()
rollback still retains owner.name for a later legitimate reconnect to
verify.
Fixes: 49110a8ce6 ("ksmbd: validate owner of durable handle on reconnect")
Assisted-by: Henry (Claude):claude-opus-4
Signed-off-by: Gil Portnoy <dddhkts1@gmail.com>
Co-developed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
When a cifs.ko client caches a read-handle (RH) lease via deferred close
and a conflicting open arrives, ksmbd breaks the lease and waits for the
acknowledgment in wait_for_break_ack() for up to OPLOCK_WAIT_TIME (35s).
__smb_break_all_levII_oplock() runs that wait while holding ci->m_lock
for read.
cifs.ko reacts to a handle-lease break by closing the deferred handle
rather than sending a lease break acknowledgment. That close path
(close_id_del_oplock() -> opinfo_del()) takes ci->m_lock for write and
is exactly what would wake the waiter, but it blocks on the read lock
held by the waiting thread. The break is then resolved only by the 35s
timeout, so xfstests generic/001 takes ~78s with leases enabled versus
~4s with oplocks only.
Collect the target opinfos (each pinned with a reference) while holding
ci->m_lock, then break them after releasing it, matching how
smb_grant_oplock() already breaks a conflicting lease using only a
reference. The reference keeps the opinfo (and its conn and lease)
alive across the unlocked window, and a close racing the break is
handled by the existing OPLOCK_CLOSING state check. Apply the same fix
to the parent lease break paths.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
session_fd_check() and ksmbd_reopen_durable_fd() walk ci->m_op_list with
list_for_each_entry_rcu() while holding ci->m_lock for write. That is
the local inode/oplock serializer, but the RCU-list iterator does not
currently tell lockdep about it.
Pass lockdep_is_held(&ci->m_lock) to these iterators so
CONFIG_PROVE_RCU_LIST can see the rwsem protection already in place.
This was found by our static analysis tool and then manually reviewed
against the current tree. The dynamic triage evidence is a
target-matched CONFIG_PROVE_RCU_LIST warning; the change is limited
to documenting the existing protection contract.
This is a lockdep annotation cleanup. It does not change oplock list
lifetime or durable-handle behavior.
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
smb2_validate_credit_charge() adds the request's CreditCharge to
conn->outstanding_credits when an SMB2 PDU is received, and
smb2_set_rsp_credits() subtracts it again when the response is built.
However smb2_set_rsp_credits() only runs on the normal response path:
- __process_request() returning SERVER_HANDLER_ABORT (unimplemented
command, command index out of range, signature check failure, or a
handler that sets send_no_response such as a cancelled blocking
lock) breaks out of the processing loop before set_rsp_credits() is
called;
- smb2_set_rsp_credits() itself returns early with -EINVAL (total
credit overflow or insufficient credits) before the subtraction.
On all of these paths the charge added at receive time is never
returned, so conn->outstanding_credits only grows. Because a client can
repeatedly trigger them (e.g. by sending unimplemented commands or by
issuing and cancelling blocking locks), outstanding_credits eventually
reaches total_credits and smb2_validate_credit_charge() then rejects
every subsequent request, wedging the connection.
Record the charge that was added in work->credit_charge and release any
charge still pending at the single send. exit point of
__handle_ksmbd_work(), which all abort and error paths fall through to.
smb2_set_rsp_credits() clears work->credit_charge once it has returned
the charge so the response path is unchanged and the credit is never
released twice. Paths that never charged a credit (no multi-credit
support, validation failure) leave work->credit_charge at zero and are
unaffected.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
smb2_validate_credit_charge() computes the credit charge a request is
allowed to consume from the payload size:
CreditCharge = (max(SendPayloadSize, ResponsePayloadSize) - 1)/65536 + 1
For SMB2 QUERY_INFO, the server must validate CreditCharge based on the
*maximum* of InputBufferLength and OutputBufferLength. ksmbd instead
summed the two lengths, which overestimates the required charge.
As a result a single-credit QUERY_INFO whose InputBufferLength and
OutputBufferLength each fit in 64KB but whose sum exceeds 64KB is
rejected with STATUS_INVALID_PARAMETER, even though it is a valid
request. IOCTL already uses max() of the request and response sizes;
make QUERY_INFO consistent by feeding InputBufferLength as the request
length and OutputBufferLength as the expected response length so that
smb2_validate_credit_charge() takes their maximum.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
smb2_read() allocates the read payload buffer with kvzalloc(), zeroing up
to max_read_size bytes (1MB or more with multichannel) on every read,
only to immediately overwrite the region with file data via kernel_read().
The zero-fill is pure overhead: ksmbd_vfs_read() returns the number of
bytes actually read ('nbytes'), and only those nbytes are ever consumed -
they are pinned into the response iov (ksmbd_iov_pin_rsp_read()), sent
over the RDMA channel (smb2_read_rdma_channel()), or copied by the
compression path (ksmbd_compress_response() uses iov_len == nbytes). The
ALIGN(length, 8) tail padding and any short-read remainder are never read
or transmitted, so they need not be initialized.
Use kvmalloc() instead to skip the redundant zeroing. This reduces CPU
and memory-bandwidth usage on large sequential reads.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
set_ntacl_dacl() copies each ACE from the attacker-controlled stored
security descriptor verbatim into the response DACL without checking
sid.num_subauth. The ACE bytes (including an unchecked num_subauth)
originate from an authenticated SMB2_SET_INFO(SecInfo=DACL) that is
stored raw via ksmbd_vfs_set_sd_xattr(); parse_dacl() rejects a bad ACE
with `break` rather than an error, so parse_sec_desc() still returns
success and the malformed SD reaches the xattr intact.
On a subsequent SMB2_QUERY_INFO(SecInfo=DACL) for an inode carrying a
POSIX access ACL, build_sec_desc() -> set_ntacl_dacl() ->
set_posix_acl_entries_dacl() walks the copied ACEs and reads
ntace->sid.sub_auth[ntace->sid.num_subauth - 1]
with num_subauth taken straight from the stored SD. Since sub_auth[]
is fixed at SID_MAX_SUB_AUTHORITIES (15), a crafted num_subauth (e.g.
255) drives an out-of-bounds heap read of ~1 KB with an offset fully
controlled by an authenticated client.
The sibling functions already gate this field:
parse_dacl() -- num_subauth == 0 || > SID_MAX_SUB_AUTHORITIES
parse_sid() -- num_subauth > SID_MAX_SUB_AUTHORITIES
smb_copy_sid() -- min_t(u8, num_subauth, SID_MAX_SUB_AUTHORITIES)
set_ntacl_dacl() is the lone inconsistent path that omits the check.
Add the same num_subauth validation in set_ntacl_dacl() before copying
the ACE, matching the gate already enforced by parse_dacl().
Signed-off-by: Haofeng Li <lihaofeng@kylinos.cn>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Suggested-by: Namjae Jeon <linkinjeon@kernel.org>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
parse_dacl() limits the attacker-controlled ACE count by comparing it
with the number of minimal ACEs that fit in the DACL size. The DACL size
field is 16 bits, but the expression subtracts sizeof(struct smb_acl).
Because sizeof() is unsigned, a DACL size smaller than the ACL header
underflows to a large size_t.
A malicious client can reach this with:
SMB2_SET_INFO (InfoType=SMB2_O_INFO_SECURITY)
-> smb2_set_info_sec()
-> set_info_sec()
-> parse_sec_desc()
-> parse_dacl()
-> init_acl_state(..., 0xffff)
-> init_acl_state(..., 0xffff)
-> kmalloc_objs(..., 0xffff)
Thus a malformed security descriptor can make num_aces pass the guard
and drive large temporary ACL state and pointer-array allocations.
Reject DACLs smaller than struct smb_acl before doing the subtraction,
so the ACE count check cannot be bypassed by the underflow.
Fixes: e2f34481b2 ("cifsd: add server-side procedures for SMB3")
Signed-off-by: Haofeng Li <lihaofeng@kylinos.cn>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Free ndr buffer data when ndr_encode_dos_attr() returns error
to avoid memory leak.
Signed-off-by: Qiang Liu <liuqiang@kylinos.cn>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
1. When ndr_decode_v4_ntacl() fails, the code jumped to free_n_data
which only freed n.data, skipping kfree(acl.sd_buf) and leaking
the buffer. Zero-initialize struct xattr_ntacl acl, reorder error
labels to out_free to release acl.sd_buf on all error paths.
2. if (acl.sd_size < sizeof(struct smb_ntsd)) is true, original code
returned success without freeing sd_buf and left stale *pntsd.
Set rc = -EINVAL before jumping to out_free to return error code and
free buffer.
Signed-off-by: Qiang Liu <liuqiang@kylinos.cn>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
ndr_encode_v4_ntacl() allocates sd_ndr.data via kzalloc() at entry.
If any subsequent ndr_write_*() call returns error during encoding,
the allocated sd_ndr.data won't be freed and causes memory leak.
Move kfree(sd_ndr.data) into out label to ensure the buffer gets
released on all success and error return paths.
Signed-off-by: Qiang Liu <liuqiang@kylinos.cn>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
-----BEGIN PGP SIGNATURE-----
iQGzBAABCgAdFiEE6fsu8pdIjtWE/DpLiiy9cAdyT1EFAmo9nx0ACgkQiiy9cAdy
T1GiNgwAuxlhhJghBrXqNqzW7FwJRsrC+ibEfCQDdI5ifREo+6Ov6I9FBEu5z/XX
OAXFJWRHYHzZEc7XqpjXOqIXRq9GN3ADA2bwQ0fsh94TazmIeYXEDcMDnutQOECM
rqm+5kO+mY+c76XPm1C1CcNi+Te3q7Ry4O6rY7JhgGn5agZPyEEVHQ2nhRbS4YCp
LQESWWd169CIeHkBJCUnPbLPPaC56pkkHaEKgoPuZk0lbSTMOeQrmWInxX2lXBNQ
udO79m7Fz2XrGy0vjBwBFfOaWdBd2Gk1b76Hc/3EwfVVm2NN8w6ge0L4m+1RglDz
3VQSBsJ3nNVNRxSLpfjRwAE+pFvIy7+XmHA4lJ0REeklpyVF+27i/QvqPuCovm/g
AfMbrExIcDt+96DEe+h5iJtbP7WiuCIsYWtvpUtnDUdz4aTrmRlJhzaDvz88vmDi
5GRmLm13oGoPN3UTF3PQDgFG4Jigs+1YlKrbeIBOXkIbcYBslIDIIxN0b82i2GRT
6osoVCRX
=/FfE
-----END PGP SIGNATURE-----
Merge tag 'v7.2-rc-part2-smb3-server-fixes' of git://git.samba.org/ksmbd
Pull smb server updates from Steve French:
"This is mostly a correctness and compatibility update for ksmbd's
SMB2/3 lease, oplock, durable handle, compound request, CREATE,
rename, stream and share-mode handling.
A large part of the series fixes cases found by smbtorture where ksmbd
diverged from the SMB2/3 protocol requirements.
The main changes are:
- Rework SMB2 lease state handling so lease state is shared per
ClientGuid/LeaseKey across opens, with better validation of lease
create contexts, ACK handling, epochs, break-in-progress reporting,
v2 lease notification routing, and chained lease breaks
- Fix several oplock break corner cases, including ACK validation,
timeout downgrade behavior, level-II break handling on unlink,
share-conflict lease breaks, and read-control/stat-open behavior
- Fix durable handle behavior around delete-on-close, stale
reconnects, reconnect context parsing, oplock/lease break
invalidation, and durable v2 AppInstanceId replacement
- Fix compound request handling so related commands propagate failed
statuses correctly, preserve response framing across chained
errors, keep compound FIDs across READ/WRITE/FLUSH, and send
interim STATUS_PENDING where clients expect cancellable compound
I/O
- Tighten CREATE and stream semantics, including create attribute
validation, allocation size reporting, explicit create security
descriptors, unnamed DATA stream handling, stream directory
validation, and stream delete sharing against the base file
- Fix rename and metadata behavior, including parent directory
sharing checks, denying directory rename with open children, and
preserving SMB ChangeTime across rename for open handles
- Fix two important safety issues: a multichannel byte-range lock
list owner race that could lead to use-after-free, and an NTLMv2
session key update before authentication proof validation
- Fix a concurrent SMB2 NEGOTIATE preauth use-after-free, a UBSAN
warning in compression capability parsing, a false hung-task
warning in the durable handle scavenger, endian debug logging,
Smatch indentation warnings, and kernel-doc warnings
- Increase the default SMB3 transaction size from 1MB to 4MB to
better match modern read/write negotiation and improve sequential
I/O behavior"
* tag 'v7.2-rc-part2-smb3-server-fixes' of git://git.samba.org/ksmbd: (50 commits)
ksmbd: fix kernel-doc warnings in smb2_lease_break_noti()
ksmbd: fix inconsistent indenting warnings
ksmbd: validate NTLMv2 response before updating session key
ksmbd: increase SMB3_DEFAULT_TRANS_SIZE from 1MB to 4MB
ksmbd: fix UBSAN array-index-out-of-bounds in decode_compress_ctxt()
ksmbd: sleep interruptibly in the durable handle scavenger
ksmbd: start file id allocation at 1
ksmbd: treat read-control opens as stat opens only for leases
ksmbd: validate :: stream type against directory create
ksmbd: break conflicting-open leases only as far as needed
ksmbd: break handle caching for share conflicts
ksmbd: normalize ungrantable lease states
ksmbd: return oplock protocol error for level II ack
ksmbd: avoid level II oplock break notification on unlink
ksmbd: downgrade oplock after break timeout
ksmbd: apply create security descriptor first
ksmbd: return requested create allocation size
ksmbd: tighten create file attribute validation
ksmbd: reject empty-attribute synchronize-only create
ksmbd: honor stream delete sharing for base file
...
