| Age | Commit message (Collapse) | Author |
|---|
|
Pull smb client fixes from Paulo Alcantara:
- File type corruption fixes in reparse point handling: setting S_IFMT
bits without clearing the existing type first corrupted the file mode
(e.g. S_IFREG | S_IFCHR == S_IFLNK). Fixed in the WSL, POSIX and
native symlink reparse parsers. Also fixes an uninitialized SID
structure in the POSIX readdir path when parsing fails.
- Ownership mapping fixes: forceuid/forcegid mount options were
ignored in several code paths (SID-to-id mapping, WSL extended
attributes, POSIX extensions getattr), allowing an untrusted server
to dictate local file ownership despite explicit mount overrides.
- Heap overflow and overflow fixes in DACL rewriting: replacing short
SIDs with long ones could overflow the DACL buffer, and the u16
accumulator for DACL size could wrap around with enough ACEs.
- Reference count leak fixes in oplock break and deferred close:
duplicate oplock breaks on a queued work item leaked a
cifsFileInfo reference, and deferred close had a similar leak when
requeueing a running work item. Both cause busy-inode oopses on
unmount.
- DFS superblock use-after-free fix: the iterator callback stored a
raw superblock pointer without pinning it, racing with automount
expiry.
- One-byte slab OOB read in the native symlink parser when handling
share-root relative paths.
- Hardening of legacy SMB1 input: reject userspace-crafted
cifs.idmap key descriptions that bypass kernel origin checks, and
validate DataOffset in CIFSSMBRead() to prevent heap info
disclosure from a malicious server.
- DFS cache fix: defer metadata updates until target copying
succeeds to prevent partial-state cache entries on allocation
failure.
* tag 'cifs-fixes-7.3-rc3' of https://git.manguebit.org/linux:
smb: client: fix one-byte OOB read in smb2_parse_native_symlink()
smb: client: fail DACL rewrite when the new DACL exceeds 64K
smb: client: fix heap overflow in DACL owner/group rewrite
smb: client: fix file type corruption in cifs_reparse_point_to_fattr()
smb: client: fix file type corruption in posix_reparse_to_fattr()
smb: client: fix file type corruption in wsl_to_fattr()
smb: client: avoid using uninitialized SIDs in cifs_posix_to_fattr()
smb: client: fix WSL reparse point uid/gid override
smb: client: honor forceuid/forcegid when mapping SIDs to uid/gid
smb: client: fix uid/gid override in getattr with posix extensions
smb: client: fix cifsFileInfo reference leak in deferred close
smb: client: avoid leaking refcount when cifs_sb_tlink() fails
smb: client: avoid leaking refcount in cifs_queue_oplock_break()
smb: client: fill cache fields after populating cache in copy_ref_data()
smb: client: pin DFS superblock in iterator callback
smb: client: reject userspace cifs.idmap descriptions
smb: client: reject out-of-bounds DataOffset in CIFSSMBRead()
smb: client: reject short READ responses in CIFSSMBRead()
|
|
When parsing a share-root relative native symlink, memcpy copies
smb_target+1 (skipping the leading separator) but uses
strlen(smb_target)+1 as the length, reading one byte past the
allocated buffer.
This fixes the following KASAN splat when accessing an SMB symlink
with a target of '\a\b':
BUG: KASAN: slab-out-of-bounds in smb2_parse_native_symlink+0x4f5/0xca0
Read of size 5 at addr ffff88800878fe21 by task netfsfuzz-execu/1
CPU: 1 UID: 0 PID: 1 Comm: netfsfuzz-execu Tainted: G N
7.2.0-11943-g2709dd5ae32f-dirty #1 PREEMPT(lazy)
Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix,
1996)
Call Trace:
<TASK>
dump_stack_lvl+0x7b/0xa0
print_report+0xd0/0x630
kasan_report+0xe5/0x120
kasan_check_range+0x105/0x1b0
__asan_memcpy+0x23/0x60
smb2_parse_native_symlink+0x4f5/0xca0
parse_reparse_point+0x68a/0x1530
reparse_info_to_fattr+0x752/0xa20
cifs_get_fattr+0x873/0x15b0
cifs_get_inode_info+0xc0/0x310
cifs_lookup+0x308/0xa70
__lookup_slow+0x122/0x2b0
lookup_slow+0x50/0x70
path_lookupat+0x525/0xaf0
filename_lookup+0x1f2/0x550
vfs_statx+0xd1/0x1a0
vfs_fstatat+0x65/0xc0
__do_sys_newfstatat+0x9a/0x120
do_syscall_64+0xdd/0x4a0
entry_SYSCALL_64_after_hwframe+0x77/0x7f
Reported-by: Yuanfu Xie <yuanfuxie@stu.pku.edu.cn>
Fixes: 723f4ef90452 ("cifs: Fix parsing native symlinks relative to the export")
Suggested-by: Pali Rohar <pali@kernel.org>
Reviewed-by: Pali Rohar <pali@kernel.org>
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
Cc: stable@vger.kernel.org
|
|
replace_sids_and_copy_aces() and set_chmod_dacl() accumulate the size of
the DACL they build in a u16. That accumulator can wrap.
validate_dacl() caps num_aces at (dacl_size - sizeof(struct smb_acl)) /
20, i.e. 3276 for a maximally sized DACL, while each rewritten ACE can
grow to sizeof(struct smb_ace) (76 bytes) once its SID is replaced with
one carrying SID_MAX_SUB_AUTHORITIES sub-authorities. The worst case is
therefore sizeof(struct smb_acl) + 3276 * 76 = 248984 bytes, far beyond
what a u16 can hold. A wraparound is reached with 863 ACEs.
After the wraparound, ndacl_ptr->size becomes meaningless and the offset
will point anywhere in the ACE array. As a result, we will see
corruption of the DACL, which then gets sent to the server. This is not
an out-of-bounds write as the allocation now covers the worst-case
expansion, so writes will always go into the buffer.
Adjust the code to use a u32 internally and return -EOVERFLOW in the
overflow case. The operation must be refused, because a DACL can only
hold 2^16-1 bytes on the wire and larger DACLs cannot be represented.
set_chmod_dacl() carries the same pattern and is fixed the same way. It
only wraps once the source DACL comes within roughly 380 bytes of the
64K ceiling, but the failure mode is identical.
Suggested-by: Namjae Jeon <linkinjeon@kernel.org>
Cc: stable@vger.kernel.org
Fixes: f5065508897a ("cifs: Retain old ACEs when converting between mode bits and ACL.")
Assisted-by: Kiro:claude-opus-5
Signed-off-by: Bjoern Doebel <doebel@amazon.de>
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
When id_mode_to_cifs_acl rewrites an existing DACL, it allocates a
buffer sized according to the on-disk DACL length reported by
dacl_ptr->size. However, replace_sids_and_copy_aces may rewrite each
ACE with a new owner/group SID obtained from the cifs.idmap upcall.
Those SIDs can have up to SID_MAX_SUB_AUTHORITIES (15) sub-authorities,
making each ACE up to 76 bytes (sizeof(struct smb_ace)).
If the original DACL contains short SIDs (e.g., 1 sub-authority) while
the replacement SIDs are long, the rewritten ACEs overflow the
allocation.
Fix this by always budgeting for worst-case SID expansion: allocate
sizeof(struct smb_acl) plus num_aces * sizeof(struct smb_ace), which
covers the smb_acl header and room for every ACE at maximum SID size.
This replaces the previous split logic that used dacl_ptr->size for
cifsacl mounts but num_aces * sizeof(struct smb_ace) for mode_from_sid
mounts: both paths can trigger the same rewrite and need the same
headroom.
KASAN reports this as:
BUG: KASAN: slab-out-of-bounds in build_sec_desc+0x1e8a/0x2680 [cifs]
Write of size 4 at addr ffff8881a5e25374 by task chown/5298
...
The buggy address is located 0 bytes to the right of
allocated 884-byte region [ffff8881a5e25000, ffff8881a5e25374)
Cc: stable@vger.kernel.org
Fixes: bc3e9dd9d104 ("cifs: Change SIDs in ACEs while transferring file ownership.")
Assisted-by: Kiro:claude-opus-4.6
Signed-off-by: Bjoern Doebel <doebel@amazon.de>
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Fixes: 5c3564852c58 ("cifs: Minimize the number of cifs_acl memory allocations")
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
Setting the file type in cf_mode without clearing the existing S_IFMT
bits first is wrong as it corrupts the file type when cf_mode already
has type bits set (e.g. S_IFREG | S_IFLNK == S_IFDIR | S_IFREG).
Clear S_IFMT before setting S_IFLNK for native and SMB1 symlinks.
Closes: https://sashiko.dev/#/patchset/20260906181540.647469-1-pc%40manguebit.org
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
Cc: stable@vger.kernel.org
|
|
Setting the file type in cf_mode without clearing the existing S_IFMT
bits first is wrong as it corrupts the file type when cf_mode already
has type bits set (e.g. S_IFREG | S_IFCHR == S_IFLNK).
Use a local ftype variable to collect the new file type and apply it
after validation succeeds, clearing S_IFMT and setting the new type in
a single assignment. This avoids stripping cf_mode on malformed
reparse points where the function returns false early.
Closes: https://sashiko.dev/#/patchset/20260906172005.627163-1-pc%40manguebit.org
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
Cc: stable@vger.kernel.org
|
|
Setting the file type in cf_mode without clearing the existing S_IFMT
bits first is wrong as it corrupts the file type when cf_mode already
has type bits set (e.g. S_IFREG | S_IFCHR == S_IFLNK).
Clear S_IFMT before the switch statement.
Closes: https://sashiko.dev/#/patchset/20260906172005.627163-1-pc%40manguebit.org
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
Cc: stable@vger.kernel.org
|
|
cifs_posix_to_fattr() ignores the return value of posix_info_parse().
When a malformed POSIX directory entry is encountered (e.g. invalid
SID lengths from an untrusted server), posix_info_parse() returns -1
without populating the 'parsed' struct. The uninitialized stack
memory in parsed.owner and parsed.group is then passed to
sid_to_id(), which processes the garbage bytes and passes them to
request_key() to construct a SID string, potentially leaking kernel
stack contents to the userspace idmap daemon.
Fix this by checking the return value and skipping the SID-to-id
mapping when parsing fails. The remaining fattr fields (timestamps,
mode, etc.) are populated directly from the 'info' pointer so they
are unaffected.
Closes: https://sashiko.dev/#/patchset/20260906172005.627163-1-pc%40manguebit.org
Closes: https://sashiko.dev/#/patchset/20260906181540.647469-1-pc%40manguebit.org
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
Cc: stable@vger.kernel.org
|
|
wsl_to_fattr() unconditionally overwrites cf_uid/cf_gid with values
from WSL extended attributes ($LXUID/$LXGID), ignoring the forceuid
and forcegid mount options.
Fix this by initializing cf_uid/cf_gid to the mount defaults and
gating the $LXUID/$LXGID EA parsing on forceuid/forcegid.
Closes: https://sashiko.dev/#/patchset/20260906190803.667489-1-pc%40manguebit.org
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
Cc: stable@vger.kernel.org
|
|
When the administrator mounts with forceuid or forcegid (uid=/gid=
mount options), they expect all files to appear owned by the specified
user/group. However, several code paths unconditionally called
sid_to_id() to overwrite cf_uid/cf_gid with server-provided values,
ignoring the administrator's explicit override:
- smb311_posix_info_to_fattr() (stat via POSIX extensions)
- cifs_posix_to_fattr() (readdir via POSIX extensions)
- parse_sec_desc() (CIFS ACL ownership mapping)
This allowed an untrusted server to dictate local file ownership even
when the mount was configured to force specific uid/gid values.
Fix all three call sites to check CIFS_MOUNT_OVERR_UID and
CIFS_MOUNT_OVERR_GID before calling sid_to_id(), following the
same pattern already used by cifs_unix_basic_to_fattr() for unix
extensions.
Closes: https://sashiko.dev/#/patchset/20260906155816.603278-1-pc%40manguebit.org
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
Cc: stable@vger.kernel.org
|
|
When mounting with 'multiuser,posix' options, cifs_getattr() overrides
the server-provided uid/gid with the current process's fsuid/fsgid.
This is because the condition only checks for unix extensions
(tcon->unix_ext) but not posix extensions (tcon->posix_extensions).
With SMB3 POSIX extensions, the server provides real uid/gid values
just like with unix extensions, so they should be preserved rather
than replaced with the caller's credentials.
Add a tcon->posix_extensions check to the condition so that uid/gid
from the server are properly reported in stat results.
Reported-by: Arthur Lesuisse <arthur.lesuisse@ulb.be>
Closes: https://lore.kernel.org/r/DB9P190MB2012266F6B8DECBE5D26A1798DB52@DB9P190MB2012.EURP190.PROD.OUTLOOK.COM
Suggested-by: Arthur Lesuisse <arthur.lesuisse@ulb.be>
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
Cc: stable@vger.kernel.org
|
|
When cifs_close() defers a close, it hands the cifsFileInfo reference
of the closing struct file to the queued work. Each execution of
smb2_deferred_work_close() drops one such reference.
deferred_close_scheduled can be false while the work is pending: the
workqueue clears PENDING when the callback starts to run, before the
callback clears the flag under deferred_lock. A close in that
interval requeues the running work, and the callback then clears the
flag, leaving the requeued work pending with the flag down. A later
cifs_open() can reuse the handle and its cifs_close() reaches the
same branch: queue_delayed_work() fails because the work is still
pending, but cifs_close() returns without dropping the closing file's
reference. The cifsFileInfo count stays pinned and its tlink, dentry
and server handle are leaked.
Check the return value and hand off the reference only when work was
actually queued. Otherwise, use the shared _cifsFileInfo_put(), like
the mod_delayed_work() branch above: the pending execution already
owns its reference.
This issue was found by an in-house static analysis tool.
Fixes: c3f207ab29f7 ("cifs: Deferred close for files")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.6
Co-developed-by: Song Li <songl@zju.edu.cn>
Signed-off-by: Song Li <songl@zju.edu.cn>
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
cifs_oplock_break() takes over the reference that
cifs_queue_oplock_break() acquired when it queued the work, and drops it
with _cifsFileInfo_put() once the break has been processed.
Only in setups with "-o multiuser", cifs_sb_tlink() may fail, at which
point cifs_oplock_break() returns without putting the file reference,
mirroring the reference leak we already fixed in the companion patch to
cifs_queue_oplock_break().
This would trigger a crash due to busy inodes on the next unmount:
BUG: Dentry ... still in use (1) [unmount of cifs cifs]
VFS: Busy inodes after unmount of cifs (cifs)
Drop the reference on that path as well. Doing so before the out label
mirrors the normal path, which also puts the reference before
cifs_done_oplock_break().
Found by Sashiko code review. The failure path was not exercised at
runtime.
Fixes: e8f5f849ffce2 ("cifs: fix potential oops in cifs_oplock_break")
Cc: stable@vger.kernel.org
Assisted-by: Kiro:claude-opus-5
Signed-off-by: Bjoern Doebel <doebel@amazon.de>
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
cifs_queue_oplock_break() unconditionally takes a reference on the
target file before queueing cifs_oplock_break(). Only that work item
decreases the reference counter again.
If another oplock break arrives while that work is still queued,
queue_work() will return false and not queue this second work item. As a
result, we will never reach the point to drop the file reference again
and are leaking this reference. This can be triggered when interacting
with a slow-responding server.
As a result, later unmount operations for this file system will fail with
BUG: Dentry ... still in use (1) [unmount of cifs cifs]
VFS: Busy inodes after unmount of cifs (cifs)
kernel BUG at fs/super.c:777!
Fix this by only incrementing the reference count if the work has been
queued successfully. Taking it after queue_work() is safe because all
three callers hold tcon->open_file_lock across the call and
_cifsFileInfo_put() decrements under that same lock, so a worker that
starts the handler in the window cannot drop the reference before it has
been taken.
Fixes: b98749cac4a69 ("CIFS: keep FileInfo handle live during oplock break")
Cc: stable@vger.kernel.org
Assisted-by: Kiro:claude-opus-5
Signed-off-by: Bjoern Doebel <doebel@amazon.de>
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
This is another run of the Coccinelle script for converting kmalloc()
family of allocations to kmalloc_obj() via the existing rules in
scripts/coccinelle/api/kmalloc_objs.cocci
This catches both the set of kmalloc() uses added since the first
kmalloc_obj() conversions in v7.0 and adds a large group missed in the
first pass due to Coccinelle not interacting well with the cleanup.h
scoped_...() family of macros[1]. I worked around this with spatch's
"--macro-file" argument to a file with all the scoped_...() macros mapped
to Coccinelle's YACFE_ITERATOR[2] as that was the closest viable control
flow indicator I could find.
Build tested allmodconfig on x86, arm64, arm, loongarch, mips, powerpc,
riscv, and s390 with no new warnings.
Link: https://lore.kernel.org/lkml/202609021314.8A9C0B8@keescook/ [1]
Link: https://github.com/coccinelle/coccinelle/blob/master/standard.h [2]
Signed-off-by: Kees Cook <kees+treewide@kernel.org>
|
|
In copy_ref_data(), struct cache_entry *ce has its fields populated
at the beginning of the function. Later, if alloc_target fails with
an ERR_PTR, free_tgts() is called on the cache, leaving the cache
metadata populated without any targets. Critically, this extends
ce->etime, making the cache appear valid for longer without any
targets.
Also, free_tgts() does not set ce->numtgts to zero. On error, when
the cache is freed, ce->numtgts is not zeroed, and other cache users
may attempt to access nonexistent entries.
Update fields after copying targets to prevent partial-state updates.
Set ce->numtgts to zero at the end of free_tgts().
Signed-off-by: Fredric Cover <fredric.cover.lkernel@gmail.com>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
tcon_super_cb() stores a raw superblock pointer, but __cifs_get_super()
takes its active reference only after iterate_supers_type() has dropped
s_umount and its passive reference. Concurrent DFS automount expiry can
therefore free the superblock before cifs_sb_active() uses it.
A deterministic KASAN test reproduces the race as:
BUG: KASAN: slab-use-after-free in cifs_sb_active+0x77/0x80
The same test passes with this change applied.
Take the active reference in the callback while iterate_supers_type()
still holds s_umount shared. cifs_put_tcp_super() remains the matching
release.
Fixes: bacd704a95ad ("cifs: handle prefix paths in reconnect")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
cifs.idmap key descriptions carry authority-bearing fields (owner and
group SIDs and uid/gid values in "os:"/"gs:"/"oi:"/"gi:" form) that the
cifs.idmap upcall helper treats as kernel-originating inputs. Unlike
its sibling cifs.spnego, the cifs.idmap key type has no vet_description
hook, so userspace can create keys of this type through
request_key(2)/add_key(2) and supply those fields without CIFS origin.
A request_key(2) call with a non-NULL callout then drives a root
usermodehelper upcall (/sbin/request-key -> cifs.idmap) that consumes
the unvetted description in root context.
Only accept cifs.idmap descriptions while CIFS is using its private
root_cred to request the key. id_to_sid()/sid_to_id() already run
under override_creds(root_cred), so the kernel-originated path is
unaffected.
This mirrors commit 3da1fdf4efbc ("smb: client: reject userspace
cifs.spnego descriptions"), which applied the same restriction to
cifs.spnego.
Fixes: 4d79dba0e007 ("cifs: Add idmap key and related data structures and functions (try #17 repost)")
Reported-by: TencentOS Corvus AI <corvus@tencent.com>
Cc: stable@vger.kernel.org
Assisted-by: CodeBuddy:Kimi-K3
Signed-off-by: Aohan Mei <henrymei@tencent.com>
Acked-by: David Howells <dhowells@redhat.com>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
The SMB1 synchronous read helper CIFSSMBRead() validates the server's
DataLength against CIFSMaxBufSize and the caller's count, but never
validates DataOffset. The copy source is formed as
&pSMBr->hdr.Protocol + le16_to_cpu(pSMBr->DataOffset)
and memcpy()'d for DataLength bytes with no check that the
[DataOffset, DataOffset + DataLength) range lies within the response
actually received from the server.
A malicious or compromised SMB1 server can return a response carrying
an in-range DataLength and a large DataOffset, driving the source
pointer past the end of the response buffer. The memcpy() then copies
adjacent kernel heap into the caller's read buffer (information
disclosure), or reads unmapped memory and oopses (denial of service).
SMB1 is not negotiated by default; reaching this code requires an
explicit vers=1.0 mount.
Both DataOffset and the received response length recorded in
rsp_iov.iov_len are relative to the start of the SMB header, so reject
the response unless DataOffset + DataLength fits within that length,
using overflow-safe arithmetic, before forming the source pointer.
The response length has been validated by the previous patch, so the
DataOffset and DataLength fields can be read safely here.
While here, make data_length unsigned. It holds a length derived from
unsigned on-the-wire fields and is only ever compared against unsigned
quantities; print it with %u accordingly, and add __func__ to the
cifs_dbg() calls in this function.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org # 6.19.x
Assisted-by: Bynario AI
Signed-off-by: Diego Oliva <diego@bynar.io>
Reviewed-by: David Howells <dhowells@redhat.com>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
CIFSSMBRead() reads DataLengthHigh, DataLength and DataOffset out of
the READ_RSP returned by the server without first checking that a
whole READ_RSP was actually received. The length of the response is
recorded in rsp_iov.iov_len, but nothing constrains it to be at least
read_rsp_size before those fields are dereferenced.
A malicious or compromised SMB1 server can return a response shorter
than the READ_RSP header, so that parsing the header itself reads past
the end of the receive buffer. SMB1 is not negotiated by default;
reaching this code requires an explicit vers=1.0 mount.
Reject the response unless it is at least read_rsp_size bytes long.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Suggested-by: Paulo Alcantara <pc@manguebit.org>
Cc: stable@vger.kernel.org # 6.19.x
Assisted-by: Bynario AI
Signed-off-by: Diego Oliva <diego@bynar.io>
Reviewed-by: David Howells <dhowells@redhat.com>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
ksmbd_tree_conn_connect() publishes a new tree connection in
sess->tree_conns with a single reference and returns its pointer to
smb2_tree_connect(). The handler continues to initialize the object and
build the response after publication. A concurrent session logoff can
erase the connection and drop that reference, freeing the object while
the handler still uses it.
BUG: KASAN: slab-use-after-free in smb2_tree_connect+0xe3d/0xf90
smb2_tree_connect (fs/smb/server/smb2pdu.c:2872)
handle_ksmbd_work
process_one_work
worker_thread
kthread
After xa_store() succeeds, take a second reference before releasing
tree_conns_lock. The original reference belongs to the xarray entry and
the second belongs to the creating smb2_tree_connect() handler.
Keep the references balanced in every path:
- On normal exit or an error after publication, smb2_tree_connect()
drops its creator reference. Error cleanup also calls
ksmbd_tree_conn_disconnect(), which drops the xarray reference only if
it removes the exact entry.
- SMB2 TREE_DISCONNECT uses the same helper to remove the entry and drop
its xarray reference. The request's existing lookup reference remains
owned by the request and is released by the existing cleanup.
- Session LOGOFF removes each entry and drops its xarray reference. If
it wins the race, later cleanup sees that the entry is gone and does
not drop that reference again.
To enforce this ownership, claim the disconnected state and erase the
exact entry atomically under tree_conns_lock. This guarantees one drop
for the xarray reference and one drop by each in-flight user, regardless
of which teardown path wins. If logoff removes the entry before
initialization completes, fail the connect instead of marking the
detached object TREE_CONNECTED.
Fixes: 33b235a6e6eb ("ksmbd: fix race condition between tree conn lookup and disconnect")
Reported-by: Xiang Mei (Microsoft) <xmei5@asu.edu>
Cc: AutonomousCodeSecurity@microsoft.com
Cc: stable@vger.kernel.org
Signed-off-by: Cen Zhang (Microsoft Security FORGE Labs) <cenzhang@linux.microsoft.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
ksmbd_vfs_copy_file_ranges() rejects negative source offsets in the
copy loop, but it does not validate target offsets. It also calculates
lock and overlap endpoints before ensuring that either range fits within
MAX_LFS_FILESIZE.
When the target is an alternate data stream, the buffered path passes a
negative target offset to ksmbd_vfs_stream_write(). Let n be Length and
let -d be TargetOffset, where 0 < d < n <= XATTR_SIZE_MAX. For an empty
stream, the writer allocates n - d bytes, then copies n bytes starting d
bytes before the allocation. An authenticated SMB client can control d
and the source data, overwrite kernel heap memory, and crash the host.
Validate both ranges before lock, overlap, or I/O calculations.
Fixes: 8482150a0743 ("ksmbd: support copychunk for alternate data streams")
Assisted-by: Antiproof:GPT-5.6-Sol
Signed-off-by: Alon Shakevsky <shakevsky@berkeley.edu>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
smb2_oplock_break_noti() reads opinfo->conn without any lock and
dereferences it after two allocations which may sleep. When the
durable handle owning the oplock is disconnected, session_fd_check()
clears opinfo->conn and drops its conn reference under ci->m_lock, and
the last ksmbd_conn_put() frees the connection. A break triggered by
another connection that races with the teardown can then resurrect the
freed connection: ksmbd_conn_get() is a plain atomic_inc, and the
queued break work later dereferences the stale conn via
ksmbd_conn_write(), a use-after-free reachable by any authenticated
client holding a durable batch oplock.
Thread the caller's inode into the notification path instead of taking
a new reference on it. Every caller of oplock_break() already holds a
live ksmbd_file (or an explicit ksmbd_inode_lookup_lock() reference,
in the parent lease break paths) on the inode that owns the break
target's oplock list, so ci cannot be freed during the call, and its
lock can be taken without dereferencing opinfo->o_fp, which a
concurrent close may free. Select and pin the connection under
ci->m_lock, the same lock session_fd_check() and
ksmbd_reopen_durable_fd() use to update opinfo->conn, so a concurrent
detach either loses the race to the clear or keeps the connection
alive until the notification work releases it. Transfer the reference
to the work item and release it on allocation failures.
Fixes: b003086d7696 ("ksmbd: fix NULL-deref of opinfo->conn in oplock/lease break notifiers")
Cc: stable@vger.kernel.org
Signed-off-by: Abdifatah Suruur <suruurism@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
Use an unsigned int for the work state so xchg() uses a supported
4-byte operation on sparc.
Fixes: d12168084c8c ("ksmbd: safely drain sessions during logoff")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202609021157.8f7Wx34I-lkp@intel.com/
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
Pull smb client fixes from Paulo Alcantara:
- Fixes for fallocate range operations (insert, collapse, zero, punch
hole)
The insert range implementation copied overlapping chunks in the
wrong direction, corrupting file data on every server except Windows.
Several related issues in the same area are also addressed — stale
page cache and FS-Cache readback, an integer truncation on large
files, missing RLIMIT_FSIZE validation and missing sparse file
marking.
- Data corruption fixes in the O_TRUNC open path: one where i_size was
zeroed before the server confirmed the truncate and another where the
lack of locking allowed concurrent buffered writes to be silently
discarded
- Heap overflow fixes in legacy SMB1 paths: one in extended attribute
writes and one in POSIX ACL handling, both exploitable via
unprivileged setxattr(2)
- Fix for multiuser mount with krb5 failing because the username option
was not propagated to new per-user connections
- Fix for split debug message in __release_mid() after a printk
conversion
* tag 'cifs-fixes-7.3-rc2' of https://git.manguebit.org/linux:
smb: client: reject SetEA requests that do not fit the request buffer
smb: client: fix data corruption with concurrent writes and O_TRUNC
cifs: don't update i_size in cifs_do_truncate without a cached handle
smb: client: fix heap overflow in cifs_do_set_acl()
smb: client: fix multiuser mount with krb5
smb: client: transport: Fix debug printing in __release_mid()
smb/client: invalidate fscache for fallocate range operations
smb/client: fix stale page cache in insert/collapse range
smb/client: fix integer truncation in collapse range
smb/client: fix data corruption in emulated insert range
smb/client: mark file sparse before emulating insert range
smb/client: validate new EOF for zero range
smb/client: validate new EOF for insert range
cifs: add revalidation on FSCTL failure in smb2_duplicate_extents()
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/linkinjeon/smb
Pull smb server fixes from Namjae Jeon:
- Prevent unintended data exposure by clearing pipe compound padding
and the response buffer
- Initialize missing fields in FS_OBJECT_ID_INFORMATION,
FS_CONTROL_INFORMATION, and FS_POSIX_INFORMATION
- Propagate DACL parsing and allocation failures so malformed security
descriptors are rejected
- Rate-limit errors for unmapped SIDs to prevent kernel log flooding
- Drain multichannel sessions during LOGOFF, wake deferred locks and
cancellable requests, and ensure cancellation callbacks run only once
- Fix listener kthread reference handling and teardown ordering during
netdevice events
- Validate normalized-name and IPC share configuration response lengths
- Update the KSMBD MAINTAINERS entry and add Paulo Alcantara as an
SMBDIRECT co-maintainer
* tag 'ksmbd-for-7.3-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/linkinjeon/smb:
ksmbd: validate normalized name response length
ksmbd: fix listener task lifetime on netdev events
ksmbd: prevent out-of-bounds reads in share config responses
ksmbd: rate limit unmapped SID errors
ksmbd: propagate DACL parsing errors
ksmbd: zero pipe read compound padding
ksmbd: safely drain sessions during logoff
MAINTAINERS: Update the KSMBD entry
MAINTAINERS: Add Paulo Alcantara as an SMBDIRECT co-maintainer
ksmbd: fill in FileSysIdentifier in FS_POSIX_INFORMATION
ksmbd: initialize FileSystemControlFlags in FS_CONTROL_INFORMATION
ksmbd: zero the FS_OBJECT_ID_INFORMATION buffer before filling it in
|
|
CIFSSMBSetEA() copies the caller's extended attribute value into the
SMB request buffer without checking that it fits. The requirement is
stated in the source but was never implemented:
/*BB add length check to see if it would fit in
negotiated SMB buffer size BB */
/* if (ea_value_len > buffer_size - 512 (enough for header)) */
if (ea_value_len)
memcpy(parm_data->list.name + name_len + 1,
ea_value, ea_value_len);
The only bound applied on the way in is in cifs_xattr_set():
#define MAX_EA_VALUE_SIZE CIFSMaxBufSize
...
if (size > MAX_EA_VALUE_SIZE)
CIFSMaxBufSize is the full payload capacity of the buffer, so a value
of exactly that size leaves no room for the SMB header, the TRANS2
parameter block, the fealist header and the EA name that are written
ahead of it in the same object.
SendReceive() already enforces the correct limit on this very length:
if (in_len > CIFSMaxBufSize + MAX_CIFS_HDR_SIZE)
but it is called after the copy has taken place. An unprivileged
setxattr(2) on an SMB1 mount with a 250-byte name and a 16384-byte
value writes 16384 bytes starting 345 bytes into a 16588-byte
cifs_request object, ending 141 bytes past it:
BUG: KASAN: slab-out-of-bounds in CIFSSMBSetEA+0xabc/0xde0
Write of size 16384 at addr ffff888003aa0159 by task init/68
__asan_memcpy+0x3c/0x60
CIFSSMBSetEA+0xabc/0xde0
cifs_xattr_set+0xd3a/0xff0
__vfs_setxattr+0x13e/0x1a0
The buggy address is located 345 bytes inside of
allocated 16588-byte region
Apply SendReceive()'s limit to the assembled request before the copy
rather than after it, and widen the byte counters so the sum cannot
wrap before it is tested.
byte_count is also tested against U16_MAX, because it is stored in the
16-bit pSMB->ByteCount. That becomes reachable when CIFSMaxBufSize is
raised at module load, where it may be set as high as 1024*127: with a
5-byte EA name and a 65521-byte value, count is exactly U16_MAX while
byte_count is 65556, and cpu_to_le16() would truncate it to 20 and
transmit a frame whose ByteCount does not match its length. Testing
byte_count covers count as well, since byte_count is the larger of the
two and count's only 16-bit consumer is written after this point.
check_add_overflow() is evaluated first so that total_len is assigned
before it is reported.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: Yunpeng Tian <shionthanatos@gmail.com>
Reported-by: Mingda Zhang <npczmd@qq.com>
Reported-by: Gongming Wang <gmwgg05@gmail.com>
Reported-by: Qinrun Dai <jupmouse@gmail.com>
Cc: stable@vger.kernel.org
Signed-off-by: Yunpeng Tian <shionthanatos@gmail.com>
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
cifs_do_truncate() flushes dirty pages with filemap_write_and_wait()
and truncates the file on the server, but in the old code both
operations ran without holding i_rwsem or invalidate_lock. A
concurrent buffered write via netfs_perform_write() -- which only
needs i_rwsem shared -- could dirty new pages after the flush but
before the local truncation, and those pages would be silently
discarded by cifs_setsize() -> truncate_pagecache().
Fix by acquiring inode_lock (exclusive i_rwsem) and
filemap_invalidate_lock at the top of cifs_do_truncate(), so the
entire flush-truncate-resize sequence is atomic with respect to:
- buffered writes (blocked by exclusive i_rwsem, since
netfs_start_io_write takes i_rwsem shared),
- read page faults (blocked by exclusive invalidate_lock, since
filemap_fault takes it shared),
- writeback collection (blocked by netfs_wb_begin/netfs_wb_end
around the server truncate and local resize, since
netfs_writepages also acquires the wb lock).
Fixes: 110fee6b9bb5 ("smb: client: fix missing timestamp updates with O_TRUNC")
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
Cc: stable@vger.kernel.org
|
|
FILE_NORMALIZED_NAME_INFORMATION converts the open file path to UTF-16.
smb2_allocate_rsp_buf() leaves these responses in the 448-byte small
buffer, and get_file_normalized_name_info() converts the path without
checking the remaining space.
An authenticated client can query a long path and make
smbConvertToUTF16() write beyond work->response_buf.
Use the large response buffer for normalized-name queries. Before
conversion, verify that the response has room for the worst-case UTF-16
output and its terminator.
Fixes: 10aeff72ab82 ("ksmbd: support normalized name information")
Assisted-by: Antiproof:GPT-5.6-Sol
Signed-off-by: Alon Shakevsky <shakevsky@berkeley.edu>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
The listener thread exits when its listening socket is shutdown. The
netdevice notifier shuts down the socket before calling kthread_stop(), so
the task_struct can be freed before kthread_stop() gets its reference.
Create the listener in a stopped state and hold an extra task_struct
reference until kthread_stop_put() completes. Also stop and release
listeners before freeing their interface records during TCP teardown.
Fixes: 3316a8fc840d ("ksmbd: server: avoid busy polling in accept loop")
Reported-by: Farhad Alemi <farhad.alemi@berkeley.edu>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
Validate IPC share configuration payload sizes before consuming
variable-length fields. Bound veto list parsing and account for
the separator byte when deriving the path length.
Fixes: a677ebd8ca2f ("ksmbd: validate payload size in ipc response")
Reported-by: Kanishka De Silva <kpskanna1915@gmail.com>
Reported-by: Farhad Alemi <farhad.alemi@berkeley.edu>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
A client can include many structurally valid but unmapped SIDs in a DACL.
Logging every mapping failure lets one request generate hundreds of kernel
error messages.
Rate limit the message to prevent an authenticated client from flooding
the kernel log.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Reported-by: Cheryl Babcock <cheryl@renat.io>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
parse_dacl() silently accepts truncated ACEs and allocation failures,
allowing set_info_sec() to continue with an incomplete ACL conversion.
Return parsing and allocation errors to parse_sec_desc() so malformed
security descriptors are rejected before inode attributes or ACL xattrs
are updated.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Reported-by: Cheryl Babcock <cheryl@renat.io>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
Compound response handling extends the last response iov to an eight-byte
boundary.
smb2_read_pipe() allocates only the payload size, so the alignment padding
can expose up to seven bytes of uninitialized kernel heap memory.
Allocate the aligned size and clear the unused tail before pinning the
response buffer.
Fixes: e2b76ab8b5c9 ("ksmbd: add support for read compound")
Reported-by: Cheryl Babcock <cheryl@renat.io>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
SMB3 multichannel allows requests for one session to run on multiple
connections. Wait for all channels bound to a session before freeing
shared session objects.
A deferred byte-range lock remains counted as a running request and only
wakes when its file closes. Wake blocked locks during the drain without
unpublishing or modifying their file objects. Synchronous CANCEL requests
must invoke their cancellation callback to wake pending operations, while
CHANGE_NOTIFY completion remains specific to the asynchronous path.
Serialize session teardown with channel registration and previous-session
cleanup, and use atomic work-state transitions so LOGOFF, CANCEL, and
connection teardown invoke cancellation callbacks only once.
Fixes: 76e98a158b20 ("ksmbd: fix race condition between destroy_previous_session() and smb2 operations()")
Reported-by: Cheryl Babcock <cheryl@renat.io>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
smb2_get_info_filesystem() reports 56 bytes for FS_POSIX_INFORMATION,
that is the whole of FILE_SYSTEM_POSIX_INFO, but never assigns
FileSysIdentifier. Those eight bytes go to the client as they are found
in the response buffer.
The buffer is zeroed on allocation, so a standalone request leaks
nothing. A compound request can leak: the offset of the next response
is advanced by the length pinned for the previous one, so a reply that
was written into the buffer and then dropped in favour of the short
error response of smb2_set_err_rsp() stays there, and the next reply is
laid over it with only the header cleared.
Report the file system id statfs() returned, which is what the field is
for. FileSysIdentifier is __le64 and f_fsid is a pair of ints, so
assemble the value first, val[0] as the low half, and convert it on the
way out.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Cc: stable@vger.kernel.org
Signed-off-by: Aleksandr Khromov <haa@amicon.ru>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
smb2_get_info_filesystem() reports 48 bytes for FS_CONTROL_INFORMATION,
that is the whole of struct smb2_fs_control_info, but never assigns
FileSystemControlFlags. Those four bytes go to the client as they are
found in the response buffer.
The buffer is zeroed on allocation, so a standalone request leaks
nothing. A compound request can leak: the offset of the next response
is advanced by the length pinned for the previous one, so a reply that
was written into the buffer and then dropped in favour of the short
error response of smb2_set_err_rsp() stays there, and the next reply is
laid over it with only the header cleared.
ksmbd does not implement quota tracking, so report no control flags.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Cc: stable@vger.kernel.org
Signed-off-by: Aleksandr Khromov <haa@amicon.ru>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
smb2_get_info_filesystem() reports 64 bytes for FS_OBJECT_ID_INFORMATION,
that is the whole of struct object_id_info, but writes only 46 of them:
- objid[] is 16 bytes, and when the volume UUID is not available only
sizeof(stfs.f_fsid) (8) bytes are copied into it;
- extended_info.version_string[] is STRING_LENGTH (28) bytes, and only
strlen("1.1.0") (5) bytes are copied into it.
The response buffer is zeroed on allocation (kvzalloc() in
smb2_allocate_rsp_buf()), so for a standalone request the remaining 31
bytes are zero. In a compound request they need not be. The offset of
the next response is advanced by the length pinned for the previous one,
so if a preceding command wrote its reply into the buffer and then
failed, smb2_set_err_rsp() pins only the short error response and the
next reply lands inside the area that has already been written. Only
the header is cleared there:
memset((char *)rsp_hdr, 0, sizeof(struct smb2_hdr) + 2);
The client then receives up to 31 bytes of a response it was not meant
to see, including one that failed with an access denied error.
Clear the structure before filling it in. As a side effect
version_string is now NUL terminated.
Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")
Suggested-by: ChenXiaoSong <chenxiaosong@chenxiaosong.com>
Cc: stable@vger.kernel.org
Signed-off-by: Aleksandr Khromov <haa@amicon.ru>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
|
|
If find_writable_file() returns null, cifs_file_flush will return
0 without issuing set_file_size, and the outer 'if (!rc)' block
will set i_size to 0 before telling the server to truncate. If
the cifs_open() then fails, the inode will have size 0, while
the server file is unchanged.
Move the netfs_resize_file() and cifs_setsize() into the 'if
(cfile)', so they only run after a successful set_file_size.
In the no-handle else branch, evict stale pages with
truncate_inode_pages before the O_TRUNC open to dispose of old
cache pages, and let the open response set the i_size.
Fixes: 110fee6b9bb5 ("smb: client: fix missing timestamp updates with O_TRUNC")
Cc: stable@vger.kernel.org
Signed-off-by: Frank Sorenson <sorenson@redhat.com>
Acked-by: David Howells <dhowells@redhat.com>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
cifs_set_acl() validates ACL size using posix_acl_xattr_size():
4 + (count * 8) // 4-byte header + 8 bytes per ACE
cifs_do_set_acl() then calls posix_acl_to_cifs() to write the CIFS
wire format into the same buffer:
6 + (count * 10) // 6-byte header + 10 bytes per ACE
An ACL that passes the xattr-based check in cifs_set_acl() can
overflow the heap when posix_acl_to_cifs() writes the larger CIFS
format.
Validate the CIFS format size against the remaining buffer space and
USHRT_MAX before converting--data_count is __u16, so sizes above
USHRT_MAX truncate the on-wire packet length, causing the server to
apply a partial ACL. Replace MaxDataCount = 1000 with
min(CIFSMaxBufSize, USHRT_MAX).
Fixes: dc1af4c4b4721 ("cifs: implement set acl method")
Cc: stable@vger.kernel.org
Signed-off-by: Frank Sorenson <sorenson@redhat.com>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
Customer reported that they could no longer mount their SMB shares
with multiuser mount option and krb5. Turned out that the client
wasn't duplicating username option when creating multiuser
connections, therefore failing to retrieve credentials as
cifs.upcall(8) couldn't find them in keytab.
Fix this by duplicating username option (if set) from original fs
context before creating multiuser connections with krb5.
Reproducer:
```
$ ktutil
ktutil: add_entry -password -p testuser -k 1 -e aes256-cts
Password for testuser@ZELDA.TEST:
ktutil: write_kt /etc/krb5.keytab
ktutil: quit
$ klist -ke
Keytab name: FILE:/etc/krb5.keytab
KVNO Principal
---- ----------------------------------------------------------------
1 testuser@ZELDA.TEST (aes256-cts-hmac-sha1-96)
$ mount.cifs //w22-root2/scratch /mnt/1 -o \
uid=1000,sec=krb5,username=testuser@ZELDA.TEST,multiuser
mount error(13): Permission denied
Refer to the mount.cifs(8) manual page (e.g. man mount.cifs) and
kernel log messages (dmesg)
```
Reported-by: Jacob Shivers <jshivers@redhat.com>
Fixes: 12b4c5d98cd7 ("smb: client: fix krb5 mount with username option")
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: stable@vger.kernel.org
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
Long time ago during upgrading printk():s to the respective pr_<level>()
calls one misconversion happened and nobody has noticed that. So,
previously printk(KERN_DEBUG) + printk() worked as one long debug print
since the trailing '\n' is only present in the followup printk() format
string. The culprit change missed that and split the message to two on
the different levels. Restore the original behaviour to make users be
less confused in the most likely never happen cases of partially getting
that message.
Fixes: 0b456f04bcdf ("cifs: convert printk(LEVEL...) to pr_<level>")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
smb3_zero_range(), smb3_punch_hole(), smb3_insert_range(), and
smb3_collapse_range() modify file contents through server-side range
operations. These operations discard the affected page cache, but leave
the FS-Cache cookie valid, so a later read may return data cached before
the range operation.
Fix this by invalidating FS-Cache after outstanding I/O has completed
and before modifying the file on the server.
Run the following as root on a CIFS mount with fsc enabled and an active
CacheFiles backend:
bash -c '
MNT=/mnt/cifs
FILE="$MNT/repro"
# Generate four 1 MiB random blocks: [A][B][C][D].
dd if=/dev/urandom of=/tmp/src bs=1M count=4 status=none
# Expected contents after zeroing B: [A][zero][C][D].
cp /tmp/src /tmp/expected
dd if=/dev/zero of=/tmp/expected bs=1M seek=1 count=1 \
conv=notrunc status=none
cp /tmp/src "$FILE"
# Populate FS-Cache, then discard the page cache.
sync
echo 1 > /proc/sys/vm/drop_caches
cat "$FILE" > /dev/null
sync
echo 1 > /proc/sys/vm/drop_caches
fallocate --zero-range -o 1M -l 1M "$FILE"
if cmp -s /tmp/expected "$FILE"; then
echo "readback: OK"
else
echo "readback: STALE DATA"
fi
'
Before this change, the readback differs from /tmp/expected:
readback: STALE DATA
After this change, it matches:
readback: OK
Fixes: 30175628bf7f ("[SMB3] Enable fallocate -z support for SMB3 mounts")
Fixes: 31742c5a3317 ("enable fallocate punch hole ("fallocate -p") for SMB3")
Fixes: 5476b5dd82c8 ("cifs: add support for FALLOC_FL_COLLAPSE_RANGE")
Fixes: 7fe6fe95b936 ("cifs: add FALLOC_FL_INSERT_RANGE support")
Signed-off-by: Huiwen He <hehuiwen@kylinos.cn>
Suggested-by: Namjae Jeon <linkinjeon@kernel.org>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
smb3_insert_range() and smb3_collapse_range() use
truncate_pagecache_range() to invalidate the affected page cache.
However, if off or old_eof is not page-aligned, the boundary pages are
only partially zeroed and remain uptodate. As a result, the client may
return stale data after a successful insert/collapse range operation.
For example, with 4K pages:
page 0 page 1 page 2
0------4K 4K------8K 8K------12K
^ ^
off=2K old_eof=10K
Page 1 is removed from the page cache, while the boundary pages are
only partially zeroed. After COPYCHUNK moves the data on the server,
these cached pages may still return stale data.
This can be reproduced on a CIFS mount:
bash -c '
FILE=/mnt/scratch/repro
# Use a 6 KiB file so EOF is not page-aligned.
dd if=/dev/urandom of=/tmp/src bs=1K count=6 status=none
# Expected: a 4 KiB hole followed by the original data.
rm -f /tmp/expected
truncate -s 4K /tmp/expected
cat /tmp/src >> /tmp/expected
cp /tmp/src "$FILE"
# Prime the page cache before moving data on the server.
cat "$FILE" > /dev/null
fallocate --insert-range -o 0 -l 4K "$FILE"
if cmp -s /tmp/expected "$FILE"; then
echo "readback: OK"
else
echo "readback: STALE DATA"
fi
'
Fix this by writing back dirty data and discarding the page cache from
the start of the page containing off to EOF before moving data on the
server.
Fixes: 9c8b7a293f50 ("smb3: fix temporary data corruption in insert range")
Fixes: fa30a81f255a ("smb3: fix temporary data corruption in collapse range")
Signed-off-by: Huiwen He <hehuiwen@kylinos.cn>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
smb3_collapse_range() stores the ssize_t return value of
smb2_copychunk_range() in an int. A successful copy larger than
INT_MAX is truncated to a negative value and treated as an error.
Reproducer:
MNT=/mnt/scratch
truncate -s 2056M "$MNT/file"
fallocate --collapse-range -o 1M -l 1M "$MNT/file"
Fix this by using __smb2_copychunk_range(), which reports success as
zero instead of returning the copied byte count.
Before this change, the reproducer fails with:
fallocate: fallocate failed: Success
and the file size remains unchanged at 2056 MiB. After this change, the
reproducer succeeds and the file size becomes the expected 2055 MiB.
Fixes: 5476b5dd82c8 ("cifs: add support for FALLOC_FL_COLLAPSE_RANGE")
Signed-off-by: Huiwen He <hehuiwen@kylinos.cn>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
smb3_insert_range() shifts [off, EOF) right with COPYCHUNK, copying from
low to high offsets. When the ranges overlap, the copy can overwrite
source data that has not yet been copied. For a 1 MiB insert at offset 0:
offset: 0 1M 2M 3M 4M 5M
before: | A | B | C | D |
expected: | hole | A | B | C | D |
current: | hole | A | A | A | A | (corrupted)
Let x be the insertion offset, L the total length to move, delta the
insert length, and C the normal chunk size allowed by the server.
Insert range maps
[x, x + L) -> [x + delta, x + delta + L).
When delta >= L, the complete source and target ranges are disjoint, so
the normal copy order and chunk size are safe:
offset: 0 4 8 12 16 20 24 28 32
source: [--S0--][--S1--][--S2--][--S3--]
target: [--T0--][--T1--][--T2--][--T3--]
When delta < L, the complete source and target ranges overlap, so the
copy must proceed from EOF backwards. There are two subcases.
If delta >= C, each corresponding source and target chunk is disjoint.
The 1 MiB example has L = 4 MiB and delta = C = 1 MiB:
offset: 0 1M 2M 3M 4M 5M
source: [--S0--][--S1--][--S2--][--S3--]
target: [--T0--][--T1--][--T2--][--T3--]
Copying S0 from [0, 1M) to [1M, 2M) overwrites S1 before it is copied.
Processing chunks from EOF backwards prevents this inter-chunk
overwrite.
If delta < C, the source and target ranges of a normal chunk also
overlap. For example, with L = 16, delta = 2 and C = 4:
offset: 0 2 4 6 8 10 12 14 16 18
source: [--S0--][--S1--][--S2--][--S3--]
target: [--T0--][--T1--][--T2--][--T3--]
Here S0 and T0 overlap over [2,4), S1 and T1 over [6,8), and so on.
Backward ordering cannot control how the server copies bytes inside one
descriptor, so the chunk size must be limited to delta.
Fix this by copying overlapping right shifts from EOF backwards. Limit
the chunk size to delta when delta < C so that each chunk's source and
target ranges do not overlap. Using larger chunks would require a way to
identify servers that safely handle overlapping COPYCHUNK descriptors.
Therefore:
delta >= L:
keep the normal copy order and chunk size
delta < L:
delta >= C: copy backwards and keep the normal chunk size
delta < C: copy backwards and limit the chunk size to delta
Only the delta < C subcase requires reducing the chunk size for data
integrity.
Reproducer:
bash -c '
MNT=/mnt/scratch
# Generate four 1 MiB random blocks: [A][B][C][D].
dd if=/dev/urandom of=/tmp/src bs=1M count=4 status=none
# With C = 1 MiB, test delta = C and delta < C.
for delta in 1M 1K; do
truncate -s 0 /tmp/expected
truncate -s "$delta" /tmp/expected
cat /tmp/src >> /tmp/expected
cp /tmp/src "$MNT/file"
fallocate --insert-range -o 0 -l "$delta" "$MNT/file"
if cmp -s /tmp/expected "$MNT/file"; then
echo "delta=$delta: OK"
else
echo "delta=$delta: CORRUPTED"
fi
done
'
The corruption reproduces with Samba and ksmbd, while Windows handles
the overlapping COPYCHUNK ranges safely.
The 1 MiB case tests delta >= C, while the 1 KiB case tests delta < C.
Before this change, the reproducer reports:
delta=1M: CORRUPTED
delta=1K: CORRUPTED
After this change, it passes against both ksmbd and Samba:
delta=1M: OK
delta=1K: OK
Fixes: 7fe6fe95b936 ("cifs: add FALLOC_FL_INSERT_RANGE support")
Signed-off-by: Huiwen He <hehuiwen@kylinos.cn>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
The SMB client emulates FALLOC_FL_INSERT_RANGE with SET_EOF, COPYCHUNK
and SET_ZERO_DATA.
SET_ZERO_DATA creates a hole only when the file is sparse. On a
non-sparse file, it clears the inserted range but leaves its blocks
allocated, causing the extent count check in xfstests generic/064 to
fail.
Fix this by marking the file sparse before modifying it.
This patch produces the expected sparse extents in xfstests generic/064
only when the server-reported block size is compatible with the server's
deallocation granularity.
For ksmbd, the reported block size follows the backing filesystem,
and the test passes. For Samba, the test passes with a block size
matching the backend granularity, for example, 4 KiB on Btrfs, but not
with the default 1 KiB value. For Windows Server 2022, 4 KiB inserts do
not generate holes, while aligned inserts of 64 KiB or larger do.
Fixes: 7fe6fe95b936 ("cifs: add FALLOC_FL_INSERT_RANGE support")
Signed-off-by: Huiwen He <hehuiwen@kylinos.cn>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
When FALLOC_FL_ZERO_RANGE is used without FALLOC_FL_KEEP_SIZE,
smb3_zero_range() may extend EOF without checking RLIMIT_FSIZE, allowing
the file to grow beyond the caller's file-size limit.
Fix this by calling inode_newsize_ok() before sending the zero-range
request when the operation would extend EOF.
Reproducer, using a file on a CIFS mount:
bash -c '
FILE=/mnt/cifs/repro
trap "" SIGXFSZ
ulimit -f 3072
truncate -s 2M "$FILE"
fallocate --zero-range -o 0 -l 4M "$FILE"
echo "fallocate rc=$?"
stat -c "file size=%s" "$FILE"
'
Before this change, the operation succeeds despite the 3 MiB limit:
fallocate rc=0
file size=4194304
After this change, fallocate fails and leaves the file at 2 MiB.
Fixes: 72c419d9b073 ("cifs: fix smb3_zero_range so it can expand the file-size when required")
Signed-off-by: Huiwen He <hehuiwen@kylinos.cn>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
smb3_insert_range() does not check if the new file size
(i_size + len) is valid. This allows FALLOC_FL_INSERT_RANGE to bypass
RLIMIT_FSIZE, exceed s_maxbytes, or produce a size outside the loff_t
range.
Use check_add_overflow() to calculate the new EOF. Validate it with
inode_newsize_ok() before modifying the file.
Reproducer, using a file on a CIFS mount:
bash -c '
FILE=/mnt/cifs/repro
trap "" SIGXFSZ
ulimit -f 3072 # RLIMIT_FSIZE = 3 MiB
# A regular write is stopped at 3 MiB.
dd if=/dev/zero of="$FILE" bs=1M count=4 status=none
stat -c "size after write: %s" "$FILE"
# Insert 2 MiB into a 2 MiB file.
truncate -s 2M "$FILE"
fallocate -i -o 0 -l 2M "$FILE"
stat -c "size after insert: %s" "$FILE"
'
Before this change, the regular write stops at the 3 MiB limit, but
insert range grows the file to 4 MiB:
dd: error writing '/mnt/cifs/repro': File too large
size after write: 3145728
size after insert: 4194304
After this change, insert range also fails at the limit and leaves the
2 MiB file unchanged:
dd: error writing '/mnt/cifs/repro': File too large
size after write: 3145728
fallocate: fallocate failed: File too large
size after insert: 2097152
Fixes: 7fe6fe95b936 ("cifs: add FALLOC_FL_INSERT_RANGE support")
Signed-off-by: Huiwen He <hehuiwen@kylinos.cn>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm
Pull more MM updates from Andrew Morton:
- "mm/rmap: index MAP_PRIVATE file-backed folios by anonymous pgoff"
(Lorenzo Stoakes)
Index MAP_PRIVATE file-backed folios by their anonymous page offset
to resolve confusion around reverse mapping for zeroed and CoW'd
file-backed memory.
Use this new VMA anonymous page offset tracking to eliminate index
conflicts and lay the foundation for scalable CoW performance
improvements.
- "promote mapped executable folios after first usage for MGLRU"
(Baolin Wang)
Make MGLRU's protection of mapped executable file folios more
reliable. Follow the classical LRU's logic, promoting mapped
executable file folios after their first usage to give executable
code a better chance to stay in memory and improve workload
performance.
- "mm: vmscan: fix node reclaim ignoring swappiness parameter" (Ridong
Chen)
Fix per-node proactive reclaim interface's ignoring the swappiness
parameter when CONFIG_MEMCG is disabled by consolidating
sc_swappiness() into a single function that checks
proactive_swappiness regardless of kernel configuration.
- "mm/vmscan: reduce lru_lock contention via vmstat-derived
scan-balance cost" (Usama Arif)
Reduce lru_lock contention in the reclaim path by deriving
scan-balance costs from vmstat counters rather than lock-acquired
producer updates.
Read and decay these cost signals on the reclaim side under a
dedicated per-lruvec lock, reducing total LRU lock wait time by over
60% without impacting scan throughput.
- "zram: fix zram issues reported by sashiko" (Sergey Senozhatsky)
Fix two low-risk zram bugs which Sashiko spotted in drive-by review.
- "Honor XA_FLAGS_ACCOUNT in xas_split_alloc() and charge to folio's
memcg" (Zi Yan)
Fix xas_split_alloc() by enabling target folio memcg charging during
splits and adding the missing __GFP_ACCOUNT flag for proper XArray
node memory accounting.
- "selftests/mm: use pattern matching in .gitignore" (Pratyush Mallick)
Replace hardcoded binary names in selftests/mm/.gitignore with a
generic pattern-matching rule to automatically ignore generated test
files and avoid manual updates when adding new tests.
- "mm/page_ext: remove pgdat_page_ext_init()" (Sang-Heon Jeon)
Make the incompatibility between FLATMEM and NUMA explicit in
mm/Kconfig and remove the unused pgdat_page_ext_init() function.
- "zram: fix zstd error paths and add parameter validation" (Haoqin
Huang)
Clean up zram compression backends by removing redundant error
cleanup, adding parameter and dictionary validation, auto-prefixing
algorithm error logs, and resetting parameters prior to
reinitialization.
- "zram: fix stale scan bounds after reinitialization" (Longlong Xia)
Prevent out-of-bounds slot accesses during concurrent zram resets by
moving table scan bound calculations under dev_lock in
writeback_store() and read_block_state().
- "add anon mTHP collapse test cases" (Baolin Wang)
Extend selftests helper functions to support arbitrary page orders
and add new test cases and options for mTHP collapse in khugepaged.
- "selftests/mm: Handle unsupported and transient test conditions"
(Muhammad Usama Anjum)
Update MM selftests to report a SKIP status instead of a failure when
required kernel or filesystem features are unsupported, while adding
retry logic for transient page migration errors.
- "mm/zswap: Fixes and improves the zswap shrink" (Hao Jia)
Fix the missing zswap global shrinker when CONFIG_MEMCG is disabled
and extend shrink_memcg() to support batch writeback for improved
writeback efficiency.
- "alloc_tag: introduce IOCTL-based filtering for MAP" (Suren
Baghdasaryan)
Introduce an IOCTL-based binary interface for memory allocation
profiling that enables kernel-side filtering before per-CPU counter
aggregation.
This eliminates the text-parsing overhead of /proc/allocinfo and
provides up to a 20x speedup by transferring only filtered allocation
data to userspace.
- "better block swap batching and a different take on swap_ops v5"
(Christoph Hellwig)
Refactor block swap I/O to use swap_iocb for batching instead of
single-bio requests and rebase the swap_ops interface, achieving
faster swap throughput during kernel builds.
- "mm: kmemleak: reduce transient false positives by confirming leaks"
(Catalin Marinas)
Reduce false-positive kmemleak reports by combining two kmemleak
enhancements that add a second confirmation scan and a configurable
minimum unreferenced scan count module parameter.
- "mm: kmemleak: default min_unref_scans to 2 for verbose kernels"
(Breno Leitao)
Auto-scanning kernels can generate false-positive memory leak reports
on single scans, so this patch defaults min_unref_scans to 2 when
CONFIG_DEBUG_KMEMLEAK_VERBOSE is enabled to require a second
confirming scan.
- "swap_ops updates" (Christoph Hellwig)
Batching I/O for synchronous swap devices causes performance
regressions and filesystem-based swap suffers from double-indirection
overhead. This series resolves both issues by reintroducing per-folio
writes for synchronous swap and allowing filesystems to directly
export their own swap_ops.
- "mm/khugepaged: several cleanups" (Nico Pache)
khugepaged accumulated redundant state-checking patterns and outdated
comments following mTHP integration. Introduce dedicated helpers for
PTE validation and event counting while refreshing the internal
documentation.
- "maple_tree: lock checking and clean ups" (Liam Howlett)
Syzbot reports incorrectly blame memory management exit paths for
locking bugs, maple tree erase operations risk allocation failures
without gfp flags and internal documentation lacks clarity.
Improve lock error detection, update docs, fix race and allocation
edge cases and optimize erase allocations using a fallback to
GFP_KERNEL | GFP_NOFAIL.
* tag 'mm-stable-2026-08-26-15-22' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm: (172 commits)
selftests/proc: make proc-maps-race work with READ_IMPLIES_EXEC
memcg: move LRU size accounting on reparenting instead of copying it
mm/vmscan: fix comment logic in balance_pgdat
maple_tree: add helper mas_make_walkable()
maple_tree: avoid extra gap calculation
maple_tree: fix argument name in header
maple_tree: change two GFP flags in tests
maple_tree: document erase and allocations better
maple_tree: avoid mas_erase() and mtree_erase() failures
maple_tree: document that erase may use GFP_KERNEL for allocations
maple_tree: catch race in mas_alloc_cyclic()
maple_tree: add bulk parent set helper
maple_tree: micro optimisation of mas_wr_store_type()
maple_tree: optimise mas_wr_node_store() when not in rcu mode
maple_tree: use prefetched value in mas_wr_store_type()
maple_tree: clarify comments on mas_nomem()
maple_tree: drop MAPLE_ALLOC_SLOTS
maple_tree: drop dead code from mas_extend_spanning_null()
maple_tree: documentation fix
maple_tree: add write lock checking with lockdep sequence numbers
...
|
