| Age | Commit message (Collapse) | Author |
|---|
|
signing_negotiated is set when a signing context is received but never
read; drop it.
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Reviewed-by: David Howells <dhowells@redhat.com>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
|
|
sec_kerberosu2u is set when the krb5 user-to-user OID is seen but is
never read, so the OID branch that sets it has no effect. Drop both.
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Reviewed-by: David Howells <dhowells@redhat.com>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
|
|
max_rw is assigned from the SMB1 negotiate response but never read, so
drop it.
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Reviewed-by: David Howells <dhowells@redhat.com>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
|
|
overrideSecFlg has no readers or writers anywhere in the tree; drop it.
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Reviewed-by: David Howells <dhowells@redhat.com>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
|
|
CIFS carried a hand-maintained internal version (CIFS_VERSION and
SMB3_PRODUCT_BUILD) that had to be bumped by hand on every change. The
kernel version already identifies which features and fixes are present, so
drop it.
Remove MODULE_VERSION() and the "CIFS Version" line in DebugData, and send
LINUX_VERSION_SUBLEVEL in the NTLMSSP ProductBuild field so the advertised
version is derived entirely from the kernel version -- its major and minor
were already taken from there.
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Reviewed-by: David Howells <dhowells@redhat.com>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
|
|
Remove the leftover commented-out debug message, drop the now-unnecessary
braces around the single return, and test !buf_to_free instead of
comparing against NULL.
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Reviewed-by: David Howells <dhowells@redhat.com>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
|
|
Drop this leftover commented-out assignment.
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Reviewed-by: David Howells <dhowells@redhat.com>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
|
|
Drop this leftover commented-out debug message.
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Reviewed-by: David Howells <dhowells@redhat.com>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
|
|
Drop this leftover commented-out call.
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Reviewed-by: David Howells <dhowells@redhat.com>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
|
|
Drop this leftover commented-out debug message.
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Reviewed-by: David Howells <dhowells@redhat.com>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
|
|
Drop this leftover commented-out declaration.
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Reviewed-by: David Howells <dhowells@redhat.com>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
|
|
Drop this leftover commented-out debug message.
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Reviewed-by: David Howells <dhowells@redhat.com>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
|
|
Drop this leftover commented-out assignment.
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Reviewed-by: David Howells <dhowells@redhat.com>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
|
|
Drop these leftover commented-out calls.
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Reviewed-by: David Howells <dhowells@redhat.com>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
|
|
Drop this leftover commented-out assignment.
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Reviewed-by: David Howells <dhowells@redhat.com>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
|
|
Drop this leftover commented-out definition.
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Reviewed-by: David Howells <dhowells@redhat.com>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
|
|
When mounting without an explicit sec= option, ses->sectype stays
Unspecified while the real mechanism is chosen later through
->select_sectype(), so a failed session setup logs a useless name:
CIFS: VFS: \\server failed to create a new SMB session with Unknown: -13
Resolve the mechanism through ->select_sectype() before formatting it, as
cifs_debug.c already does, so the message names the actual security type.
Fixes: a8fce7c807b1 ("smb: client: improve error message when creating SMB session")
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Reviewed-by: David Howells <dhowells@redhat.com>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
|
|
cifs_check_mf_symlink() probes any file whose size matches
CIFS_MF_SYMLINK_FILE_SIZE as a possible Minshall+French symlink.
smb2_query_mf_symlink() did so with a fresh leaseless SMB2_open, so the
server saw a second conflicting opener and broke the RWH lease the client
had just been granted on the same file, needlessly churning the cache.
Rework smb2_query_mf_symlink() to issue a single OPEN/READ/CLOSE compound
that either reuses an already open handle via cifs_get_readable_path() or,
failing that, opens with the inode's lease key so the server treats it as
the same lease holder. Thread the inode down to the ->query_mf_symlink()
handlers, rename check_mf_symlink() to cifs_check_mf_symlink(), and
validate the read payload against the response buffer before copying it.
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Reviewed-by: David Howells <dhowells@redhat.com>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
|
|
cifs_new_fileinfo() -> set_fid() sets CIFS_I(inode)->lease_granted only
after cifs_get_inode_info() has run. Opens that cifs_get_inode_info()
issues for the inode -- such as the Minshall+French symlink probe or an
open compounded in smb2_compound_op() -- therefore find lease_granted
clear, go out leaseless, and make the server break the lease just granted
to the file.
Set lease_granted right after the open succeeds, before
cifs_get_inode_info(), so those opens reuse the inode's lease key. Only
raise it: lease_granted is kept set for the inode's lifetime, so lowering
it here could clobber a lease still held through another open.
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Reviewed-by: David Howells <dhowells@redhat.com>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
|
|
smb3_query_mf_symlink() and smb3_create_mf_symlink() are installed from
smb20_operations through smb311_operations, so they serve every SMB2
dialect rather than only SMB3. Rename them with the smb2_ prefix to
match. No functional change.
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
Reviewed-by: David Howells <dhowells@redhat.com>
Cc: Namjae Jeon <linkinjeon@kernel.org>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Cc: Shyam Prasad N <sprasad@microsoft.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: Bharath SM <bharathsm@microsoft.com>
|
|
cifs_fill_dirent() rejects an entry whose name extends past the end of
the response before it returns, for both of its callers. The two
checks cifs_filldir() applies after the parse can no longer fail: the
one that commit f8cf09a53a0d ("smb: client: bound dirent name against
end of SMB response in cifs_filldir") added tests the same name
against the same end, and the older one, which compares the name
length with the length of the response, is implied by it because the
name starts inside the response. Remove both.
This patch depends on "smb: client: fix OOB read of the resume name
sent in TRANS2_FIND_NEXT2", which added the check to
cifs_fill_dirent().
No functional change intended.
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>
|
|
CIFSFindFirst() and CIFSFindNext() read the SearchHandle, SearchCount,
EndofSearch and LastNameOffset of a TRANS2_FIND_FIRST2 or
TRANS2_FIND_NEXT2 response at ParameterOffset. validate_t2() bounds
the offset at 1024 and ParameterCount below 512, and nothing compares
them with the number of bytes SendReceive() copied into the buffer or
with the size of the response parameters. smb_init() places the
request and the response in the same cifs_request allocation and
cifs_buf_get() clears only the start of it, so a short response with a
ParameterOffset past its end has the search handle, the entry count,
the end-of-search flag and the last-entry offset read out of the
request that was just sent, or out of whatever the allocation held
before, and a ParameterCount smaller than the parameters has them read
past the end of the parameter block. The reads stay inside the
allocation, but what they return is not part of the response, and
CIFSFindFirst() then hands back a search handle the server never sent.
That handle is sent back to the server in the next FIND_NEXT2 or
FIND_CLOSE2, so two bytes of the buffer leak at an offset the server
picks.
SMB1 is not negotiated by default; reaching this code requires an
explicit vers=1.0 mount.
Reject a response whose parameter area is smaller than the response
parameters or extends past the received length, returning smb_EIO2()
with a new smb_eio_traces value for each of the two responses, like
the other response checks this file gained since v6.19. Backports to
trees before v6.19 need smb_EIO2() replaced with -EINVAL and the
trace.h hunk dropped.
A conforming server is not affected. The parameter block of a
transaction response is part of the SMB, so ParameterOffset +
ParameterCount cannot exceed the smbCalcSize() bytes that
SendReceive() copies, and the response parameters are 10 bytes for
FIND_FIRST2 and 8 bytes for FIND_NEXT2, which is what
T2_FFIRST_RSP_PARMS and T2_FNEXT_RSP_PARMS hold.
This patch depends on "smb: client: reject FIND data areas that run
past the received response", whose declarations it extends.
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>
|
|
CIFSFindFirst() and CIFSFindNext() take the directory entries of a
TRANS2_FIND_FIRST2 or TRANS2_FIND_NEXT2 response from DataOffset for
DataCount bytes. validate_t2() bounds DataOffset at 1024 and
ParameterCount + DataCount by the byte count, and nothing compares
DataOffset + DataCount with the number of bytes SendReceive() copied
into the buffer.
smb_init() places the request and the response in the same
cifs_request allocation and cifs_buf_get() clears only the start of
it, so a short response whose data area extends past its end has
srch_entries_start point at bytes of the request that was just sent,
or at whatever the allocation held before, and those bytes are parsed
as directory entries. cifs_readdir() and find_cifs_entry() walk the
entries with nxt_dir_entry(), which checks every entry but the first
of a response against the end of the SMB; cifs_query_path_info() and
cifs_backup_query_path_info() read the first entry with no check at
all. Those reads stay inside the allocation, but what they return is
not part of the response. The last entry, which LastNameOffset locates
relative to the data area, is bounded against the received response
by an earlier patch of this series, as is every entry that
cifs_fill_dirent() parses; the walk itself and the two query functions
are not.
SMB1 is not negotiated by default; reaching this code requires an
explicit vers=1.0 mount.
Reject a response whose data area extends past the received length,
with the -EINVAL that validate_t2() returns for the other malformed
transaction responses. cifs_query_path_info() and
cifs_backup_query_path_info() still read a whole entry from a data
area that may be smaller than one; that is not changed here.
A conforming server is not affected. The data block of a transaction
response is part of the SMB, so DataOffset + DataCount cannot exceed
the smbCalcSize() bytes that SendReceive() copies; coalesce_t2()
appends each secondary response to the data area and adds its length
to both DataCount and the byte count, which keeps that relation.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
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>
|
|
cifs_fill_dirent_unix() finds the length of an SMB_FIND_FILE_UNIX
entry name by scanning for its terminating NUL, over up to PATH_MAX + 1
UTF-16 units in cifs_unicode_bytelen() or PATH_MAX bytes in strnlen().
Neither scan knows where the response ends. cifs_fill_dirent() only
requires the fixed part of the entry to fit, so the name can start
exactly at the end of the received data, and a name that holds no
terminator is then scanned for 8194 bytes, or 4096 bytes without
unicode, past the end of the received data. SendReceive() copies at
most 16468 bytes into the 16588-byte cifs_request allocation, so for a
response of that length up to 8074 of the bytes scanned lie past the
end of the allocation. Before the preceding patches bounded where the
entry starts, the entry itself could start up to 820 bytes past the
end of the allocation at the default CIFSMaxBufSize, and its name 928
bytes past it, so all 8194 scanned bytes would lie outside.
cifs_fill_dirent() rejects the name afterwards because it extends past
the end of the response, so nothing of what was read is used, but the
out-of-bounds read itself remains.
SMB_FIND_FILE_UNIX is only used with SMB1 servers that offer the Unix
extensions; SMB1 is not negotiated by default and requires an explicit
vers=1.0 mount.
Before the preceding patches, an entry placed past the buffer left the
scan out of bounds from its first read. They bound where the entry
starts, but nothing bounds the scan, so a name that holds no
terminator still runs off the end of the allocation. Reproduced on
the unpatched tree with KASAN enabled, for an entry that starts
inside the response, which is the case the preceding patches leave
open:
==================================================================
BUG: KASAN: slab-out-of-bounds in cifs_save_resume_key.isra.0+0x75c/0x7f0
Read of size 2 at addr ffff88807ee640cc by task ls/84
CPU: 0 UID: 0 PID: 84 Comm: ls Not tainted 7.3.0-rc4-00457-gf14572c203d5 #69 PREEMPT(lazy)
Call Trace:
<TASK>
kasan_report+0xdf/0x1a0
cifs_save_resume_key.isra.0+0x75c/0x7f0
cifs_readdir+0x1ed6/0x29c0
iterate_dir+0x1c0/0x570
__x64_sys_getdents64+0x133/0x270
do_syscall_64+0x109/0x5d0
entry_SYSCALL_64_after_hwframe+0x77/0x7f
</TASK>
Allocated by task 84 on cpu 1 at 15.751078s:
cifs_buf_get+0x36/0x90
smb_init+0x4f/0x110
CIFSFindNext+0xf7/0x14c0
cifs_readdir+0xf51/0x29c0
The buggy address belongs to the object at ffff88807ee60000
which belongs to the cache cifs_request of size 16588
The buggy address is located 0 bytes to the right of
allocated 16588-byte region [ffff88807ee60000, ffff88807ee640cc)
==================================================================
The two bytes read there are one UTF-16 unit of the scan, at the first
byte past the end of the object. The scan carries on from there,
reported again 2, 4 and 6 bytes to the right.
Pass the number of bytes that follow the fixed part of the entry to
cifs_fill_dirent_unix() and stop both scans there. A scan that reaches
the end of the response without finding a terminator now fails: it
returns a name that ends exactly at the end of the response and would
otherwise pass the bound, so cifs_fill_dirent() requires the position
of the terminator of an SMB_FIND_FILE_UNIX name to lie inside the
response as well. Accepting such a name truncated at the end of the
response would turn a malformed entry into a directory entry, or a
resume name, that no server sent. A name longer than the PATH_MAX cap
of the scans is still truncated there, as before, when the response
extends past the cap. The message cifs_unicode_bytelen() logs when it
finds no terminator now covers both limits.
This patch depends on "smb: client: fix OOB read of the resume name
sent in TRANS2_FIND_NEXT2", which adds the end argument of
cifs_fill_dirent() and the length it derives from it; the backport
note in that patch applies to this one as well.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
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>
|
|
cifs_fill_dirent() reads the fixed part of a directory entry and the
name length it holds, or for SMB_FIND_FILE_UNIX scans the name for its
terminating NUL, without knowing where the response ends.
cifs_save_resume_key() calls it for the entry at the LastNameOffset the
server chose, after every FindNext and after the FindFirst of a search
rewind, and records the name it returns as the resume name of the
search without bounding it; the next CIFSFindNext() copies up to
PATH_MAX - 1 bytes from that name into its request and sends them to
the server. cifs_filldir() bounds
the name against the end of the response since
commit f8cf09a53a0d ("smb: client: bound dirent name against end of
SMB response in cifs_filldir"), but only after cifs_fill_dirent() has
read the entry, and nxt_dir_entry() checks the entries it walks only
against sizeof(FILE_DIRECTORY_INFO), for every level but
SMB_FIND_FILE_INFO_STANDARD, and never sees the first entry of a
response.
Bounding where last_entry starts, as the preceding patch does, does
not bound the read. A last entry that starts within the final bytes of
the response still has its fixed part read past the end of the
received data, and the name length it declares, a __le32 for every
level but SMB_FIND_FILE_INFO_STANDARD and SMB_FIND_FILE_UNIX, is
recorded without being checked against anything. CIFSFindNext()
rejects only a length of PATH_MAX or more, then copies the name, up to
4095 bytes, into its request and sends it, and as the report below
shows that copy can run past the end of the cifs_request allocation
that holds the response. For SMB_FIND_FILE_UNIX the name is in
addition scanned for its terminating NUL over up to PATH_MAX + 1
UTF-16 units, or PATH_MAX bytes, before any check. The FindNext that
carries the name is only sent by the SMB1 dialect, which is not
negotiated by default; reaching it requires an explicit vers=1.0
mount.
With KASAN enabled, a response whose last entry declares a name
longer than the bytes that follow it gives:
==================================================================
BUG: KASAN: slab-out-of-bounds in CIFSFindNext+0x8ca/0x14c0
Read of size 4080 at addr ffff88807c823f98 by task ls/83
CPU: 0 UID: 0 PID: 83 Comm: ls Not tainted 7.3.0-rc4-00457-gf14572c203d5 #69 PREEMPT(lazy)
Call Trace:
<TASK>
kasan_report+0xdf/0x1a0
kasan_check_range+0x10f/0x1e0
__asan_memcpy+0x23/0x60
CIFSFindNext+0x8ca/0x14c0
cifs_readdir+0xf51/0x29c0
iterate_dir+0x1c0/0x570
__x64_sys_getdents64+0x133/0x270
do_syscall_64+0x109/0x5d0
entry_SYSCALL_64_after_hwframe+0x77/0x7f
</TASK>
Allocated by task 83 on cpu 1 at 13.497119s:
cifs_buf_get+0x36/0x90
smb_init+0x4f/0x110
CIFSFindNext+0xf7/0x14c0
cifs_readdir+0xf51/0x29c0
The buggy address belongs to the object at ffff88807c820000
which belongs to the cache cifs_request of size 16588
The buggy address is located 16280 bytes inside of
allocated 16588-byte region [ffff88807c820000, ffff88807c8240cc)
==================================================================
The read is the memcpy() that copies the recorded name into the next
request, so those bytes go to the server.
Pass the end of the response to cifs_fill_dirent() and have it reject
an entry whose fixed part does not fit or whose name extends past the
end, before either is used, so that both callers are covered;
cifs_save_resume_key() computes the end the same way cifs_readdir()
does. The name bound compares the bytes left between the name and the
end with the name length rather than adding the length to the name
pointer, since the length is a 32-bit value taken from the entry and
the sum could wrap on 32-bit systems. cifs_fill_dirent() is also used
on the SMB2 readdir path; there smb2_parse_query_directory() has
already checked every entry against the end of the response, so a
valid SMB2 response is not rejected by the new checks. The check
cifs_filldir() applies after the parse is left in place. The NUL scan
for SMB_FIND_FILE_UNIX names is capped by a separate patch.
An entry and its name lie inside the data area of the response, so a
server that points LastNameOffset at the entry is not affected. One
that points it at the file name, as MS-CIFS 2.2.6.2.2 describes, has
its last entry rejected here; the listing is unaffected, because
cifs_readdir() records the resume name from an entry it emitted, and
before this series such a response could instead end the listing early,
since a name length read from the wrong place is usually PATH_MAX or
more, which CIFSFindNext() rejects with -EINVAL.
On trees before v6.19 the type for the level 0x105 fixed part is
spelled SEARCH_ID_FULL_DIR_INFO; adjust when backporting.
Fixes: 0752f1522a91 ("[CIFS] make sure we have the right resume info before calling CIFSFindNext")
Cc: stable@vger.kernel.org # 6.1.x: f8cf09a53a0d: smb: client: bound dirent name against end of SMB response in cifs_filldir
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>
|
|
CIFSFindFirst() and CIFSFindNext() locate the last entry of a
TRANS2_FIND_FIRST2 or TRANS2_FIND_NEXT2 response by adding the
LastNameOffset of the response parameters to the start of the data
area, and store the result in psrch_inf->last_entry. MS-CIFS 2.2.6.2.2
defines that offset as the offset of the entry's file name; the client
follows the reading of NT and Samba servers, which point it at the
entry. LastNameOffset is only checked against CIFSMaxBufSize, which
says nothing about the response actually received: validate_t2() lets
the data area start up to 1024 bytes into the buffer, and SendReceive()
copies at most CIFSMaxBufSize + MAX_CIFS_HDR_SIZE bytes of a response,
16468 by default, into a cifs_request allocation of CIFSMaxBufSize +
MAX_SMB2_HDR_SIZE bytes, 16588 by default. The pointer itself can
therefore be placed anywhere from the end of the received data to 820
bytes past the end of the allocation.
The read through that pointer is far larger than its displacement.
find_cifs_entry() hands last_entry to cifs_save_resume_key() after every
FindNext and after the FindFirst of a search rewind, and
cifs_fill_dirent() takes the length of the name from the entry itself: a
__le32 FileNameLength for most levels, a __u8 for
SMB_FIND_FILE_INFO_STANDARD, and for SMB_FIND_FILE_UNIX the result of a
scan for the terminating NUL over up to PATH_MAX + 1 UTF-16 units. The
name is recorded as the resume name of the search, and the next
CIFSFindNext() rejects only a recorded length of PATH_MAX or more before
copying the name into the ResumeFileName of its request and sending it
with the resume key taken from the same entry: up to 4 KiB of memory
from past the end of the allocation leaks to the server, and for
SMB_FIND_FILE_UNIX the NUL scan runs about twice as far before the
length is even known. The entry does not have to lie outside the
response for that either: a maximal response ends about 120 bytes before
the end of the allocation, so an entry in its last bytes with a name
just under PATH_MAX has nearly all of that name copied from past the
allocation.
It needs nothing unusual from the client: a FindNext response that
reports no entries and does not end the search makes find_cifs_entry()
send the next FindNext at once, carrying whatever was parsed at
LastNameOffset, and a search rewind can do the same after its
FindFirst. SMB1 is not negotiated by default: this takes a malicious
or compromised server, a mount of it with an explicit vers=1.0, which
CONFIG_CIFS_ALLOW_INSECURE_LEGACY (default y) permits, and a directory
listing on that mount.
With KASAN enabled, a response that places the last entry past the
buffer, with a large DataOffset and a LastNameOffset near the end of
its range, gives:
==================================================================
BUG: KASAN: slab-out-of-bounds in cifs_save_resume_key.isra.0+0x7aa/0x7f0
Read of size 4 at addr ffff88807b55c33b by task ls/83
CPU: 1 UID: 0 PID: 83 Comm: ls Not tainted 7.3.0-rc4-00457-gf14572c203d5 #69 PREEMPT(lazy)
Call Trace:
<TASK>
kasan_report+0xdf/0x1a0
cifs_save_resume_key.isra.0+0x7aa/0x7f0
cifs_readdir+0x1ed6/0x29c0
iterate_dir+0x1c0/0x570
__x64_sys_getdents64+0x133/0x270
do_syscall_64+0x109/0x5d0
entry_SYSCALL_64_after_hwframe+0x77/0x7f
</TASK>
Allocated by task 83 on cpu 0 at 19.317556s:
cifs_buf_get+0x36/0x90
smb_init+0x4f/0x110
CIFSFindNext+0xf7/0x14c0
cifs_readdir+0xf51/0x29c0
The buggy address belongs to the object at ffff88807b558000
which belongs to the cache cifs_request of size 16588
The buggy address is located 623 bytes to the right of
allocated 16588-byte region [ffff88807b558000, ffff88807b55c0cc)
==================================================================
The four bytes read there are the FileNameLength of the entry, which
cifs_fill_dirent() takes before anything checks where the entry lies.
Ignore the last entry when it does not start inside the received
response or the response holds no entries; for a response without
entries, last_entry pointed into or past its empty data area and
cifs_save_resume_key() parsed whatever the buffer held there. The
entry is bounded by the response that was received, which is what the
buffer holds, rather than by the data area the response declares,
which a later patch checks against the received length. Record the
search handle before the last entry is examined, since a response
without a usable last entry still carries the handle that the search
is continued and closed with; leaving the assignment in the else
branch would drop the handle of every response without entries.
This bounds where the last entry starts, not how far it is parsed: an
entry that starts within the final bytes of the response still has
its fixed part read past the end of the received data and its name,
up to 4095 bytes, copied from past the end of the allocation, until
the following patch bounds the parse itself. A response without a
usable last entry continues the search with an empty resume name,
since the preceding patch, "smb: client: fix use-after-free infoleak
via the readdir resume name", drops the recorded name whenever a new
response is installed. This patch applies on top of that one and
relies on it: without it, every response rejected here would leave
the previous name in place, pointing into a buffer that has been
released.
A conforming server is not affected: the last entry lies inside the
data area of the response.
Fixes: 0752f1522a91 ("[CIFS] make sure we have the right resume info before calling CIFSFindNext")
Fixes: b77d753c413e ("[CIFS] Check that last search entry resume key is valid")
Cc: stable@vger.kernel.org
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>
|
|
CIFSFindNext() copies the resume name that cifs_save_resume_key()
recorded from the last entry of the previous response into its request;
psrch_inf->presume_name points into the response buffer that entry came
from, and nothing ever clears it. find_cifs_entry() records a new
name only from a response with a usable last entry, and cifs_readdir()
only from an entry it walks to, so a response that carries no entries
and leaves last_entry NULL, because CIFSFindFirst() or CIFSFindNext()
rejected LastNameOffset, refreshes neither and presume_name keeps
pointing into the previous response. That buffer has already been
released: by CIFSFindNext() before it installs the new response, or by
find_cifs_entry() when it rewinds the search, which does not drop the
name either and can then fail in initiate_cifs_search() before a new
response replaces it. The next CIFSFindNext() then copies up to
PATH_MAX - 1 bytes of the resume name out of the released buffer into
its request and sends them to the server: a use-after-free read
that leaks kernel memory to a malicious or compromised server, which
can provoke it with a single FindNext response that carries no entries
and a LastNameOffset above CIFSMaxBufSize, which is all the existing
check rejects.
With KASAN enabled, a server that answers a listing that way gives:
CIFS: VFS: ignoring corrupt resume name
==================================================================
BUG: KASAN: slab-use-after-free in CIFSFindNext+0x8ca/0x14c0
Read of size 4080 at addr ffff88807c823f98 by task ls/83
CPU: 0 UID: 0 PID: 83 Comm: ls Tainted: G B 7.3.0-rc4-00457-gf14572c203d5 #69 PREEMPT(lazy)
Tainted: [B]=BAD_PAGE
Call Trace:
<TASK>
kasan_report+0xdf/0x1a0
kasan_check_range+0x10f/0x1e0
__asan_memcpy+0x23/0x60
CIFSFindNext+0x8ca/0x14c0
cifs_readdir+0xf51/0x29c0
iterate_dir+0x1c0/0x570
__x64_sys_getdents64+0x133/0x270
do_syscall_64+0x109/0x5d0
entry_SYSCALL_64_after_hwframe+0x77/0x7f
</TASK>
Allocated by task 83 on cpu 1 at 13.497119s:
cifs_buf_get+0x36/0x90
smb_init+0x4f/0x110
CIFSFindNext+0xf7/0x14c0
cifs_readdir+0xf51/0x29c0
Freed by task 83 on cpu 0 at 13.584018s:
cifs_buf_release+0x41/0x80
CIFSFindNext+0xfce/0x14c0
cifs_readdir+0xf51/0x29c0
The buggy address is located 16280 bytes inside of
freed 16588-byte region [ffff88807c820000, ffff88807c8240cc)
==================================================================
The same memcpy() first reads past the end of that object while it is
still live, a slab-out-of-bounds read that the following patches
bound; that is the taint the report above carries.
SMB1 is not negotiated by default; reaching this code requires an
explicit vers=1.0 mount.
Clear presume_name and resume_name_len whenever a new response is
installed and when the rewind path releases the search buffer, so that
the name never outlives the buffer it points into, and reset
resume_key with them, since it was taken from the same entry and would
otherwise be sent with an empty name. A response without a usable
last entry then continues the search with an empty resume name rather
than a stale one; CIFSFindNext() skips the copy of a zero-length name
so that it never runs on the NULL pointer.
The next patch in the series leaves last_entry NULL for a response
that carries no entries, and bounds LastNameOffset against the
received response rather than against CIFSMaxBufSize. Responses that
are given a last entry today then take the path fixed here, so that
patch must not be applied without this one.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Fixes: b77d753c413e ("[CIFS] Check that last search entry resume key is valid")
Cc: stable@vger.kernel.org
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>
|
|
The error path of cifs_ses_add_channel() calls cifs_put_tcp_session()
before cifs_chan_clear_need_reconnect(). The latter reaches
cifs_ses_get_chan_index(), which dereferences server->terminate; if
the put was the final reference, TCP_Server_Info is already freed and
this is a use-after-free. Same put-then-use pattern as the recently
fixed sibling in commit 717e0a25036b ("cifs: Fix server use-after-free
in cifs_chan_skip_or_disable()").
Move the put last, but do not read chan->server after dropping
chan_lock: chan points into ses->chans[], and once chan_count is
decremented a concurrent add_channel can reuse the slot and overwrite
the pointer, so the put would act on the new channel's server. Cache
the pointer under chan_lock, clear the reconnect bit and decrement
chan_count while still holding the reference, and release it after
the unlock.
This issue was discovered by Tencent CodeBuddy Security.
Fixes: ee1d21794e55a ("cifs: handle when server stops supporting multichannel")
Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
Cc: stable@vger.kernel.org
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Reviewed-by: David Howells <dhowells@redhat.com>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
smb_send_kvec() keeps retrying when server needs to reconnect, which
makes no sense as cifs_reconnect() will never run in parallel (because
both need server mutex).
IOW, retrying will never succeed, but only delay reconnects further.
Bail out early from smb_send_kvec() when need to reconnect.
Also shutdown socket queues when CifsNeedReconnect is first detected,
so any sends/receives fails immediately (and don't wait for socket to
timeout).
Add TCP_Server_Info::need_sock_shutdown to ensure socket is shutdown
only once -- even though socket/TCP API will prevent multiple shutdowns,
we need to prevent, from cifs side, concurrent tasks from accidentally
shutting down the socket again in case cifs_reconnect() is successful.
kernel_sock_shutdown() will be called from whichever task detects it
first; cifsd (cifs_tcp_ses_needs_reconnect()), or sending tasks,
through cifs_call_async() or compound_send_recv().
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
Reviewed-by: David Howells <dhowells@redhat.com>
Signed-off-by: Paulo Alcantara <pc@manguebit.org>
|
|
ath12k_mac_vdev_delete() walks txmgmt_idr with idr_for_each(), and its
callback removes each entry belonging to the interface. Removing the
current entry can invalidate the radix-tree iterator retained by
idr_for_each().
Move the walk into a helper that uses idr_for_each_entry(). It performs a
fresh lookup for every iteration, so each matching entry can be removed
through the locked removal helper without retaining iterator state across
the removal.
Fixes: d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices")
Signed-off-by: Jiale Yao <yaojiale02@163.com>
Reviewed-by: Baochen Qiang <baochen.qiang@oss.qualcomm.com>
Reviewed-by: Rameshkumar Sundaram <rameshkumar.sundaram@oss.qualcomm.com>
Link: https://patch.msgid.link/20261003092104.520135-6-yaojiale02@163.com
Signed-off-by: Jeff Johnson <jeff.johnson@oss.qualcomm.com>
|
|
The pending management TX cleanup walks txmgmt_idr with idr_for_each(),
and its callback removes the current entry. This can invalidate the
radix-tree iterator retained by idr_for_each().
Use idr_for_each_entry() so every iteration starts with a fresh lookup.
Continue to remove each entry under txmgmt_idr_lock before freeing its skb,
preventing concurrent TX completion from finding a stale skb pointer.
Fixes: d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices")
Signed-off-by: Jiale Yao <yaojiale02@163.com>
Reviewed-by: Baochen Qiang <baochen.qiang@oss.qualcomm.com>
Reviewed-by: Rameshkumar Sundaram <rameshkumar.sundaram@oss.qualcomm.com>
Link: https://patch.msgid.link/20261003092104.520135-5-yaojiale02@163.com
Signed-off-by: Jeff Johnson <jeff.johnson@oss.qualcomm.com>
|
|
ath11k_mac_op_remove_interface() walks txmgmt_idr with idr_for_each(),
and its callback removes each entry belonging to the interface. Removing
the current entry can invalidate the radix-tree iterator retained by
idr_for_each().
Move the walk into a helper that uses idr_for_each_entry(). It performs a
fresh lookup for every iteration, so each matching entry can be removed
through the locked removal helper without retaining iterator state across
the removal.
Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices")
Signed-off-by: Jiale Yao <yaojiale02@163.com>
Reviewed-by: Baochen Qiang <baochen.qiang@oss.qualcomm.com>
Reviewed-by: Rameshkumar Sundaram <rameshkumar.sundaram@oss.qualcomm.com>
Link: https://patch.msgid.link/20261003092104.520135-4-yaojiale02@163.com
Signed-off-by: Jeff Johnson <jeff.johnson@oss.qualcomm.com>
|
|
The pending management TX cleanup walks txmgmt_idr with idr_for_each(),
and its callback removes the current entry. This can invalidate the
radix-tree iterator retained by idr_for_each().
Use idr_for_each_entry() so every iteration starts with a fresh lookup.
Continue to remove each entry under txmgmt_idr_lock before freeing its skb,
preventing concurrent TX completion from finding a stale skb pointer.
Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices")
Signed-off-by: Jiale Yao <yaojiale02@163.com>
Reviewed-by: Baochen Qiang <baochen.qiang@oss.qualcomm.com>
Reviewed-by: Rameshkumar Sundaram <rameshkumar.sundaram@oss.qualcomm.com>
Link: https://patch.msgid.link/20261003092104.520135-3-yaojiale02@163.com
Signed-off-by: Jeff Johnson <jeff.johnson@oss.qualcomm.com>
|
|
ath10k_htt_flush_tx_queue() walks pending_tx with idr_for_each().
Its callback calls ath10k_txrx_tx_unref(), which removes the current
entry from pending_tx through ath10k_htt_tx_free_msdu_id().
idr_for_each() keeps radix-tree iterator state across the callback.
Removing the current entry can therefore invalidate that state and
make the remaining walk unsafe.
Use idr_for_each_entry(), which starts a fresh lookup after each
callback. The current entry can then be removed safely, while the
following idr_destroy() continues to release the IDR itself.
Fixes: 89d6d83565e9 ("ath10k: use idr api for msdu_ids")
Signed-off-by: Jiale Yao <yaojiale02@163.com>
Reviewed-by: Baochen Qiang <baochen.qiang@oss.qualcomm.com>
Reviewed-by: Rameshkumar Sundaram <rameshkumar.sundaram@oss.qualcomm.com>
Link: https://patch.msgid.link/20261003092104.520135-2-yaojiale02@163.com
[changed ath10k_htt_tx_clean_up_pending() signature]
Signed-off-by: Jeff Johnson <jeff.johnson@oss.qualcomm.com>
|
|
Currently, NAPI instances and their associated netdevs allocated for AHB
external IRQ groups are not released when external IRQ configuration
fails. They are also not freed when arch_init() fails after
ath12k_ahb_config_irq() succeeds during probe, or when the driver is
unloaded.
Fix this by adding a common cleanup helper to release the NAPI instances
and associated netdevs. Call it from the external IRQ configuration error
path, probe error path, and driver cleanup path.
Tested-on: IPQ5332 hw1.0 AHB WLAN.WBE.1.6-01275-QCAHKSWPL_SILICONZ-1
Fixes: 6cee30f0da75 ("wifi: ath12k: add AHB driver support for IPQ5332")
Fixes: 614c23e24ee8 ("wifi: ath12k: Support arch-specific DP device allocation")
Signed-off-by: Aaradhana Sahu <aaradhana.sahu@oss.qualcomm.com>
Reviewed-by: Baochen Qiang <baochen.qiang@oss.qualcomm.com>
Reviewed-by: Rameshkumar Sundaram <rameshkumar.sundaram@oss.qualcomm.com>
Link: https://patch.msgid.link/20261001045423.493121-1-aaradhana.sahu@oss.qualcomm.com
[renamed label err_free_irq]
Signed-off-by: Jeff Johnson <jeff.johnson@oss.qualcomm.com>
|
|
misc.2026.10.06a: Miscellaneous fixes.
torture.2026.10.12a: Make call_{s,}rcu() invocable from any environment,
add atomic SRCU, additional SRCU updates, and torture-test updates.
|
|
Add testtype 7 to verify that lockdep detects an IRQ-context
mismatch when an atomic SRCU read-side critical section is held
with IRQs enabled on one CPU and synchronize_srcu_atomic() is
called from an IPI handler on another CPU.
This covers the cross-CPU case that cannot be detected by
checking the current task's held locks.
Co-developed-by: Zqiang <qiang.zhang@linux.dev>
Signed-off-by: Zqiang <qiang.zhang@linux.dev>
Signed-off-by: Kunwu Chan <kunwu.chan@gmail.com>
Acked-by: Boqun Feng <boqun@kernel.org>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
|
|
When CONFIG_PROVE_LOCKING is missing, the deadlock-detection loop
and the mixed-SRCU-readers section each increment nerrs once in
the configuration check and again in the common error path,
counting the same error twice.
Remove the stray increments so that each error advances nerrs
by one.
Signed-off-by: Kunwu Chan <kunwu.chan@gmail.com>
Acked-by: Boqun Feng <boqun@kernel.org>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
|
|
Add testtypes 4 (atomic SRCU same-type deadlock) and 5 (atomic SRCU
+ raw_spinlock dependency cycle) to the deadlock-detection loop.
Add a separate loop for testtype 6 (rcu_read_lock() →
synchronize_srcu_atomic()), which also verifies via console.log that
no lockdep warning is triggered.
Co-developed-by: Zqiang <qiang.zhang@linux.dev>
Signed-off-by: Zqiang <qiang.zhang@linux.dev>
Signed-off-by: Kunwu Chan <kunwu.chan@gmail.com>
Acked-by: Boqun Feng <boqun@kernel.org>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
|
|
Add three testtypes covering atomic SRCU lockdep behavior:
testtype 4: atomic SRCU same-type deadlock
testtype 5: atomic SRCU + raw spinlock dependency cycle
testtype 6: synchronize_srcu_atomic() inside rcu_read_lock()
Co-developed-by: Zqiang <qiang.zhang@linux.dev>
Signed-off-by: Zqiang <qiang.zhang@linux.dev>
Signed-off-by: Kunwu Chan <kunwu.chan@gmail.com>
Acked-by: Boqun Feng <boqun@kernel.org>
Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
|
|
Discovered in the Open Source Lab of Oregon State University when running
torture.sh on a ppc64le VM. Two bugs were exposed:
1. The kvm-transform.sh call hard-coded "bzImage" as the kernel image
name. On ppc64, the boot image is vmlinux, not bzImage, so the
re-run failed to find the image. Fix this by extracting the QEMU
binary from the qemu-cmd file and passing it to identify_boot_image()
to obtain the correct architecture-specific image name, the same way
kvm.sh already does.
2. The rm -f invocation on re-run listed vmlinux among the files to
delete. On ppc64 vmlinux is the boot image, so deleting it broke
the re-run on that architecture. Drop vmlinux from the unconditional
list, and instead conditionally delete it only when the boot image
basename is not vmlinux.
In addition, identify qemu_binary before the copy step so that on
non-PowerPC systems (where vmlinux is not the boot image) the vmlinux
file can be removed immediately after the run directory is copied,
saving storage before any tests run. This also eliminates the
per-iteration re-computation of qemu_binary and boot_image inside
the qemu-cmd transform loop.
Tested on a local x86_64 machine and on a PPC VM of the Open Source Lab
of Oregon State University.
Signed-off-by: Zhouyi Zhou <zhouzhouyi@gmail.com>
Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
|
|
Some environments require use of alternative commands to access the
test hosts. This commit therefore adds a KVM_REMOTE_SSH environment
variable for this purpose. If this variable is unset, ssh is used.
Any alternative ssh command must support the usual ssh arguments,
including the command to be executed remotely. In some cases, you may
need a wrapper script to make the alternative ssh-like command look
enough like ssh to satisfy kvm-remote.sh.
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
|
|
The rcu_torture_reader() drives RCU readers from interrupt context via
smp_call_function_single(cpu, rcu_torture_irq, NULL, 0) with wait=0, to
runs rcu_torture_irq() on a remote CPU. this is async, nothing waits for
the remote handler to run.
On shutdown, torture_stop_kthread() only waits for each reader kthread to
return, and the reader's timer_delete_sync() only drains its timer. Neither
waits for a rcu_torture_irq() which still pending or executing on a remote
CPU, so it can run after all readers have exited and rcu_torture_cleanup()
has already advanced.
1. rcu_torture_irq() may issue cur_ops->call(rhp, rcu_torture_timer_cb)
after cur_ops->cb_barrier() has been waiting for all outstanding
callbacks complete. once the module is unloaded, fires into freed
module text, a use-after-free happen.
2. rcu_torture_irq() may still be inside rcu_torture_one_read(), holding
a read-side critical section, when cur_ops->cleanup() tears the flavor
down (e.g. cleanup_srcu_struct()), triggering an active-reader warning
or use-after-free of the torn-down structure.
This commit therefore issue a kick_all_cpus_sync() after all readers
kthread have returned and before cur_ops->cb_barrier(), synchronous IPI
round trip to every CPU guarantees that every rcu_torture_irq() which
previously issued by any reader has completed.
Signed-off-by: Zqiang <qiang.zhang@linux.dev>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
|
|
When fwd_progress_div=1, the forward-progress test computes:
sd4 = (sd + div - 1) / div = sd
dur = sd4 + torture_random(&trs) % (sd - sd4) = sd4 + % 0
The modulo operation with a zero divisor triggers an integer
division by zero (undefined behavior at the C level, #DE trap on x86),
causing a kernel Oops and panic. On x86_64, this manifests as:
rcu_torture_fwd_prog_nr: Starting forward-progress test 0
Oops: divide error: 0000 [#1] SMP PTI
RIP: 0010:rcu_torture_fwd_prog+0x90b/0x1160
R12: 0000000000000000
The existing guard only handles non-positive values. However,
fwd_progress_div=1 also makes the random range empty because
sd4 == sd.
Change the guard to reject values below 2. The forward-progress test
only reaches this calculation when stall_dur() is positive, so
sd = stall_dur() + 1 >= 2. For fwd_progress_div >= 2, sd4 < sd,
ensuring that sd - sd4 is at least 1.
Keep the existing fallback to the default value of 4 for invalid
values.
Verified with QEMU/KVM: a 138-second run with fwd_progress_div=1
completed 81 forward-progress test cycles without a crash.
Fixes: 1b27291b1ea4f ("rcutorture: Add forward-progress tests for RCU grace periods")
Signed-off-by: Kunwu Chan <kunwu.chan@gmail.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
|
|
synchronize_srcu_atomic() may be called with interrupts disabled, but
its callchain uses raw_spin_lock_irq_rcu_node() /
raw_spin_unlock_irq_rcu_node(), which unconditionally enables
interrupts on unlock. This can incorrectly change the caller's IRQ
state.
Use the irqsave/irqrestore variants for the locks in this callchain
so that the caller's IRQ state is preserved.
Co-developed-by: Zqiang <qiang.zhang@linux.dev>
Signed-off-by: Zqiang <qiang.zhang@linux.dev>
Signed-off-by: Kunwu Chan <kunwu.chan@gmail.com>
Acked-by: Boqun Feng <boqun@kernel.org>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
|
|
Add the RCU_LOCKDEP_WARN and early-boot check that the Tree version
gained, srcu_lock_sync() was already present.
Suggested-by: Zqiang <qiang.zhang@linux.dev>
Signed-off-by: Kunwu Chan <kunwu.chan@gmail.com>
Reviewed-by: Bradley Morgan <brads@mainlining.org>
Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
|
|
Add the lockdep annotation, same-type SRCU nesting warning, and
early-boot check used by __synchronize_srcu().
Suggested-by: Zqiang <qiang.zhang@linux.dev>
Signed-off-by: Kunwu Chan <kunwu.chan@gmail.com>
Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
|
|
Atomic SRCU does not use the srcu_node combining tree. Exclude it
from the SRCU_SIZING_IS_TORTURE() transition in
srcu_torture_stats_print(), and warn if such a transition is requested.
Signed-off-by: Kunwu Chan <kunwu.chan@gmail.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
|
|
Atomic SRCU must remain in the small size state. Warn if this
invariant is violated and avoid transitioning to big in that case.
[ paulmck: Folded "if" onto one line for 100-character limit. ]
Signed-off-by: Kunwu Chan <kunwu.chan@gmail.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
Reviewed-by: Bradley Morgan <brads@mainlining.org>
|
|
The is_atomic parameter of init_srcu_struct_fields() exists so that
atomic SRCU never transitions to big, but neither
init_srcu_struct_atomic() nor its lockdep counterpart
__init_srcu_struct_atomic() sets it.
On systems where srcutree.convert_to_big selects SRCU_SIZING_INIT,
this needlessly allocates a full srcu_node combining tree for any
dynamically initialized atomic srcu_struct, despite atomic SRCU
having neither callbacks nor srcu_barrier() operations.
Pass true from both atomic entry points, adding an is_atomic parameter
to __init_srcu_struct_common().
Signed-off-by: Kunwu Chan <kunwu.chan@gmail.com>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
|
