| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
A node receiving a DLM_MIG_LOCKRES message trusts several fields of the
peer-supplied dlm_migratable_lockres without validation. num_locks and
lockname_len are bounded only on the sending side, and the message is
never checked to actually carry num_locks migratable_lock entries. As a
result dlm_process_recovery_data() walks mres->ml[0..num_locks) past the
kmalloc(data_len) copy of the message (an out-of-bounds read that ends in
a BUG_ON panic), and dlm_init_lockres() copies lockname_len bytes into the
fixed 32-byte o2dlm_lockname slab object (a heap out-of-bounds write).
Both are reachable by any node in the domain.
Validate these fields right after dlm_grab(), before anything uses them --
including the not-joined error path, which already prints mres->lockname
with the unbounded lockname_len as a %.*s precision. Reject the message
unless lockname_len <= DLM_LOCKID_NAME_MAX, num_locks <=
DLM_MAX_MIGRATABLE_LOCKS (the bound the sender already asserts), and the
payload is large enough to hold the claimed locks. Conforming recovery
and migration messages are unaffected.
Link: https://lore.kernel.org/20260629-b4-disp-94fb6521-v1-2-6953bcc0421f@proton.me
Fixes: 6714d8e86bf4 ("[PATCH] OCFS2: The Second Oracle Cluster Filesystem")
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
Cc: Mark Fasheh <mark@fasheh.com>
Cc: Joel Becker <jlbec@evilplan.org>
Cc: Junxiao Bi <junxiao.bi@oracle.com>
Cc: Changwei Ge <gechangwei@live.cn>
Cc: Jun Piao <piaojun@huawei.com>
Cc: Heming Zhao <heming.zhao@suse.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
|
|
Patch series "ocfs2/dlm: bound peer-controlled lengths in the o2dlm".
The o2dlm receive handlers trust u8 length and count fields from the wire
without bounding them, so a node in a DLM domain can corrupt or panic any
other node with a malformed message. Three defects:
- dlm_migrate_request_handler() passes migrate->namelen unchecked to
dlm_init_mle(), which memcpy()s it into the 32-byte mname[] of an
o2dlm_mle slab object: a heap out-of-bounds write of up to ~215
attacker-controlled bytes.
- dlm_mig_lockres_handler() passes mres->lockname_len unchecked to
dlm_init_lockres(), which memcpy()s it into the 32-byte o2dlm_lockname
slab object: a heap out-of-bounds write of up to ~223 bytes.
- the same handler trusts mres->num_locks without checking that the
message is large enough to hold that many entries, so
dlm_process_recovery_data() walks mres->ml[] past the kmalloc(data_len)
copy and trips a BUG_ON (an out-of-bounds read ending in a panic).
The other o2dlm receive handlers already reject an oversized name; the
migration and recovery handlers have omitted it since the DLM was added
(see the Fixes tags). Patch 1 bounds namelen; patch 2 validates
lockname_len, num_locks, and the payload size. Conforming recovery and
migration traffic is unaffected.
o2net authenticates peers only by the DLM domain key, so any node that has
joined the domain -- including a compromised or malicious member -- can
send these messages. There is no local trigger; the attacker must already
be a member of the cluster.
Each sink was confirmed under KASAN with an out-of-tree module mirroring
it exactly -- a kmem_cache/kmalloc of the real destination size, then the
same unclamped memcpy/loop: slab-out-of-bounds Write for the two writes,
Read for the recovery walk, and a panic. A userspace AddressSanitizer
build faults identically under -m32 and -m64. Scrubbed logs are available
on request.
I reported this privately to security@kernel.org and the ocfs2 maintainers
on 2026-06-20; with no response after the standard embargo period I am
posting the fix publicly. I have no embargo requirement.
This patch (of 2):
A node receiving a DLM_MIGRATE_REQUEST message trusts the peer-supplied
name length (migrate->namelen) without bounding it. dlm_init_mle() then
copies that many bytes into the fixed DLM_LOCKID_NAME_MAX-byte mname[]
array of an o2dlm_mle slab object, so a malformed message from a cluster
peer overflows the slab object by up to ~215 bytes: a heap out-of-bounds
write of attacker-controlled data, reachable by any node in the domain.
Reject an oversized name, the way dlm_master_request_handler() and the
other o2dlm receive handlers already do; the migration handler omits the
check entirely. Conforming messages are unaffected.
Link: https://lore.kernel.org/20260629-b4-disp-94fb6521-v1-0-6953bcc0421f@proton.me
Link: https://lore.kernel.org/20260629-b4-disp-94fb6521-v1-1-6953bcc0421f@proton.me
Fixes: 6714d8e86bf4 ("[PATCH] OCFS2: The Second Oracle Cluster Filesystem")
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
Cc: Changwei Ge <gechangwei@live.cn>
Cc: Heming Zhao <heming.zhao@suse.com>
Cc: Joel Becker <jlbec@evilplan.org>
Cc: Jun Piao <piaojun@huawei.com>
Cc: Junxiao Bi <junxiao.bi@oracle.com>
Cc: Mark Fasheh <mark@fasheh.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
|
|
glibc 2.42 added __attribute_const__ to sys/cdefs.h:
# define __attribute_const__ __attribute__ ((__const__))
GCC 15 warns when a macro is redefined to a different replacement list
(-Wbuiltin-macro-redefined). Since host tool Makefiles (resolve_btfids,
objtool) pass -Werror, this conflict becomes fatal.
The warning is suppressed on standard native builds because GCC treats
/usr/include as a system header path (-isystem), and macro-redefinition
warnings from system headers are silently suppressed by GCC. It fires
when glibc headers are on a regular include path (-I) instead, which
is the case in cross-compilation setups such as NixOS, where the
sysroot's glibc is passed explicitly via -I rather than -isystem.
Per (C11 6.10.3), identical replacement lists are accepted silently.
Match the glibc definition exactly, including the space before "((", so
the redefinition is accepted without warning regardless of whether
glibc headers are treated as system or non-system includes.
Link: https://lore.kernel.org/20260701200635.3992767-1-rkr0k0r@gmail.com
Signed-off-by: Joy H.J. Lee <rkr0k0r@gmail.com>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: David Laight <david.laight.linux@gmail.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
|
|
Link: https://lore.kernel.org/20260701011153.93426-1-sarasena.adr@gmail.com
Signed-off-by: Sara Sena <sarasena.adr@gmail.com>
Cc: Fan Yu <fan.yu9@zte.com.cn>
Cc: Wang Yaxin <wang.yaxin@zte.com.cn>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
|
|
In has_pending_signals there was a switch/case used for optimizations.
However, today's compilers perform loop unrolling efficiently, thus it is
not needed anymore.
Put i inside the for declaration so we do not risk its escape from the
scope. Moreover, i starts now from 0 and counts up, as it is a more usual
pattern.
Link: https://lore.kernel.org/20260520062849.183621-2-andrea.calabrese@amarulasolutions.com
Signed-off-by: Andrea Calabrese <andrea.calabrese@amarulasolutions.com>
Acked-by: Oleg Nesterov <oleg@redhat.com>
Cc: Adrian Huang <adrianhuang0701@gmail.com>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Marco Elver <elver@google.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
|
|
fat_ent_write() may have committed the new link to the primary FAT but
then failed on the mirror copy, leaving the chain pointing to new_dclus
even though the caller will free it. Restore the original value to keep
the chain consistent.
Link: https://lore.kernel.org/20260525085649.781643-1-n05ec@lzu.edu.cn
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Yemu Lu <prcups@krgm.moe>
Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
Reported-by: Yuan Tan <yuantan098@gmail.com>
Reported-by: Yifan Wu <yifanwucs@gmail.com>
Reported-by: Juefei Pu <tomapufckgml@gmail.com>
Reported-by: Xin Liu <bird@lzu.edu.cn>
Acked-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
Cc: Christian Brauner <brauner@kernel.org>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
|
|
Add a KUnit test suite for the polynomial_calc() function, which had no
in-kernel test coverage. The tests verify correct evaluation of constant,
linear, quadratic, and cubic polynomials, including negative coefficients,
negative input data, zero-coefficient terms.
The Kconfig entry uses 'select POLYNOMIAL' rather than 'depends on
POLYNOMIAL' because POLYNOMIAL is a promptless tristate that cannot
be manually enabled on UML without an explicit selector.
Link: https://lore.kernel.org/20260606030319.316752-1-adinata.softwareengineer@gmail.com
Signed-off-by: Adi Nata <adinata.softwareengineer@gmail.com>
Cc: Maxim Kaurkin <maxim.kaurkin@baikalelectronics.ru>
Cc: Serge Semin <Sergey.Semin@baikalelectronics.ru>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Brendan Higgins <brendan.higgins@linux.dev>
Cc: David Gow <david@davidgow.net>
Cc: Rae Moar <raemoar63@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
|
|
send_signal_locked() unconditionally copies siginfo before the namespace
translation to avoid corrupting a shared siginfo.
Not that I think this can actually hurt performance-wise, just it doesn't
look clean to me; the copy is only needed in the unlikely case when the
translation will actually change something.
Defer it to the two cases where si_pid/si_uid are rewritten, and while at
it add #ifdef's just for completeness.
Link: https://lore.kernel.org/akkaAgNfUby5_3nM@redhat.com
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Reviewed-by: Bradley Morgan <include@grrlz.net>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Eric Biederman <ebiederm@xmission.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
|
|
prepare_kill_siginfo(PIDTYPE_TGID) fills si_code = SI_USER and sets
si_pid/si_uid in the sender's namespace. Then send_signal_locked()
translates si_pid/si_uid to the target's namespace.
SEND_SIG_NOINFO exists precisely for the case when si_code == SI_USER
and si_pid/si_uid are the sender's ids; this is exactly what sys_kill()
does via prepare_kill_siginfo(PIDTYPE_TGID). Change sys_kill() to use
it directly.
SEND_SIG_NOINFO produces the same result: si_code = SI_USER, and
__send_signal_locked() computes si_pid/si_uid directly in the target's
namespace. The force computation is also the same: both check if the
sender is visible in the target's pid namespace.
This is just a cleanup and microoptimization (especially with [1]),
this skips the has_si_pid_and_uid() block in send_signal_locked() and
offloads the namespace translation logic to
__send_signal_locked(SEND_SIG_NOINFO) which uses the simpler
computations.
NOTE: As a "side effect" this also fixes the kill(pid < 0, sig) case
where send_signal_locked() rewrites si_pid/si_uid in the shared
siginfo, corrupting it for subsequent recipients. But for other group
senders like __kill_pgrp_info() we still need the fix from Bradley
Morgan [1] who found this problem.
TODO: kill prepare_kill_siginfo() and change other users to use
SEND_SIG_NOINFO too. This needs trivial changes in
__send_signal_locked() and TP_STORE_SIGINFO().
Link: https://lore.kernel.org/aj6btAZqYuv59a8w@redhat.com
Link: https://lore.kernel.org/all/20260622164029.11474-1-include@grrlz.net/ [1]
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Reviewed-by: Bradley Morgan <include@grrlz.net>
Cc: Eric Biederman <ebiederm@xmission.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
|
|
send_signal_locked() rewrites sender ids for the target namespace. Group
sends reuse the same siginfo, so one recipient can affect the next.
Copy the siginfo before changing it.
Link: https://lore.kernel.org/86a8857d58d43ee26a8b365b837fd24830343494.1782159692.git.include@grrlz.net
Fixes: 7a0cf094944e ("signal: Correct namespace fixups of si_pid and si_uid")
Signed-off-by: Bradley Morgan <include@grrlz.net>
Acked-by: Oleg Nesterov <oleg@redhat.com>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Adrian Huang <adrianhuang0701@gmail.com>
Cc: Aleksandr Nogikh <nogikh@google.com>
Cc: Christian Brauner <brauner@kernel.org>
Cc: Marco Elver <elver@google.com>
Cc: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
|
|
Fix misspellings of "softlockup" in the watchdog enabled bit definitions
and related comments. Also fix a nearby "successful" typo.
No functional change.
Link: https://lore.kernel.org/20260615174557.1836562-1-edcr1790@gmail.com
Signed-off-by: Matthew Chen <edcr1790@gmail.com>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Reviewed-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
|
|
Inside parse_one(), the core parameter-parsing engine prints the address
of the parameter-set callback function using %p: pr_debug("handling %s
with %p\n", param, params[i].ops->set);
Since the string value of the parameter being parsed (val) is already
available, print the parameter name and its value instead, and avoid
tainting the kernel by exposing a kernel-ptr.
Link: https://lore.kernel.org/20260618190715.3563047-1-jim.cromie@gmail.com
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
|
|
memchr_inv() takes a size_t length but counts 8 byte words in an unsigned
int. At 32GiB that count wraps, so the scan can quietly miss most of the
range.
Use size_t for the word count.
Link: https://lore.kernel.org/20260621121133.16460-1-include@grrlz.net
Fixes: 798248206b59 ("lib/string.c: introduce memchr_inv()")
Signed-off-by: Bradley Morgan <include@grrlz.net>
Cc: Akinbou Mita <akinobu.mita@gmail.com>
Cc: Andy Shevchenko <andy@kernel.org>
Cc: Christoph Lameer <cl@linux-foundation.org>
Cc: Joern Engel <joern@logfs.org>
Cc: Kees Cook <kees@kernel.org>
Cc: Pekka Enberg <penberg@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
|
|
PREEMPT_RT's rtmutex PI chain walker warns about a lock dependency cycle
when inode_lock(orphan_dir_inode) is called while holding
inode_lock(file_inode):
ocfs2_file_write_iter()
inode_lock(file_inode) [class 0]
ocfs2_dio_end_io_write()
ocfs2_del_inode_from_orphan()
inode_lock(orphan_dir_inode) [class 0] <- warning!
However this is a false positive. write_iter() is never called on a
directory, and orphan_dir is always a directory, so these two locks can
never actually conflict in practice.
Fix by using inode_lock_nested(orphan_dir_inode, I_MUTEX_NONDIR2) in all
three places where orphan_dir_inode is locked in namei.c, placing it in a
separate lock class so the rtmutex PI chain walker understands these locks
have distinct roles and does not warn about their ordering.
Link: https://lore.kernel.org/20260620231223.46588-1-kartikey406@gmail.com
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
Suggested-by: Matthew Wilcox <willy@infradead.org>
Reported-by: syzbot+ce129763ce7d7e914739@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=ce129763ce7d7e914739
Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
Cc: Mark Fasheh <mark@fasheh.com>
Cc: Joel Becker <jlbec@evilplan.org>
Cc: Junxiao Bi <junxiao.bi@oracle.com>
Cc: Changwei Ge <gechangwei@live.cn>
Cc: Jun Piao <piaojun@huawei.com>
Cc: Heming Zhao <heming.zhao@suse.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
|
|
o2nm_node_local_store() handles local=0 by stopping o2net and setting
cl_local_node to O2NM_INVALID_NODE_NUM, but it leaves cl_has_local set.
That stale state makes o2nm_this_node() return 255, blocks a later local=1
attempt with -EBUSY, and can feed 255 to heartbeat users that call
o2nm_this_node() dynamically.
Clearing cl_has_local is required when the local node is reset. But
heartbeat threads can still be running at that point. They pin the local
node config item at startup, yet o2hb_do_disk_heartbeat() and thread
teardown re-read o2nm_this_node() for the local slot and for
o2nm_undepend_this_node(). Once local=0 has cleared the live local-node
state, those dynamic reads return O2NM_MAX_NODES, which is also the
invalid node number 255.
Store the local node number in the heartbeat region when the region
starts. Use that stable node for heartbeat slot writes/checks,
negotiation messages, and the final configfs undepend. Stop the heartbeat
loop when the current local node no longer matches the stored node, and
clear cl_has_local together with cl_local_node in the local=0 path so
nodemanager state matches node removal.
Validation reproduced this kernel report:
KASAN slab-out-of-bounds in o2hb_do_disk_heartbeat+0x372/0xb30
RIP: 0010:memset+0xf/0x20
Read of size 8
Call trace:
dump_stack_lvl+0x66/0xa0
print_report+0xd0/0x630
o2hb_do_disk_heartbeat+0x372/0xb30 (fs/ocfs2/cluster/heartbeat.c:1079)
srso_alias_return_thunk+0x5/0xfbef5
__virt_addr_valid+0x188/0x2f0
kasan_report+0xe4/0x120
o2hb_do_disk_heartbeat+0x5/0xb30 (fs/ocfs2/cluster/heartbeat.c:1079)
o2hb_thread+0x14e/0x770
kthread_affine_node+0x139/0x180
lockdep_hardirqs_on_prepare+0xda/0x190
trace_hardirqs_on+0x18/0x130
kthread+0x19d/0x1e0
ret_from_fork+0x37a/0x4d0
__switch_to+0x2d5/0x6f0
ret_from_fork_asm+0x1a/0x30
Link: https://lore.kernel.org/20260616074931.3774929-1-zzzccc427@gmail.com
Fixes: a7f6a5fb4bde ("[PATCH] OCFS2: The Second Oracle Cluster Filesystem")
Assisted-by: Codex:gpt-5.5
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
Suggested-by: Joseph Qi <joseph.qi@linux.alibaba.com>
Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
Cc: Mark Fasheh <mark@fasheh.com>
Cc: Joel Becker <jlbec@evilplan.org>
Cc: Junxiao Bi <junxiao.bi@oracle.com>
Cc: Changwei Ge <gechangwei@live.cn>
Cc: Jun Piao <piaojun@huawei.com>
Cc: Heming Zhao <heming.zhao@suse.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
|
|
The "resource sanity check" warning need not mean that there is a
functional issue and in the cases when there isn't one, it is just useless
and confusing noise.
For this reason, downgrade the log level of it from "warn" to "debug".
This reduces log clutter while keeping the diagnostic information
available for debugging purposes if needed.
Link: https://lore.kernel.org/20260523000131.7086-1-sakunix@yahoo.com
Signed-off-by: Manuel Quintero Fonseca <sakunix@yahoo.com>
Cc: Andriy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Hans de Goede <hansg@kernel.org>
Cc: Mika Westeberg <mika.westerberg@linux.intel.com>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
|
|
vhost_scsi_setup_vq_cmds() runs from VHOST_SCSI_SET_ENDPOINT and allocates
each command's protection scatterlist array (prot_sgl) according to the
acknowledged VIRTIO_SCSI_F_T10_PI bit. The command pools are not rebuilt
when VHOST_SET_FEATURES changes that bit later.
Although virtio feature bits must not change after feature negotiation,
vhost_scsi_set_features() currently accepts such a request after the
endpoint is active and updates acked_features. Enabling T10-PI after
endpoint setup therefore leaves prot_sgl NULL while the I/O path follows
the new feature bit.
For a 129-page protection payload, vhost_scsi_mapal() passes the missing
first chunk to sg_alloc_table_chained():
sg_alloc_table_chained(table, 129, first_chunk=NULL,
nents_first_chunk=inline_sg_cnt)
sg_pool_index() then hits:
BUG_ON(nents > SG_CHUNK_SIZE); /* 129 > 128 */
The kernel reported the following call trace and register state:
Call Trace:
<TASK>
? __sg_alloc_table+0x1d8/0x250
? __pfx_vhost_run_work_list+0x10/0x10 [vhost]
sg_alloc_table_chained+0x59/0xf0
? __pfx_sg_pool_alloc+0x10/0x10
? vhost_scsi_calc_sgls.constprop.0+0x43/0x60 [vhost_scsi]
vhost_scsi_handle_vq+0xf02/0x1700 [vhost_scsi]
? __pfx_vhost_scsi_handle_vq+0x10/0x10 [vhost_scsi]
vhost_scsi_handle_kick+0x37/0x50 [vhost_scsi]
vhost_run_work_list+0x8e/0xd0 [vhost]
vhost_task_fn+0xe1/0x210
ret_from_fork+0x348/0x540
</TASK>
RIP: 0010:0x4
CR2 = 0x4
RSP: 0018:ffffc90000dbf940 EFLAGS: 00010202
RAX: ffffffff82396810 RBX: ffff88811dc28b80 RCX: 0000000000000000
RDX: 0000000000000000 RSI: 0000000000000820 RDI: 0000000000000081
VHOST_F_LOG_ALL is a vhost-specific runtime feature and remains the only
exception.
Reject changes to any feature other than VHOST_F_LOG_ALL while the
endpoint is active. This preserves the existing runtime log toggle while
preventing feature-dependent command resources and data-path state from
becoming inconsistent. Userspace must clear the endpoint before changing
any other negotiated feature and set the endpoint up again afterward.
Fixes: bf2d650391be ("vhost-scsi: Allocate T10 PI structs only when enabled")
Signed-off-by: Jia Jia <physicalmtea@gmail.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Message-ID: <20260726144314.1652934-1-physicalmtea@gmail.com>
|
|
vhost-scsi translates guest response descriptors into userspace iovecs
when commands are submitted. Target-core completes those commands
asynchronously, so VHOST_SET_MEM_TABLE can replace the memory table while
an in-flight command still retains response iovecs translated through the
old table.
If the old mapping is reused after VHOST_SET_MEM_TABLE returns, command
completion can write the response to an unrelated userspace object.
Flush the vhost-scsi backend after vhost_dev_ioctl() handles a device
ioctl. This waits for in-flight commands that can still use the old
response iovecs before the ioctl returns.
Signed-off-by: Jia Jia <physicalmtea@gmail.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Message-ID: <20260724060919.1569170-1-physicalmtea@gmail.com>
|
|
vq->meta_iotlb[] caches the vhost_iotlb_map that backs each vring
metadata region, and iotlb_access_ok() returns early on a cache hit,
taking the hit as proof that the region has already been validated:
if (vhost_vq_meta_fetch(vq, addr, len, type))
return true;
The cache is reset on VHOST_IOTLB_UPDATE and VHOST_IOTLB_INVALIDATE, on
device IOTLB (re)initialisation and on vq reset, but not when
VHOST_SET_VRING_ADDR replaces vq->desc, vq->avail and vq->used, nor when
VHOST_SET_VRING_NUM changes the region sizes.
With a device IOTLB attached both ioctls are accepted while the vq is
live, and neither validates the addresses at ioctl time: vq_access_ok()
and vq_log_used_access_ok() return true early because the addresses are
GIOVAs, deferring validation to prefetch time. Once the cache has been
populated that deferred validation no longer runs -- vq_meta_prefetch()
hits the stale entry and returns true -- and vhost_vq_meta_fetch() keeps
translating through the old mapping as
map->addr + addr - map->start
for an address the mapping no longer covers. vhost_copy_to_user() and
vhost_copy_from_user() consume the result with __copy_to_user() and
__copy_from_user(), which do not check it either, so a subsequent used
ring update or descriptor fetch accesses memory outside the region the
IOTLB actually maps.
Reset the metadata cache whenever the vring is reconfigured, so the new
addresses are pushed back through iotlb_access_ok()'s slow path.
Fixes: f88949138058 ("vhost: introduce O(1) vq metadata cache")
Cc: stable@vger.kernel.org
Assisted-by: tencentos-corvus-ai:kimi-k3
Signed-off-by: Jun Yang <junvyyang@tencent.com>
Message-ID: <20260803014823.68623-1-juny24602@gmail.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
|
|
When T10 PI is negotiated, vhost-scsi splits protection bytes from
the data iterator before mapping the request scatterlists. A malformed
request can claim protection bytes that cover or exceed the full payload
length. The former leaves no data bytes to map, while the latter
underflows exp_data_len before advancing the iterator. Both cases can let
a zero data SGL count reach sg_alloc_table_chained(), which triggers
BUG_ON(!nents).
Reject protection lengths that cover or exceed the payload before
subtracting prot_bytes and advancing the iterator. Also propagate
negative errors from the protection SGL calculation before calling the
allocator, matching the data SGL path.
Fixes: bca939d5bcd0 ("vhost-scsi: Dynamically allocate scatterlists")
Suggested-by: Jia Jia <physicalmtea@gmail.com>
Signed-off-by: Jia Jia <physicalmtea@gmail.com>
Assisted-by: OpenAI-Codex:GPT-5
Signed-off-by: Linfeng Sun <linfeng.sun.dev@gmail.com>
Message-ID: <20260727081841.923151-1-slf@hdu.edu.cn>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
|
|
Batch AUTO_BUF_REG COMMIT must unregister the old auto_buf index before
storing the next one. Fixed per-tag indexing (A == B) masks bugs that
clear after overwriting io->buf.
Add kublk --rotate_auto_buf so each tag alternates between two sparse
buffer indices, and test_batch_04.sh to exercise that path. Without the
driver fix, the request ref stays stuck and I/O hangs; the test uses a
short timeout and kills the ublk daemon to recover. With the fix, a
small write completes quickly.
Signed-off-by: Yang Xiuwei <yangxiuwei@kylinos.cn>
Reviewed-by: Ming Lei <tom.leiming@gmail.com>
Link: https://patch.msgid.link/20260730024050.1062354-1-yangxiuwei@kylinos.cn
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
With UBLK_F_AUTO_BUF_REG, invalid sqe->addr can fail after
ublk_fill_io_cmd() has set UBLK_IO_FLAG_ACTIVE. The uring_cmd is
completed while the tag stays active, which can hang teardown.
Split validation from buffer apply so the check has no side effects,
then take the uring_cmd and store the already-validated buffer. Apply
the same order in FETCH so io->buf is not written before __ublk_fetch()
state checks.
Fixes: 52460dda3a77 ("ublk: move auto buffer register handling into one dedicated helper")
Suggested-by: Caleb Sander Mateos <csander@purestorage.com>
Signed-off-by: Yang Xiuwei <yangxiuwei@kylinos.cn>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
ublk_map_io() and ublk_unmap_io() are no-ops for ublk devices that
enable user copy or zero copy, as well as for requests without data to
copy in the given direction. However, the implementation is a bit
convoluted, returning the full request data length and relying on the
caller to check the return value against the request length.
UBLK_F_SHMEM_ZC recently added branches to skip the ublk_{,un}map_io()
call for I/Os using a shared-memory buffer. This is a more logical place
for the device need_map and the ublk_need_{,un}map_req() checks, so move
them there from ublk_{,un}map_io().
Checking these conditions early also skips the expensive pointer-chasing
for the ublk_iod_is_shmem_zc() check in __ublk_complete_rq() for the
common case of a ublk device using user copy or zero copy.
Drop the req_op() filter in __ublk_complete_rq(), as it's redundant with
the ublk_need_unmap_req() check.
Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
Reviewed-by: Ming Lei <tom.leiming@gmail.com>
Link: https://patch.msgid.link/20260803211441.2538144-9-csander@purestorage.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
Add test loop_08, which creates a ublk device with UBLK_F_IO_DESC_SIZE
enabled and io_desc_size set to 64. The test issues verified I/O to the
device using fio.
Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
Link: https://patch.msgid.link/20260803211441.2538144-8-csander@purestorage.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
Add an optional --io_desc_size argument to the kublk add/recover
commands to enable UBLK_F_IO_DESC on the ublk device. The mmap()
arguments and ublk_get_iod() computation are adjusted accordingly.
Display the configured io_desc_size in the kublk list output for ublk
devices with UBLK_F_IO_DESC.
Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
Link: https://patch.msgid.link/20260803211441.2538144-7-csander@purestorage.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
ublk passes the parameters of incoming I/O in memory shared between the
kernel ublk driver and userspace ublk server in struct ublksrv_io_desc.
The size of this struct is currently fixed to 24 bytes, which has been
an obstacle to extending it with additional fields [1]. Additionally,
with multiple ublk server threads handling I/Os from the same ublk queue
(possible with UBLK_F_PER_IO_DAEMON or UBLK_F_BATCH_IO), false sharing
results from adjacent io_descs sharing the same cache line.
Add a ublk feature UBLK_F_IO_DESC_SIZE to allow a ublk server to
override the size of each io_desc. The size must be at least 24 and a
multiple of 8 to store a properly-aligned struct ublksrv_io_desc. It's
also limited to a maximum of 256, though this bound could be lifted in
the future.
The struct ublksrv_io_desc is located at the beginning of each io_desc
and the remainder is padding. The mmap() performed for each queue must
have a length of queue_depth * io_desc_size rounded up to the page size.
The mmap() offset must be q_id * UBLK_MAX_QUEUE_DEPTH * io_desc_size,
also rounded up to the page size.
[1]: https://lore.kernel.org/linux-block/aV8QfvaNO5P6vOs6@fedora/
Suggested-by: Ming Lei <ming.lei@redhat.com>
Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
Link: https://patch.msgid.link/20260803211441.2538144-6-csander@purestorage.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
ublk_setup_iod() is currently called to populate struct ublksrv_io_desc
on the thread submitting I/O to a ublk device. However, only the ublk
server threads read the io_descs. This basically guarantees a cache miss
on both threads for each ublk I/O. There's really no need to initialize
the io_descs on the submitting thread. Move the ublk_setup_iod() call to
ublk_dispatch_req() (for non-UBLK_F_BATCH_IO) and
__ublk_batch_prep_dispatch() (for UBLK_F_BATCH_IO), which runs on the
ublk server daemon thread before dispatching the I/O to userspace.
Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
Reviewed-by: Ming Lei <tom.leiming@gmail.com>
Link: https://patch.msgid.link/20260803211441.2538144-5-csander@purestorage.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
In preparation for moving the struct ublksrv_io_desc initialization from
the thread submitting ublk requests to the daemon thread receiving them,
split the fallible part of ublk_setup_iod{,_zoned}() into new helper
ublk_validate_req{,_zoned}(). Only ublk_setup_iod{,_zoned}() accesses
the io_desc and cannot error out.
Return a bool value from ublk_validate_req{,_zoned}() as the existing
error code ublk_setup_iod{,_zoned}() returns is only checked against
BLK_STS_OK.
Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
Reviewed-by: Ming Lei <tom.leiming@gmail.com>
Link: https://patch.msgid.link/20260803211441.2538144-4-csander@purestorage.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
struct ublk_zoned_report_desc's operation field is only ever set to
UBLK_IO_OP_REPORT_ZONES, so remove it. Replace its one load with the
constant.
Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
Reviewed-by: Ming Lei <tom.leiming@gmail.com>
Link: https://patch.msgid.link/20260803211441.2538144-3-csander@purestorage.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
The u16 nr_hw_queues and queue_depth fields of the ublk UAPI struct
ublksrv_ctrl_dev_info constrain the number of queues and queue depth of
each ublk device. However, the ublk driver is a bit inconsistent with
the type it uses to represent these values, mixing u16 with int and
unsigned int. Change all queue number, queue depth, q_id, and tag
variables/fields to u16 to save some space.
Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
Reviewed-by: Ming Lei <tom.leiming@gmail.com>
Link: https://patch.msgid.link/20260803211441.2538144-2-csander@purestorage.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
io->res is set from struct ublksrv_io_cmd's result field, which is
controlled by the ublk server process, without any validation. It's thus
possible for userspace to trigger the io->res > rq_bytes warning.
ublk_copy_user_pages() already limits the copy length to the request
data length, so drop the warning.
Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
Link: https://patch.msgid.link/20260729171041.45061-4-csander@purestorage.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
If the userspace ublk server passes an unmapped address as the data
buffer for a completed ublk read, ublk_unmap_io() will return 0
indicating no bytes could be copied. Currently, this will result in
calling blk_update_request() with nr_bytes=0, which doesn't seem
supported. Fail the I/O with BLK_STS_IOERR in this case instead.
Fixes: 71f28f3136af ("ublk_drv: add io_uring based userspace block driver")
Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
Link: https://patch.msgid.link/20260729171041.45061-3-csander@purestorage.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
import_ubuf() can fail if the address range (provided by the userspace
ublk server) is outside the allowed user address space. Return that 0
bytes were copied if import_ubuf() fails rather than passing an
uninitialized struct iov_iter to ublk_copy_user_pages().
Fixes: 981f95a571e3 ("ublk: cleanup ublk_copy_user_pages")
Reported-by: Ming Lei <tom.leiming@gmail.com>
Signed-off-by: Caleb Sander Mateos <csander@purestorage.com>
Link: https://patch.msgid.link/20260729171041.45061-2-csander@purestorage.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
bfqg_prfill_stat_recursive() and bfq_bio_bfqg() read blkg->online
locklessly, same as blkg_rwstat_recursive_sum(). Annotate with
data_race() to silence KCSAN.
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
Reviewed-by: Yu Kuai <yukuai@fygo.io>
Link: https://patch.msgid.link/20260803134104.138411-1-cui.tao@linux.dev
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
Dipayaan Roy says:
====================
net: mana: add ethtool private flag for full-page RX buffers
On some ARM64 platforms with 4K PAGE_SIZE, utilizing page_pool
fragments for allocation in the RX refill path (~2kB buffer per
fragment) causes 15-20% throughput regression under high connection
counts (>16 TCP streams at 180+ Gbps). Using full-page buffers on
these platforms shows no regression and restores line-rate
performance.
This behavior is observed on a single platform; other platforms
perform better with page_pool fragments, indicating this is not a
page_pool issue but platform-specific.
This series adds an ethtool private flag "full-page-rx" to let the
user opt in to one RX buffer per page:
ethtool --set-priv-flags eth0 full-page-rx on
There is no behavioral change by default. The flag can be persisted
via udev rule for affected platforms.
====================
Link: https://patch.msgid.link/20260729063347.3388035-1-dipayanroy@linux.microsoft.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
On some ARM64 platforms with 4K PAGE_SIZE, page_pool fragment
allocation in the RX refill path can cause 15-20% throughput
regression under high connection counts (>16 TCP streams).
Add an ethtool private flag "full-page-rx" that allows the user to
force one RX buffer per page, bypassing the page_pool fragment path.
This restores line-rate (180+ Gbps) performance on affected platforms.
Usage:
ethtool --set-priv-flags eth0 full-page-rx on
There is no behavioral change by default. The flag must be explicitly
enabled by the user or udev rule.
The existing single-buffer-per-page logic for XDP and jumbo frames is
consolidated into a new helper mana_use_single_rxbuf_per_page() which
is now the single decision point for both the automatic and
user-controlled paths.
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Dipayaan Roy <dipayanroy@linux.microsoft.com>
Link: https://patch.msgid.link/20260729063347.3388035-3-dipayanroy@linux.microsoft.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Refactor mana_get_strings() and mana_get_sset_count() from if/else to
switch statements in preparation for adding ethtool private flags
support which requires handling ETH_SS_PRIV_FLAGS.
No functional change.
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Dipayaan Roy <dipayanroy@linux.microsoft.com>
Link: https://patch.msgid.link/20260729063347.3388035-2-dipayanroy@linux.microsoft.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
io_vec_fill_bvec() computes the folio size with a plain int 1:
unsigned long folio_size = 1 << imu->folio_shift;
imu->folio_shift is unsigned int and comes from folio_shift() of the
folio backing the registered buffer, so it can be 32 or more on a 64 bit
kernel. Shifting int 1 that far is undefined, and on x86 and arm64 the
count is taken modulo 32, so a shift of 34 yields 4 rather than 16G.
Every other folio_shift shift in this file already uses 1UL.
The result is that the segment estimate and the fill loop disagree.
io_estimate_bvec_size() sizes the bvec array with the real shift:
max_segs += (iov[i].iov_len >> shift) + 2;
so a 1M iovec on a 16G folio is charged 2 segments, while
io_vec_fill_bvec() then walks the same iovec in folio_size chunks of 4
bytes and writes res_bvec[bvec_idx] a quarter of a million times, past
the end of the array it was given. src_bvec is advanced once per
iteration as well, so imu->bvec is read past its end at the same time.
validate_fixed_range() only checks that the range is inside the
registered buffer and does not bound the segment count.
Reaching it needs a folio with a shift of at least 32, which means a
gigantic hugetlb page: 16G on arm64 with 64K pages, where
CONT_PMD_SHIFT is 34 and hugetlb_add_hstate(CONT_PMD_SHIFT - PAGE_SHIFT)
registers that size, and likewise on powerpc. x86_64 tops out at 1G, so
a shift of 30, which still fits in int and is unaffected.
Use 1UL, as the rest of the file does.
Fixes: 9ef4cbbcb4ac ("io_uring: add infra for importing vectored reg buffers")
Cc: stable@vger.kernel.org
Signed-off-by: Ali Ahmet Memis <ali@iusegentoo.com>
Link: https://patch.msgid.link/20260802163030.51005-1-ali@iusegentoo.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
An io_async_cmd carries an iovec array in ->vec.iovec, allocated when the
vec has to grow and kept across recycling through ctx->cmd_cache. On two
paths nothing frees it and io_clean_op()'s kfree(req->async_data) drops
the io_async_cmd without it.
io_req_uring_cleanup() clears the async data flags only when
io_alloc_cache_put() succeeds, and the cache holds IO_ALLOC_CACHE_MAX ==
128 entries, so once it is full the put fails and the vec is left behind.
An NVMe passthrough workload gets there without doing anything unusual:
nvme_uring_cmd_io() returns -EIOCBQUEUED, so the io_async_cmd stays
attached for the lifetime of the command and the live object count tracks
the queue depth. Above 128 the puts start failing.
->cleanup is the last chance to free an inherited vec, since
io_req_uring_cleanup() returns early for an io-wq issued command and is
not called at all for one completed without ever being issued. But
io_clean_op() calls ->cleanup only if REQ_F_NEED_CLEANUP is set, and for
uring_cmd that happens only where the vec has to grow, so a command
reusing a large enough cached vec never sets it. io_rw_alloc_async() and
io_msg_alloc_async() flag an inherited vec for exactly this reason;
io_uring_cmd_prep() does not.
Flag an inherited vec in io_uring_cmd_prep(), and free the vec when the
cache put fails, as io_req_rw_cleanup() does.
The leak is invisible under KASAN, where io_alloc_cache_vec_kasan() frees
the vec unconditionally.
Fixes: 3a4689ac109f ("io_uring/cmd: add iovec cache for commands")
Cc: stable@vger.kernel.org
Signed-off-by: Woraphat Khiaodaeng <worapat.kd2@gmail.com>
Link: https://patch.msgid.link/20260802073518.419-1-worapat.kd2@gmail.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
The last_check_time field in throtl_grp was used by the
CONFIG_BLK_DEV_THROTTLING_LOW mechanism (in throtl_upgrade_check() and
the downgrade logic) to timestamp the last upgrade/downgrade check.
Commit bf20ab538c81 ("blk-throttle: remove CONFIG_BLK_DEV_THROTTLING_LOW")
removed all five of its uses in blk-throttle.c and the surrounding LOW
fields (latency_target, last_finish_time, checked_last_finish_time,
avg_idletime, idletime_threshold, bio_cnt, bad_bio_cnt,
bio_cnt_reset_time), but missed the field definition itself.
It has been a dead field since then: zero references in blk-throttle.c
and the whole tree. Remove it.
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
Reviewed-by: Yu Kuai <yukuai@fygo.io>
Reviewed-by: Tang Yizhou <yizhou.tang@shopee.com>
Link: https://patch.msgid.link/20260717054855.2018578-1-cui.tao@linux.dev
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
kyber_timer_fn() sums the per-cpu latency histograms with
for_each_online_cpu(). A CPU that goes offline mid-interval leaves its
bucket un-flushed; the samples are lost from the current decision and
re-appear (stale) when the CPU is onlined again.
Fixes: 6e25cb01ea20 ("kyber: implement improved heuristics")
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
Reviewed-by: Yu Kuai <yukuai@fygo.io>
Link: https://patch.msgid.link/20260720093726.28965-5-cui.tao@linux.dev
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
ioc_lat_stat() walks ioc->pcpu_stat with for_each_online_cpu() to
compute missed-ppm and rq_wait deltas. An offlined CPU is skipped, so
its delta is dropped from the period and its last_* watermark is not
advanced; on re-online the next collection sees a delta spanning the
whole offline interval, corrupting the latency/vrate picture.
Fixes: 7caa47151ab2 ("blkcg: implement blk-iocost")
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
Reviewed-by: Yu Kuai <yukuai@fygo.io>
Link: https://patch.msgid.link/20260720093726.28965-4-cui.tao@linux.dev
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
iolatency_check_latencies() and iolatency_ssd_stat() iterate a blkg's
per-cpu latency stats with for_each_online_cpu(). When a CPU that has
accumulated io.latency samples goes offline, its bucket is skipped: the
check loop (which also resets) neither sums nor clears it, and the show
path under-reports. On re-online the stranded samples are flushed into
a later check window, which can trigger a spurious throttle/scale
adjustment.
Fixes: d70675121546 ("block: introduce blk-iolatency io controller")
Fixes: 1fa2840e56f9 ("blk-iolatency: use a percentile approache for ssd's")
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
Reviewed-by: Yu Kuai <yukuai@fygo.io>
Link: https://patch.msgid.link/20260720093726.28965-3-cui.tao@linux.dev
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
blk_stat_timer_fn() sums and resets a callback's per-cpu buckets using
for_each_online_cpu(). A CPU that goes offline with pending samples is
skipped, so its samples are neither accumulated into the window nor
cleared; they sit in the bucket until the CPU comes back online, at
which point the stale values are flushed into whatever window is then
running.
This silently corrupts the latency picture that consumers (notably
writeback throttling via wbt, and blk-mq latency tracking) base
decisions on around CPU hotplug: under-counting while the CPU is
offline, then a burst of stale data on re-online.
Fixes: 34dbad5d26e2 ("blk-stat: convert to callback-based statistics reporting")
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
Reviewed-by: Yu Kuai <yukuai@fygo.io>
Link: https://patch.msgid.link/20260720093726.28965-2-cui.tao@linux.dev
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
Move the teardown sequence which offlines and frees per-policy
blkg_policy_data (pd) into a helper for readability.
No functional change intended.
Signed-off-by: Zheng Qixing <zhengqixing@huawei.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Tang Yizhou <yizhou.tang@shopee.com>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
Link: https://patch.msgid.link/20260802112525.3933753-5-yukuai@kernel.org
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
When switching IO schedulers on a block device, blkcg_activate_policy()
can race with concurrent blkcg deletion, leading to a use-after-free in
rcu_accelerate_cbs.
T1: T2:
blkg_destroy
kill(&blkg->refcnt) // blkg->refcnt=1->0
blkg_release // call_rcu(__blkg_release)
...
blkg_free_workfn
->pd_free_fn(pd)
elv_iosched_store
elevator_switch
...
iterate blkg list
blkg_get(blkg) // blkg->refcnt=0->1
list_del_init(&blkg->q_node)
blkg_put(pinned_blkg) // blkg->refcnt=1->0
blkg_release // call_rcu again
rcu_accelerate_cbs // uaf
Fix this by checking hlist_unhashed(&blkg->blkcg_node) before getting
a reference to the blkg. This is the same check used in blkg_destroy()
to detect if a blkg has already been destroyed. If the blkg is already
unhashed, skip processing it since it's being destroyed.
Fixes: f1c006f1c685 ("blk-cgroup: synchronize pd_free_fn() from blkg_free_workfn() and blkcg_deactivate_policy()")
Signed-off-by: Zheng Qixing <zhengqixing@huawei.com>
Reviewed-by: Tang Yizhou <yizhou.tang@shopee.com>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
Reviewed-by: Tao Cui <cuitao@kylinos.cn>
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
Link: https://patch.msgid.link/20260802112525.3933753-4-yukuai@kernel.org
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
When switching an IO scheduler on a block device, blkcg_activate_policy()
allocates blkg_policy_data (pd) for all blkgs attached to the queue.
However, blkcg_activate_policy() may race with concurrent blkcg deletion,
leading to use-after-free and memory leak issues.
The use-after-free occurs in the following race:
T1 (blkcg_activate_policy):
- Successfully allocates pd for blkg1 (loop0->queue, blkcgA)
- Fails to allocate pd for blkg2 (loop0->queue, blkcgB)
- Enters the enomem rollback path to release blkg1 resources
T2 (blkcg deletion):
- blkcgA is deleted concurrently
- blkg1 is freed via blkg_free_workfn()
- blkg1->pd is freed
T1 (continued):
- Rollback path accesses blkg1->pd->online after pd is freed
- Triggers use-after-free
In addition, blkg_free_workfn() frees pd before removing the blkg from
q->blkg_list. This allows blkcg_activate_policy() to allocate a new pd
for a blkg that is being destroyed, leaving the newly allocated pd
unreachable when the blkg is finally freed.
Fix these races by extending blkcg_mutex coverage to serialize
blkcg_activate_policy() rollback and blkg destruction, ensuring pd
lifecycle is synchronized with blkg list visibility.
Fixes: f1c006f1c685 ("blk-cgroup: synchronize pd_free_fn() from blkg_free_workfn() and blkcg_deactivate_policy()")
Signed-off-by: Zheng Qixing <zhengqixing@huawei.com>
Reviewed-by: Tang Yizhou <yizhou.tang@shopee.com>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
Reviewed-by: Tao Cui <cuitao@kylinos.cn>
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
Link: https://patch.msgid.link/20260802112525.3933753-3-yukuai@kernel.org
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
blkcg_mutex
blkg_destroy_all() iterates q->blkg_list without holding blkcg_mutex,
which can race with blkg_free_workfn() that removes blkgs from the list
while holding blkcg_mutex.
Add blkcg_mutex protection around the q->blkg_list iteration to prevent
potential list corruption or use-after-free issues.
Reviewed-by: Tang Yizhou <yizhou.tang@shopee.com>
Signed-off-by: Yu Kuai <yukuai@fygo.io>
Reviewed-by: Tao Cui <cuitao@kylinos.cn>
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
Link: https://patch.msgid.link/20260802112525.3933753-2-yukuai@kernel.org
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
blkg_rwstat_recursive_sum() reads pos_blkg->online without the queue
lock that its doc comment requires, since blkcg_print_blkgs() stopped
holding it in 56cc24f59c14. Concurrent blkg_create/destroy flips
->online, tripping KCSAN. The race is harmless (RCU-protected, stale
online only causes minor stat noise). Use data_race() to annotate
the intentional lockless read.
Also update the stale doc comment that still requires the queue lock.
Reviewed-by: Yu Kuai <yukuai@fygo.io>
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
Acked-by: Tejun Heo <tj@kernel.org>
Link: https://patch.msgid.link/20260803133400.137906-1-cui.tao@linux.dev
Signed-off-by: Jens Axboe <axboe@kernel.dk>
|
|
The sch_cake ACK filter parses packets to find the TCP header and filter
duplicated ACKs if the flow is backlogged. The parsing code contains a
WARN_ON(1) which can be triggered by a malformed IP header in certain
cases. Depending on the system configuration, this leads either to
either spamming dmesg with warnings, or a panic if panic_on_warn is set.
The code already correctly skips the offending packet in the branch that
triggers the warning, so the WARN_ON itself doesn't really serve any
purpose. So just drop it altogether to avoid the inconvenient side
effects.
Fixes: 8b7138814f29 ("sch_cake: Add optional ACK filter")
Reported-by: Zhiling Zou <zhilinz@nebusec.ai>
Reported-by: Ren Wei <enjou1224z@gmail.com>
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
Link: https://patch.msgid.link/20260729191417.45665-1-toke@toke.dk
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
