Commit 242a49e5c8 ("nbd: freeze the queue for queue limits updates")
added the freeze to keep in-flight commands from seeing
torn queue_limits. But at startup the capacity is still 0
(invalidate_disk cleared it) and the write cache is off (the previous
patch cleared it on disconnect, and nbd_set_size sets it back only after
the commit), so submit_bio_noacct() rejects any bio before it reaches
the driver and no I/O is in flight. Drop the freeze by checking
capacity and write cache state in nbd_set_size.
Reviewed-by: Yu Kuai <yukuai@fygo.io>
Signed-off-by: Yang Erkun <yangerkun@huawei.com>
Link: https://patch.msgid.link/20260805122930.57647-6-yangerkun@huawei.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
nbd_add_socket() kreallocs config->socks, which a concurrent reader in
nbd_handle_cmd() could UAF; commit b98e762e3d ("nbd: freeze the queue
while we're adding connections")froze the queue to block that. But the
freeze costs an RCU grace period on every socket added, and setup adds
them one by one.
After the previous patch, nbd_add_socket() is rejected once nbd->pid is
set, so it only runs during setup. There the capacity is 0 and the
write cache is off (cleared on disconnect by the preceding patch, and
re-enabled only later in nbd_set_size), so submit_bio_noacct() rejects
every bio before it reaches the driver -- non-zero-sector ones via
bio_check_eod(), and flush-only ones via the !bdev_write_cache() branch.
No I/O is in flight, so the freeze is unnecessary.
Reviewed-by: Yu Kuai <yukuai@fygo.io>
Signed-off-by: Yang Erkun <yangerkun@huawei.com>
Link: https://patch.msgid.link/20260805122930.57647-5-yangerkun@huawei.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
An inactive nbd device may refuse any I/O operations. The nbd_config_put
function calls invalidate_disk, which sets the device capacity to zero
to reject all read and write I/O. For zero-sector flush I/O requests
from blkdev_issue_flush, if the write cache is disabled, the zero-sector
flush I/O immediately returns 0 in submit_bio_noacct. However, since
nbd_config_put does not clear the write cache state, an inactive nbd
device might still have the write cache enabled. In this situation,
zero-sector flush I/O will return -EIO because there is no active socket.
Additionally, BLK_FEAT_FUA and BLK_FEAT_ROTATIONAL flags may also remain
stale, resetting all of them ensures consistent behavior.
The limits update uses queue_limits_commit_update() (the non-freezing
variant) because config_refs == 0 here means every fd is closed and recv
threads have drained, so no in-flight I/O can read q->limits concurrently.
Reviewed-by: Yu Kuai <yukuai@fygo.io>
Signed-off-by: Yang Erkun <yangerkun@huawei.com>
Link: https://patch.msgid.link/20260805122930.57647-4-yangerkun@huawei.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
We cannot add a socket to an already running nbd device, the reconfigure
for netlink can only active an inactive socket. But for ioctl path, we can
call NBD_SET_SOCK after NBD_DO_IT, reject this using nbd->pid which has
been setted when NBD_DO_IT. Besides, it is the root cause for commit
b98e762e3d ("nbd: freeze the queue while we're adding connections").
Reviewed-by: Yu Kuai <yukuai@fygo.io>
Signed-off-by: Yang Erkun <yangerkun@huawei.com>
Link: https://patch.msgid.link/20260805122930.57647-3-yangerkun@huawei.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
The second conditional checking nsock->fallback_index validity is the
logical inverse of the first, so drop it and let execution fall through
naturally. Consolidate the two identical dev_err_ratelimited() + return
paths into a single no_fallback label to reduce duplication.
Reviewed-by: Yu Kuai <yukuai@fygo.io>
Signed-off-by: Long Li <leo.lilong@huawei.com>
Link: https://patch.msgid.link/20260805122930.57647-2-yangerkun@huawei.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Commit 05c3e88488ed ("srcu: Queue sdp->work when the delay timer is
successfully deleted") added a check in cleanup_srcu_struct() if the
call to srcu_barrier() has been made before calling it, which
revealed a missing call to srcu_barrier() before calling
cleanup_srcu_struct(set->srcu). Fix this.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Reviewed-by: Paul E. McKenney <paulmck@kernel.org>
Link: https://patch.msgid.link/20260812060510.3220294-1-m.szyprowski@samsung.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
The ioctl handlers only test REMOVE_PENDING before entering
mtip_hw_ioctl(). Removal can set that bit immediately afterwards and free
dd->port in mtip_hw_exit() while an ioctl still dereferences it. An already
open block device can reach the handlers while del_gendisk() is in
progress.
Serialize both native and compat ioctls with removal. Set REMOVE_PENDING
before taking the mutex so new callers fail after an in-flight ioctl has
drained, and hold the mutex until the port has been torn down.
Fixes: 88523a6155 ("block: Add driver for Micron RealSSD pcie flash cards")
Signed-off-by: Hongyan Xu <getshell@seu.edu.cn>
Link: https://patch.msgid.link/20260806060441.676-1-getshell@seu.edu.cn
Signed-off-by: Jens Axboe <axboe@kernel.dk>
__ublk_shmem_remove_ranges() removes matching maple tree ranges in
batches, but first stores each range into a temporary xarray so that the
pages can be unpinned after dropping the maple tree lock.
That temporary xarray is filled under the maple tree lock with
xa_store(..., GFP_ATOMIC). If the store fails before mas_erase(), the
current range is left in the tree and the helper returns false. The
outer ublk_shmem_remove_ranges() loop then immediately retries the same
range. While the atomic allocation keeps failing, the teardown path has
no forward progress.
The issue can be reproduced with radix_tree_node failslab injection after
a SHMEM_ZC buffer has already been registered:
# Kernel config:
# CONFIG_BLK_DEV_UBLK=y
# CONFIG_DEBUG_FS=y
# CONFIG_FAULT_INJECTION=y
# CONFIG_FAULT_INJECTION_DEBUG_FS=y
# CONFIG_FAILSLAB=y
echo 10 > /proc/sys/vm/nr_hugepages
mkdir -p /tmp/htlb
mount -t hugetlbfs none /tmp/htlb
fallocate -l 4M /tmp/htlb/ublk_buf
dev_id=$(kublk add -t null --shmem_zc \
--htlb /tmp/htlb/ublk_buf |
awk -F '[ :]' '/dev id/ {print $3}')
echo 1 > /sys/kernel/slab/radix_tree_node/failslab
echo Y > /sys/kernel/debug/failslab/cache-filter
echo Y > /sys/kernel/debug/failslab/ignore-gfp-wait
echo 1 > /sys/kernel/debug/failslab/interval
echo -1 > /sys/kernel/debug/failslab/times
echo 100 > /sys/kernel/debug/failslab/probability
kublk del -n "$dev_id"
On the unfixed kernel the delete command was still running after 3
seconds. Disabling failslab made it return. The fault-injection stack
showed:
should_failslab
kmem_cache_alloc_lru_noprof
__xas_nomem
__xa_store
xa_store
__ublk_shmem_remove_ranges
ublk_cdev_rel
ublk_ctrl_del_dev
Remove the allocation from the teardown loop. Keep the existing batch
limit, but collect {base_pfn, nr_pages} pairs in a fixed-size stack array.
Once a matching range is found, the range is erased from the maple tree
before dropping the lock, so each successful scan makes progress without
depending on any GFP_ATOMIC allocation.
With the same failslab settings, the fixed kernel completed
"kublk del -n $dev_id" successfully in about 45 ms.
Fixes: 309e02dccf ("ublk: avoid unpinning pages under maple tree spinlock")
Signed-off-by: Yao Sang <sangyao@kylinos.cn>
Reviewed-by: Ming Lei <tom.leiming@gmail.com>
Link: https://patch.msgid.link/20260804125736.2011774-1-sangyao@kylinos.cn
Signed-off-by: Jens Axboe <axboe@kernel.dk>
null_zone_no() does sect >> ilog2(dev->zone_size_sects). When
zone_size_sects is 0, ilog2(0) returns -1, producing shift exponent -1
which UBSAN reports as shift-out-of-bounds.
UBSAN: shift-out-of-bounds in drivers/block/null_blk/zoned.c:21:14
shift exponent -1 is negative
Call Trace:
null_zone_no drivers/block/null_blk/zoned.c:21 [inline]
null_process_zoned_cmd+0xf76/0xf80 drivers/block/null_blk/zoned.c:728
null_handle_cmd drivers/block/null_blk/main.c:1455 [inline]
null_queue_rq+0x8bc/0xe70 drivers/block/null_blk/main.c:1703
__blk_mq_issue_directly block/blk-mq.c:2694 [inline]
blk_mq_try_issue_directly+0x3f4/0x880 block/blk-mq.c:2754
blk_mq_submit_bio+0x20c0/0x2a40 block/blk-mq.c:3208
submit_bio_noacct_nocheck+0x2f4/0xa40 block/blk-core.c:790
block_read_full_folio+0x7a6/0x810 fs/buffer.c:2463
filemap_read_folio+0x12c/0x3a0 mm/filemap.c:2510
read_part_sector+0xb6/0x2b0 block/partitions/core.c:724
adfspart_check_ICS+0xb1/0x960 block/partitions/acorn.c:357
check_partition block/partitions/core.c:143 [inline]
blk_add_partitions block/partitions/core.c:591 [inline]
bdev_disk_changed+0x851/0x17a0 block/partitions/core.c:695
blkdev_get_whole+0x372/0x510 block/bdev.c:751
add_disk_final block/genhd.c:412 [inline]
add_disk_fwnode+0x24b/0x3a0 block/genhd.c:606
null_add_dev+0x130b/0x1d70 drivers/block/null_blk/main.c:2052
nullb_device_power_store+0x240/0x380 drivers/block/null_blk/main.c:501
configfs_write_iter+0x337/0x430 fs/configfs/file.c:229
Syzkaller triggers this by creating a zoned null_blk device via
configfs. The Call Trace shows configfs_write_iter in configfs/file.c
handling a write to power file, which calls nullb_device_power_store in
main.c, which calls null_add_dev in main.c, which calls add_disk in
genhd.c, which triggers partition scan via bdev_disk_changed in
partitions/core.c.
A zoned null_blk device with zone_size 0 should not be legal. Existing
code tries to reject it via is_power_of_2() check in zoned.c and
!zone_size check in main.c, but syzkaller can still reach
null_zone_no() with zone_size_sects 0 via two paths:
1. Direct 0 via configfs: zone_size attribute store in main.c has
NULLB_DEVICE_ATTR(zone_size, ulong, NULL) with no validation callback,
so echo 0 > zone_size succeeds before power store. If zoned is false
at power store time, the !zone_size check in main.c is skipped, and
later zoned set true leaves zone_size 0.
2. Large value overflow: mb_to_sects() in zoned.c does
(sector_t)mb * SZ_1M >> SECTOR_SHIFT which is mb * 2048. If mb is
1UL << 53 (9PB), mb * 2048 overflows 64-bit to 0. The value is
power-of-two so is_power_of_2() passes, but mb_to_sects() returns 0.
Check for zero zone_size explicitly in null_init_zoned_dev() in
zoned.c, returning -EINVAL with "must be non-zero power-of-two".
Check for zero zone_size_sects after mb_to_sects() conversion,
returning -EINVAL for overflow case. Keep defensive check in
null_zone_no() returning 0 for zero sectors to avoid shift out-of-bounds
even if zero slips through.
This change should be safe because zone_size is set once in
null_init_zoned_dev() under device lock and never changes after, and 0
is never valid for a zoned device. Returning -EINVAL at init time fails
device creation early with clear error, while defensive return 0 in
null_zone_no() makes zoned command fail via offline zone check.
No new locking is introduced.
Reported-by: syzbot+abd6a8dca0f2b7726060@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=abd6a8dca0f2b7726060
Link: https://lore.kernel.org/all/6a75205c.01d0871a.3a0d52.0033.GAE@google.com/
Fixes: 8a3cf049af68 ("null_blk: add zoned block device emulation")
Cc: stable@vger.kernel.org
Assisted-by: Hermes:muse-spark-1.2 syzkaller
Signed-off-by: Rik van Riel <riel@surriel.com>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Link: https://patch.msgid.link/20260808114239.69167f68@fangorn
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Not needed, probably since this file was split out.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Anuj Gupta <anuj20.g@samsung.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Link: https://patch.msgid.link/20260810144603.453283-1-hch@lst.de
Signed-off-by: Jens Axboe <axboe@kernel.dk>
All block driver code except loop_set_dio() calls
queue_limits_start_update() before it freezes the request queue. Make
loop_set_dio() follow this convention. This patch fixes the following
lockdep complaint:
======================================================
WARNING: possible circular locking dependency detected
7.2.0-rc5-dbg #11 Not tainted
------------------------------------------------------
losetup/2924 is trying to acquire lock:
ffff88816c76da68 (&q->limits_lock){+.+.}-{4:4}, at: loop_set_dio+0x318/0x720 [loop]
but task is already holding lock:
ffff88816c76d430 (&q->q_usage_counter(io)#24){++++}-{0:0}, at: blk_mq_freeze_queue_nomemsave+0x1a/0x30
which lock already depends on the new lock.
Cc: Keith Busch <kbusch@kernel.org>
Fixes: 6c8dec275c ("loop: set dma_alignment from the backing file for direct I/O")
Reported-by: syzbot+cc0de396bac84da51919@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/linux-block/6a7d5368.d5f0ebe7.22d851.0013.GAE@google.com/
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Keith Busch <kbusch@kernel.org>
Link: https://patch.msgid.link/d919f5285d16afbec6c51ecdf201692a484566e5.1786637565.git.bvanassche@acm.org
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Disks created via blk_mq_alloc_disk_for_queue() (e.g. SCSI SD disks)
do not have GD_OWNS_QUEUE set. Currently __blk_mark_disk_dead() only
sets QUEUE_FLAG_DYING when GD_OWNS_QUEUE is set, so for such disks
blk_queue_enter() and __bio_queue_enter() cannot detect the dying
state via blk_queue_dying() and remain blocked waiting for I/O that
will never complete after surprise removal.
blk_mark_disk_dead() is the explicit "surprise removal" API -- the
caller has already decided the disk is dead. Setting QUEUE_FLAG_DYING
unconditionally here is appropriate: any in-flight I/O from other
threads should get -ENODEV immediately from blk_queue_enter()
regardless of GD_OWNS_QUEUE ownership.
For disks that already have GD_OWNS_QUEUE set, __blk_mark_disk_dead()
will set the flag again which is harmless.
Fixes: 6f8191fdf4 ("block: simplify disk shutdown")
Cc: stable@vger.kernel.org
Signed-off-by: Lianqin Hu <hulianqin@vivo.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Link: https://patch.msgid.link/PUZPR06MB62247E82E66A3ED46CC3E6C7D2DC2@PUZPR06MB6224.apcprd06.prod.outlook.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Since commit 6c6c1fc09d ("modpost: require a MODULE_DESCRIPTION()"),
modpost complains that swim3.ko is missing a module description.
WARNING: modpost: drivers/block/swim3.ko: missing MODULE_DESCRIPTION()
Add one to clear up the warning.
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Link: https://patch.msgid.link/20260811-swim3-module-description-v1-1-28398c5a0e32@kernel.org
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Add test_params_01.sh for SET_PARAMS. The test checks valid basic
parameters and several invalid parameter cases.
Also cover zoned parameters, including a non-power-of-2 zone size. This
case must fail in SET_PARAMS instead of being accepted and rejected later
when the device is started.
Signed-off-by: Yao Sang <sangyao@kylinos.cn>
Link: https://patch.msgid.link/20260814023226.354288-4-sangyao@kylinos.cn
Signed-off-by: Jens Axboe <axboe@kernel.dk>
The normal kublk add command goes through device startup. It does not
tell the shell test whether a bad parameter is rejected by SET_PARAMS or
later by START_DEV.
Add a set_params command. It creates a temporary ublk device, sends
SET_PARAMS with the command line parameters, returns the ioctl result,
and deletes the device before START_DEV.
Signed-off-by: Yao Sang <sangyao@kylinos.cn>
Link: https://patch.msgid.link/20260814023226.354288-3-sangyao@kylinos.cn
Signed-off-by: Jens Axboe <axboe@kernel.dk>
UBLK_F_ZONED uses params.basic.chunk_sectors as zone size. ublk uses
ilog2(chunk_sectors) to get number of zones, so the value must be power
of 2.
If chunk_sectors is 96 and dev_sectors is 96 * 16, userspace asks for
16 zones. But the shift calculation gets 24 zones.
Block layer rejects such zone size when the disk is started. But
SET_PARAMS has already returned success, which is confusing for
userspace. Reject it in SET_PARAMS with other zoned parameter checks.
Fixes: 29802d7ca3 ("ublk: enable zoned storage support")
Signed-off-by: Yao Sang <sangyao@kylinos.cn>
Link: https://patch.msgid.link/20260814023226.354288-2-sangyao@kylinos.cn
Signed-off-by: Jens Axboe <axboe@kernel.dk>
blk_cgroup_congested() walks the current task's blkcg ancestor chain on
every readahead decision and, once swap is in use, on every anonymous and
shmem folio allocation. The answer is almost always "no", but finding that
out costs two loads per level on two cold cache lines, plus an out-of-line
kthread_blkcg() and an RCU read-side pair. On a fleet profile of hosts
running containers with 5-10 level hierarchies it costs about as much as
all of mutex_lock(), 99.4% of it under __folio_throttle_swaprate().
Gate the walk on a global count of blkcgs with a non-zero
congestion_count. The counter only moves on the 0 <-> 1 transitions of
each blkcg's congestion_count, so the extra atomic stays in the throttle
arm/disarm paths and never appears in steady state. When something is
throttled the counter is non-zero and the walk runs as before.
Signed-off-by: Usama Arif <usama.arif@linux.dev>
Acked-by: Tejun Heo <tj@kernel.org>
Link: https://patch.msgid.link/20260814165712.510132-4-usama.arif@linux.dev
Signed-off-by: Jens Axboe <axboe@kernel.dk>
iocg_kick_delay() turns sufficiently large debt into an explicit
block-cgroup delay with blkcg_set_delay(), setting blkg->use_delay to
-1 and incrementing blkcg->congestion_count. Clearing it again depends
on iocg_kick_delay() running from the period timer, the waitq timer or
the issue path.
ioc_pd_free() removes the iocg from active_iocgs and cancels its waitq
timer, and no further bios can arrive, so once it has run nothing is
left which can reduce the debt and clear the delay. The blkcg stays
marked congested for the rest of its life.
blk_cgroup_congested() then returns true for every task in that cgroup
and its descendants: page_cache_sync_ra() cuts readahead to a single
page, page_cache_async_ra() skips it altogether, and
__folio_throttle_swaprate() takes swap_avail_lock and schedules a
throttle on anonymous folio allocation.
Clear it explicitly, after the list removal and the synchronous
hrtimer_cancel() so that neither timer processing nor an I/O path can
re-arm it. The free callback can also see policy data which was never
attached to a blkg, hence the pd->blkg check.
Fixes: 7caa47151a ("blkcg: implement blk-iocost")
Signed-off-by: Usama Arif <usama.arif@linux.dev>
Acked-by: Tejun Heo <tj@kernel.org>
Link: https://patch.msgid.link/20260814165712.510132-3-usama.arif@linux.dev
Signed-off-by: Jens Axboe <axboe@kernel.dk>
io.latency can throttle a group which has no latency target of its own.
When a sibling misses its target, check_scale_change() scales down its
peers, and a peer that reaches queue depth one gets blkcg_use_delay()
called on it on every further scale-down, even with min_lat_nsec == 0.
iolatency_pd_offline() resets the target through
iolatency_set_min_lat_nsec(), which clears the delay only on a nonzero
to zero transition, so it never clears such a peer. Freeing the policy
data then leaves blkg->use_delay set and blkcg->congestion_count
elevated with nothing left that can drop it.
blk_cgroup_congested() then returns true for every task in that cgroup
and its descendants for as long as the cgroup lives: page_cache_sync_ra()
cuts readahead to a single page, page_cache_async_ra() skips it
altogether, and __folio_throttle_swaprate() takes swap_avail_lock and
schedules a throttle on anonymous folio allocation.
Clear the delay in iolatency_pd_free(). By then bio-held blkg
references have drained, or the queue is frozen for policy
deactivation, so check_scale_change() cannot re-arm it. The free
callback can also see policy data which was never attached to a blkg,
hence the pd->blkg check.
Fixes: d706751215 ("block: introduce blk-iolatency io controller")
Signed-off-by: Usama Arif <usama.arif@linux.dev>
Acked-by: Tejun Heo <tj@kernel.org>
Link: https://patch.msgid.link/20260814165712.510132-2-usama.arif@linux.dev
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Commit c807ab520f ("block/mq-deadline: Add I/O priority support")
left the dd parameter unused in deadline_move_request().
Commit fde02699c2 ("block: mq-deadline: Remove support for zone
write locking") left dd unused in deadline_fifo_request() and
deadline_next_request().
Remove these unused function parameters.
Signed-off-by: Hongfu Li <lihongfu@kylinos.cn>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Tao Cui <cuitao@kylinos.cn>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Link: https://patch.msgid.link/20260812040729.27551-1-hongfu.li@linux.dev
Signed-off-by: Jens Axboe <axboe@kernel.dk>
The attribute store methods generated with NULLB_DEVICE_ATTR() refuse to
change the configuration of a live device by testing
NULLB_DEV_FL_CONFIGURED, but that flag is only set by
nullb_device_power_store() after null_add_dev() has returned, and the
store methods take no lock at all. configfs only serializes writes to
the same open file (buffer->mutex), so a write to any attribute can run
concurrently with null_add_dev() and change the device configuration
while it is being used.
null_add_dev() reads the configuration several times, e.g. dev->zoned is
read once to set up the queue limits and once to initialize the zone
resources:
CPU0: echo 1 > nullb0/power CPU1: echo 1 > nullb0/zoned
nullb_device_power_store()
mutex_lock(&lock)
null_add_dev()
if (dev->zoned) -> false
/* no BLK_FEAT_ZONED */ nullb_device_zoned_store()
test_bit(FL_CONFIGURED) -> 0
dev->zoned = true
blk_mq_alloc_disk()
/* queue is not zoned */
if (nullb->dev->zoned) -> true
null_register_zoned_dev()
blk_revalidate_disk_zones()
blk_revalidate_disk_zones() is then called for a queue that does not
have BLK_FEAT_ZONED set, which triggers its WARN_ON_ONCE() and fails the
device setup with -EIO:
WARNING: CPU: 2 PID: 322 at block/blk-zoned.c:2357 blk_revalidate_disk_zones+0x4c/0x560
Clearing dev->zoned in the same window is worse: the queue is created
with BLK_FEAT_ZONED but the zone resources are never initialized, so
add_disk() succeeds for a zoned disk that has no zones. And a store that
lands after the last dev->zoned test leaves dev->zoned set while
dev->zones is still NULL, which null_process_zoned_cmd() dereferences on
the first write.
Fix this by taking the global lock, which nullb_device_power_store()
already holds across null_add_dev() and null_del_dev(), around both the
NULLB_DEV_FL_CONFIGURED test and the update of the device configuration.
The submit_queues and poll_queues apply callbacks are now called with
that lock held, so remove the locking they did themselves.
Since the store methods can run as soon as configfs_register_subsystem()
returns, that is, before null_init() gets to mutex_init(&lock), also
initialize the lock statically with DEFINE_MUTEX().
Fixes: 3bf2bd2073 ("nullb: add configfs interface")
Reported-by: syzbot+643a6dd130546afdf1fb@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/linux-block/6a7d0b3f.ac361c09.22ff0a.004c.GAE@google.com/
Signed-off-by: Niklas Cassel <cassel@kernel.org>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Link: https://patch.msgid.link/20260813141456.1625857-2-cassel@kernel.org
Signed-off-by: Jens Axboe <axboe@kernel.dk>
In preparation to supporting dma-buf backed iterators and bios,
introduce bio_iov_iter_set() which attempts to set up the bio directly
from the given iterator. For now, it only supports bvec and expects
users to check the result and fall back to other means if fails, but
later we'll add more types.
Suggested-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Link: https://patch.msgid.link/4686a0e47fc14f3f888967a80d45a6f66044f1e0.1785596451.git.asml.silence@gmail.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
The _show callback in the NULLB_DEVICE_ATTR macro reads dev->NAME and the
_store path writes it. configfs does not serialize accesses across separate
open file descriptions (buffer->mutex is per-fd), and _show takes no lock,
so a concurrent read and write on the same attribute is a data race. The
_show readers also race against writes to these fields that run after the
configfs item becomes visible, e.g. in nullb_update_nr_hw_queues().
All of those writers now run under the file-scope lock: _store takes it
unconditionally, and the setup-side writers run under power_store() which
holds the same lock. The only remaining unsynchronized accesses are the
plain reads in _show. Rather than annotating every field with
READ_ONCE()/WRITE_ONCE() across files, simply take the file-scope lock in
_show (and in power_show) as well. This closes the remaining _show-vs-write
data races with a single lock and keeps the writers as plain assignments.
configfs attribute access is not on the I/O hot path, so taking the mutex
in _show is acceptable from a performance standpoint. The dev fields
written in null_alloc_dev() and dev->power in nullb_group_drop_item() need
no locking: the former runs from .make_group before the item is published,
and the latter is serialized by configfs frag_sem/frag_dead against
attribute show/store.
Suggested-by: Nilay Shroff <nilay@linux.ibm.com>
Suggested-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
Link: https://patch.msgid.link/20260725022509.714271-11-wozizhi@huaweicloud.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
The NULLB_DEVICE_ATTR _store takes no lock: apply_fn attributes
(submit_queues, poll_queues) get dev->NAME written again after apply_fn
returns, outside its lock; APPLY=NULL attributes are entirely lockless.
configfs only serializes stores per-open-file, so concurrent stores on
separate fds race.
For apply_fn attributes, once one store's apply_fn has reconfigured the
hardware, a second (losing) store can still overwrite dev->NAME
afterwards. This leaves dev->submit_queues out of sync with the live
queue count, which is later caught by the WARN_ON_ONCE() in
null_map_queues().
For !apply_fn attributes, power_store()'s null_add_dev() validates and
builds the device under "lock" but only sets CONFIGURED afterwards. A store
slipping in during this window can change a field mid-setup -- for example,
zone_nr_conv can be pushed above nr_zones after it has already been
clamped, leading to an out-of-bounds dev->zones[] access.
Take "lock" in the macro around the apply_fn call, the CONFIGURED test and
the field write, and move it out of nullb_apply_submit_queues()/
nullb_apply_poll_queues() so both paths are covered once. This serializes
stores with power_store's setup and with each other.
Fixes: 45919fbfe1 ("null_blk: Enable modifying 'submit_queues' after an instance has been configured")
Suggested-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
Link: https://patch.msgid.link/20260725022509.714271-10-wozizhi@huaweicloud.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Using guard()/scoped_guard() ties lock release to scope exit, removing the
need for manual mutex_unlock() calls and preventing missed unlocks on error
paths.
The per-attribute apply wrappers are left untouched, as those are reworked
separately by the configfs show/store serialization patches.
Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
Link: https://patch.msgid.link/20260725022509.714271-9-wozizhi@huaweicloud.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
When shared_tags is enabled, null_setup_tagset() makes the device use the
global tag_set, whose driver_data stays NULL. null_map_queues() therefore
falls back to the module-wide g_submit_queues/g_poll_queues instead of any
per-device value.
Resizing submit_queues or poll_queues via configfs on such a device calls
blk_mq_update_nr_hw_queues() on the shared set, shrinking
set->nr_hw_queues. __blk_mq_realloc_hw_ctxs() only grows the
q->queue_hw_ctx[] allocation, so on shrink it merely exits and NULLs the
now-excess hctx slots. null_map_queues(), however, keeps mapping CPUs with
the unchanged g_submit_queues/g_poll_queues, so mq_map[] ends up pointing
at those NULLed hctx slots. blk_mq_map_swqueue() then dereferences the NULL
hctx (hctx->cpumask), crashing the kernel:
[ 460.218374] KASAN: null-ptr-deref in range [0x0000000000000098-0x000000000000009f]
[ 460.219003] CPU: 24 UID: 0 PID: 1492 Comm: sh Not tainted 7.2.0-rc2+ #67 PREEMPT(full)
[ 460.219792] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.17.0-4.fc41 04/01/2014
[ 460.220452] RIP: 0010:blk_mq_map_swqueue+0x4db/0x1430
......
[ 460.228977] Call Trace:
[ 460.229175] <TASK>
[ 460.229354] blk_mq_update_nr_hw_queues+0xd49/0x11c0
[ 460.229779] ? __pfx_blk_mq_update_nr_hw_queues+0x10/0x10
[ 460.230200] nullb_update_nr_hw_queues+0x1a9/0x370 [null_blk]
[ 460.230694] nullb_device_submit_queues_store+0xd9/0x170 [null_blk]
[ 460.231190] ? __pfx_nullb_device_submit_queues_store+0x10/0x10 [null_blk]
[ 460.231776] ? configfs_write_iter+0x35c/0x4e0
[ 460.232122] configfs_write_iter+0x286/0x4e0
[ 460.232460] vfs_write+0x52d/0xd00
[ 460.232779] ? __x64_sys_openat+0x108/0x1d0
[ 460.233106] ? __pfx_vfs_write+0x10/0x10
[ 460.233413] ? fdget_pos+0x1cf/0x4c0
[ 460.233745] ? fput_close+0x133/0x190
[ 460.234038] ? __pfx_expand_files+0x10/0x10
[ 460.234368] ksys_write+0xfc/0x1d0
Reproducer:
modprobe null_blk shared_tags=1 submit_queues=64 poll_queues=1
mkdir /sys/kernel/config/nullb/dev
echo 1 > /sys/kernel/config/nullb/dev/power
echo 1 > /sys/kernel/config/nullb/dev/submit_queues
A per-device resize of a shared tag set is meaningless anyway, so reject it
with -EINVAL in nullb_update_nr_hw_queues() when the device is bound to the
global tag_set.
Fixes: 45919fbfe1 ("null_blk: Enable modifying 'submit_queues' after an instance has been configured")
Suggested-by: Nilay Shroff <nilay@linux.ibm.com>
Assisted-by: Claude-Code:GLM-5.2
Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Link: https://patch.msgid.link/20260725022509.714271-8-wozizhi@huaweicloud.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Replace remaining nullb->dev dereferences with the already-cached
local dev variable. No functional change.
Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Link: https://patch.msgid.link/20260725022509.714271-7-wozizhi@huaweicloud.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
null_init_zoned_dev() allocates dev->zones when a zoned device is powered
on, but null_del_dev() never frees it on power-off; dev->zones is only
freed later in null_free_dev(), when the configfs directory is removed. If
the device is powered off and then on again, null_init_zoned_dev()
allocates a new array and overwrites the dev->zones pointer, leaking the
previous allocation each power cycle.
Free dev->zones in null_del_dev() via null_free_zoned_dev() to solve it.
And calling null_free_zoned_dev() in null_free_dev() is no longer necessary
because every caller already invokes null_del_dev() first: via
nullb_group_drop_item() before nullb_device_release(), in the
null_add_dev() error path of null_create_dev(), and in null_destroy_dev().
Remove the redundant call.
And take &lock around zone_cond_store() in the two store wrappers to
serialize dev->zones check-and-deref against its alloc/free, which already
run under &lock. The reason there was no problem before is that only
nullb_device_release() or null_exit() frees the dev->zones, which
guarantees that subsequent users won't access the configfs interface.
Fixes: ca4b2a0119 ("null_blk: add zone support")
Assisted-by: Claude-Code:GLM-5.2
Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Link: https://patch.msgid.link/20260725022509.714271-6-wozizhi@huaweicloud.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
If shared_tags is enabled, null_setup_tagset() allocates the global tag_set
via null_init_global_tag_set(). If device creation later fails, err_dev
destroys the default devices and calls unregister_blkdev(), but never frees
the global tag_set. Since module init failed, null_exit() is never invoked,
so the global tag_set's tags and maps are permanently leaked.
Free the global tag_set in err_dev, matching null_exit() which does
if (tag_set.ops) blk_mq_free_tag_set(&tag_set).
Fixes: 82f402fefa ("null_blk: add support for shared tags")
Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
Link: https://patch.msgid.link/20260725022509.714271-5-wozizhi@huaweicloud.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
In null_exit(), unregister_blkdev() is called before the null_blk instances
are destroyed, which is inconsistent with the cleanup order in null_init().
Move it after null_destroy_dev() so that teardown happens in the reverse
order of initialization.
No functional change intended.
Suggested-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
Link: https://patch.msgid.link/20260725022509.714271-4-wozizhi@huaweicloud.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
In null_init(), configfs_register_subsystem() currently runs before
register_blkdev(), so when null_blk is built as a module, a racing mkdir()
+ poweron from userspace can reach null_add_dev() while null_major is still
0. __add_disk() then hits WARN_ON(disk->minors) (major=0 with minors!=0)
and fails:
[root@fedora ~]# [ 2366.521436] WARNING: block/genhd.c:476 at __add_disk+0x8a7/0xde0,
[ 2366.523552] Modules linked in: null_blk(+) nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib
[ 2366.529081] CPU: 26 UID: 0 PID: 1600 Comm: sh Not tainted 7.2.0-rc1+ #66 PREEMPT(full)
......
[ 2366.547251] Call Trace:
[ 2366.547575] <TASK>
[ 2366.547831] ? _raw_spin_lock+0x84/0xe0
[ 2366.548260] add_disk_fwnode+0x114/0x560
[ 2366.548739] null_add_dev+0x102d/0x1b80 [null_blk]
[ 2366.549310] ? __pfx_null_add_dev+0x10/0x10 [null_blk]
[ 2366.549906] ? mutex_lock+0xde/0x1c0
[ 2366.550361] ? __pfx_mutex_lock+0x10/0x10
[ 2366.550827] nullb_device_power_store+0x1e7/0x280 [null_blk]
[ 2366.551499] ? __pfx_nullb_device_power_store+0x10/0x10 [null_blk]
[ 2366.552177] ? __kmalloc_cache_noprof+0x1f5/0x470
[ 2366.552748] ? configfs_write_iter+0x35c/0x4e0
[ 2366.553242] configfs_write_iter+0x286/0x4e0
[ 2366.553787] vfs_write+0x52d/0xd00
[ 2366.554169] ? __pfx_vfs_write+0x10/0x10
[ 2366.554679] ? __pfx___css_rstat_updated+0x10/0x10
[ 2366.555196] ? fdget_pos+0x1cf/0x4c0
[ 2366.555649] ksys_write+0xfc/0x1d0
......
Additionally, the err_dev path destroys all devices on nullb_list while
configfs is still registered. If a racing mkdir() + poweron puts a user
device on the list, null_destroy_dev()->null_free_dev() kfrees the user
device's nullb_device but /sys/kernel/config/nullb/<name> is still
reachable. Any userspace access to the item will trigger a UAF.
For simplicity, move configfs_register_subsystem() to the end to solve
the problems above.
Fixes: 3bf2bd2073 ("nullb: add configfs interface")
Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
Link: https://patch.msgid.link/20260725022509.714271-3-wozizhi@huaweicloud.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
- Enable context analysis for the nvme host driver, annotating the
subsystem's locks, along with the LIST_HEAD_GUARDED support it needs
(Nilay, Marco)
- Harden the tcp host and target against malformed PDUs and out of range
SGL lengths (Yehyeong, Ibrahim, Greg)
- Fix unserialized page_frag_cache use in nvme-tcp request setup (Dmitry)
- Bound identify, FDP and passthrough descriptor parsing to the allocated
buffers (Hari, Guixin)
- Zoned namespace fixes for host and the target (Xixin, Guixin, Yao)
- Apple controller fixes: page aligned admin queue buffers, NVMMU TCB
setup, DMA direction and admin queue teardown (Sven, Gui-Dong)
- Add a namespace level debugfs directory exposing reservation state, and
ABI documentation for the host sysfs and target configfs interfaces
(Guixin)
- Fix cdev and namespace lifetimes (John)
- Parallelize nvme-rdma I/O queue allocation and startup (Surabhi)
- Fix nvmet-rdma response resource leak on queue teardown (Shin'ichiro)
- Authentication fixes: AUTH_RECEIVE buffer and an out of bounds read in
negotiate (Xixin, Bryam, Guixin, Eric)
- Fix pci-epf use-after-free and CQ reference leak (Shin'ichiro, Yifei)
- Reject passthrough of driver managed Set Features (Chao)
- Various error path and teardown fixes across the host and target addressing
issues with use-after-free and leaking resources
(Guixin, Maurizio, Ewan, Zhengrong, Jiang HongHui, Myeonghun, Yang,
Geliang, Yehyeong)
- Various cleanups and typo fixes (Nilay, Guixin, Pan Chuang)
-----BEGIN PGP SIGNATURE-----
iQIzBAABCAAdFiEE3Fbyvv+648XNRdHTPe3zGtjzRgkFAmp+OBEACgkQPe3zGtjz
RgnuphAAx7rT3dVc46kINaqATZXkNBgQMHsDaASnZ7waSHOnuGjWnoJreaYmP+X5
FXkfnYkQ1cL823dbQNhY+O4bxUCasWm365Lbd5lrY4DZYeNuLrk0HsC9+/+AhzdT
3DeZuJ3bCpaG5AgoiWnkNUvYj5ichrp+aZgBxInF3ZrYAFDbSg70luexZvrh3CfV
YXaQh3+AoqcvXOblTDiaHpujom/+dGt6WfB67gbrhXNAzrm6jswRSgN0L21KBpwW
FhflfhxnnbJIYRkbfTw9uz6xestpQDhLYDnuvg6QPc+6ybVZsbyQdOBIRonnuKha
dXdMQ7GMVlA3DTnmOScJx4OJMezDkfSE6GGe/6t1huFQ6Gvvn87XoQjflG5/ojdQ
WvBdB/al4fewVB3DYaqUQiXa1AU4YMK3JckWcKdrPRT9RuZ/IgftGh1dEiwuTmTj
de9I/J43Mvwe9wvKnmRoqqj2tWXzq1U4G0On3UiIiCsaMHWR3BaUnb+Hq7ePszcH
m8qBNw4rKA0q36KxPP0BKeARw5+wmdH4GU3EH2btX7Zqo7tFo4dY1hHS1n1hXdX6
9bt5YbSDHkVpnLF1sOQKT/EJaiDEUQL2EiHygwM1tmpQR2rl7u8FkVjuayrQeX3r
4o9tBo78iqB4WKLTRF8h6VNv8qeHLFbPWFKtgnUrf/Y7q+qwDoQ=
=4CYx
-----END PGP SIGNATURE-----
Merge tag 'nvme-7.3-2026-08-13' of git://git.infradead.org/nvme into for-7.3/block
Pull NVMe updates from Keith:
"- Enable context analysis for the nvme host driver, annotating the
subsystem's locks, along with the LIST_HEAD_GUARDED support it needs
(Nilay, Marco)
- Harden the tcp host and target against malformed PDUs and out of
range SGL lengths (Yehyeong, Ibrahim, Greg)
- Fix unserialized page_frag_cache use in nvme-tcp request setup
(Dmitry)
- Bound identify, FDP and passthrough descriptor parsing to the
allocated buffers (Hari, Guixin)
- Zoned namespace fixes for host and the target (Xixin, Guixin, Yao)
- Apple controller fixes: page aligned admin queue buffers, NVMMU TCB
setup, DMA direction and admin queue teardown (Sven, Gui-Dong)
- Add a namespace level debugfs directory exposing reservation state,
and ABI documentation for the host sysfs and target configfs
interfaces (Guixin)
- Fix cdev and namespace lifetimes (John)
- Parallelize nvme-rdma I/O queue allocation and startup (Surabhi)
- Fix nvmet-rdma response resource leak on queue teardown (Shin'ichiro)
- Authentication fixes: AUTH_RECEIVE buffer and an out of bounds read
in negotiate (Xixin, Bryam, Guixin, Eric)
- Fix pci-epf use-after-free and CQ reference leak (Shin'ichiro, Yifei)
- Reject passthrough of driver managed Set Features (Chao)
- Various error path and teardown fixes across the host and target
addressing issues with use-after-free and leaking resources (Guixin,
Maurizio, Ewan, Zhengrong, Jiang HongHui, Myeonghun, Yang, Geliang,
Yehyeong)
- Various cleanups and typo fixes (Nilay, Guixin, Pan Chuang)"
* tag 'nvme-7.3-2026-08-13' of git://git.infradead.org/nvme: (81 commits)
nvmet: fix max_qid race between configfs and controller allocation
nvme: nvme-fc: Fix nvme_fc_create_hw_io_queues() queue deletion in error path
nvme: ratelimit the completion-path messages driven by device data
nvme-tcp: fix host memory disclosure on R2T for a read command
nvme-tcp: do not accept C2HData based on blk_rq_payload_bytes() alone
nvme-tcp: reject a read that transferred too few bytes
nvmet: zns: reject full zone report when buffer is too small
nvme-tcp: fix usage of page_frag_cache
nvme: reject passthrough of driver-managed Set Features
nvmet: fix NULL pointer dereference in nvmet_execute_identify_ns_zns()
nvmet: pci-epf: fix use-after-free in nvmet_pci_epf_exec_iod_work()
nvmet: pci-epf: put CQ ref on create_cq mapping failure
nvme-apple: Drop the PRP null check chicken bit
nvme-apple: Require page aligned buffers on the admin queue
nvme: Add a quirk for page aligned admin queue buffers
nvme-apple: Never set the opcode in the NVMMU TCB
nvme-apple: Don't set a DMA direction for commands without a data transfer
nvme-apple: Destroy the admin queue on removal
nvmet: fix heap out-of-bounds read in nvmet_auth_negotiate()
nvme: raise FDP placement handle cap to U8_MAX and warn on overflow
...
The function nvmet_subsys_attr_qid_max_store() can race against
nvmet_alloc_ctrl() when a subsystem's max_qid limit is modified.
Suppose max_qid is currently 64. If nvmet_alloc_ctrl() executes:
ctrl->sqs = kzalloc_objs(struct nvmet_sq *, subsys->max_qid + 1);
and at this exact point, a userspace process changes max_qid to 128,
nvmet_subsys_attr_qid_max_store() will set the new max_qid value. It
attempts to delete active controllers to force a reconnect, but the
new controller won't be deleted because it hasn't been added to the
subsys->ctrls list yet.
nvmet_alloc_ctrl() then proceeds and adds the new controller to the
subsys->ctrls list. Later, when nvmet_install_queue() is called, it
will see max_qid set to 128, but the memory allocated for sqs is only
sized for 64 entries. This results in a KASAN out-of-bounds warning
and potential memory corruptions.
Fix this by protecting the queue allocations and list insertion in
nvmet_alloc_ctrl() with down_read(&nvmet_config_sem). Because
nvmet_subsys_attr_qid_max_store() acquires down_write(&nvmet_config_sem)
to modify the attribute, this safely prevents the configfs writer from
modifying max_qid during controller creation.
Copy the max_qid from the subsystem to the controller's structure
during the allocation; ctrl->max_qid never changes as long as the
controller remains in LIVE state, so this will prevent similar race
conditions.
Fixes: 3e980f5995 ("nvmet: expose max queues to configfs")
Reported-by: syzbot+2626e846cd2585c9aa67@syzkaller.appspotmail.com
Signed-off-by: Maurizio Lombardi <mlombard@redhat.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
nvme_fc_create_hw_io_queues() will call __nvme_fc_delete_hw_queue() for the
last queue on which __nvme_fc_create_hw_queue() reported an error when deleting
all the io queues if they cannot all be created. This is incorrect since the
last queue did not actually get created.
The most recent change to this code was commit 17a1ec08ce ("nvme/fc: simplify
error handling of nvme_fc_create_hw_io_queues") which moved the cleanup to the
delete_queues: label and changed the loop bounds, however the code was not
correct prior to this change in a different way. The original commit
e399441de9 ("nvme-fabrics: Add host support for FC transport") had a
different error which called __nvme_fc_delete_hw_queue() on queue index 0 which
is used for the admin queue.
Fix this by correcting the initial loop index when deleting the io queues.
Fixes: 17a1ec08ce ("nvme/fc: simplify error handling of nvme_fc_create_hw_io_queues")
Fixes: e399441de9 ("nvme-fabrics: Add host support for FC transport")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-6
Reviewed-by: Maurizio Lombardi <mlombard@redhat.com>
Reviewed-by: Laurence Oberman <loberman@redhat.com>
Reviewed-by: Justin Tee <justin.tee@broadcom.com>
Signed-off-by: Ewan D. Milne <emilne@redhat.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
nvme_find_rq() and nvme_handle_cqe() print an unratelimited message for
every completion queue entry whose command id does not resolve to an
in-flight request. Both are reached from the completion interrupt path
(nvme_irq() -> nvme_poll_cq() -> nvme_handle_cqe()) and the decision to
print is made entirely from device-supplied data, so a controller that
posts a stream of bogus command ids drives unbounded printk from hard
interrupt context.
This is not hypothetical. A single boot under an emulated controller
that posts invalid completions produced 846 "could not locate request
for tag 0x0", 846 "invalid id 0 completed on queue 2" and 123 "genctr
mismatch" lines. Once the tag set has been torn down every subsequent
completion resolves to nothing, so the print rate is bounded only by how
fast the device can post entries.
Ratelimit the three messages. The information they carry is diagnostic
and repeats, so the suppression count printed by the ratelimit helpers
is enough to tell that the condition persists. This matches how the
other device-driven error prints in the driver are already handled, for
example the status messages in nvme_log_error() and
nvme_log_err_passthru().
nvme_find_rq() lives in nvme.h and is shared by pci, tcp, rdma, apple and
target-loop, so all transports are covered.
Found by FuzzNvme.
Signed-off-by: Chao Shi <coshi036@gmail.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
nvme_tcp_handle_r2t() does not check the direction of the request the
R2T refers to. A malicious controller can send an R2T for a READ and
the host will answer it: nvme_tcp_setup_h2c_data_pdu() builds the
H2CData header and nvme_tcp_try_send_data() sends the request's data
buffer. That buffer is the READ destination, so its contents go to the
controller.
The command then completes normally and nothing is logged.
Against a test controller that answers every READ with an R2T, a 4096
byte buffered read returned all 4096 bytes, split over two R2Ts. The
pages contained stale kernel data, including an array of struct page
pointers.
Reject an R2T for a request that is not a write.
Fixes: 3f2304f8c6 ("nvme-tcp: add NVMe over TCP host driver")
Cc: stable@vger.kernel.org
Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
Signed-off-by: Keith Busch <kbusch@kernel.org>
Commit 25e5cb780e ("nvme-tcp: fix possible crash in write_zeroes
processing") established that blk_rq_payload_bytes() must not be read
without first checking blk_rq_nr_phys_segments(), and recorded the
result in nvme_tcp_setup_cmd_pdu() as req->data_len. The receive side
was left as it was.
The two differ for REQ_OP_WRITE_ZEROES, which has no physical segments
but a non-zero blk_rq_bytes(), so setup leaves req->iter untouched
while the receive gate lets a C2HData through and nvme_tcp_recv_data()
copies into whatever the previous command on that tag left there. The
driver-private area is zeroed only when the tag set is allocated.
Reproduced with a test target that leaves a residual iterator on a tag
and then sends a C2HData for a WRITE_ZEROES command on the same tag:
BUG: KASAN: wild-memory-access in _copy_to_iter+0x642/0x1330
Write of size 512 at addr ffe728c2175dfa81 by task kworker/0:1H/103
CPU: 0 UID: 0 PID: 103 Comm: kworker/0:1H Not tainted 7.2.0-rc5-NVMETCP-gf5098b6bae76 #1 PREEMPT(lazy)
Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
Workqueue: nvme_tcp_wq nvme_tcp_io_work
Call Trace:
<TASK>
dump_stack_lvl+0x53/0x70
kasan_report+0xce/0x100
? _copy_to_iter+0x642/0x1330
kasan_check_range+0x105/0x1b0
__asan_memcpy+0x3c/0x60
_copy_to_iter+0x642/0x1330
? __pfx_sock_has_perm+0x10/0x10
? worker_thread+0x45b/0xd10
? __pfx__copy_to_iter+0x10/0x10
? _raw_spin_lock_bh+0x83/0xe0
? __pfx__raw_spin_lock_bh+0x10/0x10
__skb_datagram_iter+0xf3/0x820
? __pfx_simple_copy_to_iter+0x10/0x10
? __asan_memcpy+0x3c/0x60
? skb_copy_bits+0x58d/0x830
skb_copy_datagram_iter+0x37/0x120
nvme_tcp_recv_skb+0xa07/0x4320
? __pfx_nvme_tcp_recv_skb+0x10/0x10
__tcp_read_sock+0x1ab/0x810
? __pfx_nvme_tcp_recv_skb+0x10/0x10
? __pfx_lock_sock_nested+0x10/0x10
? __pfx___tcp_read_sock+0x10/0x10
nvme_tcp_try_recv+0x152/0x1e0
? __pfx_nvme_tcp_try_recv+0x10/0x10
? __pfx_mutex_unlock+0x10/0x10
nvme_tcp_io_work+0x1e4/0x6c0
? __schedule+0x181a/0x49f0
? __pfx_nvme_tcp_io_work+0x10/0x10
process_one_work+0x633/0x1030
Keep the blk_rq_payload_bytes() test and add req->data_len to it. The
old test is what rejects a C2HData naming a tag that is no longer in
flight, because blk_update_request() zeroes rq->__data_len on
completion; req->data_len and req->curr_bio are driver-private and
survive completion, so they cannot stand in for it. Setup initialises
the iterator only when both req->curr_bio and req->data_len are set, so
the gate now tests the same two.
Fixes: 25e5cb780e ("nvme-tcp: fix possible crash in write_zeroes processing")
Cc: stable@vger.kernel.org
Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
Signed-off-by: Keith Busch <kbusch@kernel.org>
nvme_tcp_recv_data() completes a request once the current C2HData PDU
has been consumed. Nothing compares the total bytes received against
the length the command asked for: struct nvme_tcp_request has no
receive-side counter, queue->data_remaining is per queue, and
blk_mq_end_request() completes for blk_rq_bytes(rq) unconditionally
with no residual concept anywhere above.
A controller can therefore answer a 4096-byte read with 512 bytes and
have it reported as a complete read; user space then gets 4096 bytes of
which 3584 are whatever was already in the page. I reproduced that with
a test target.
Count the bytes received and refuse to complete a successful read whose
count does not match, at the two NVME_TCP_F_DATA_SUCCESS paths and in
nvme_tcp_process_nvme_cqe(). The success test shifts req->status right
by one, because the driver keeps the wire value there and shifts it on
completion, so the check must see what the completion path will see.
Only REQ_OP_READ is checked, because there the length comes from the
sectors the request covers; a passthrough command is built by its
submitter, which picks both command and buffer, so the kernel has
nothing to compare against.
Fixes: 3f2304f8c6 ("nvme-tcp: add NVMe over TCP host driver")
Cc: stable@vger.kernel.org
Signed-off-by: Yehyeong Lee <yhlee@isslab.korea.ac.kr>
Signed-off-by: Keith Busch <kbusch@kernel.org>
Zone Management Receive uses the Partial Report (PR) bit in dword 13. On a
partial report (PR bit set), the host accepts an incomplete listing and
Number of Zones must not exceed the zone descriptors copied to the host
buffer. On a full report (PR bit clear), Number of Zones is the total
number of matching zones and every descriptor must fit in the buffer (ZNS
Command Set Specification Rev 1.2, section 3.4.2).
nvmet_bdev_zone_zmgmt_recv_work() already caps Number of Zones for partial
reports, but on a full report it may still succeed when the buffer only
holds part of the matching descriptors. Reject the command in that case.
Signed-off-by: Xixin Liu <liuxixin@kylinos.cn>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Keith Busch <kbusch@kernel.org>
nvme uses page_frag_cache to preallocate PDU for each preallocated request
of block device. Block devices are created in parallel threads,
consequently page_frag_cache is used in not thread-safe manner.
That leads to incorrect refcounting of backstore pages and premature free.
That can be catched by !sendpage_ok inside network stack:
WARNING: CPU: 7 PID: 467 at ../net/core/skbuff.c:6931 skb_splice_from_iter+0xfa/0x310.
tcp_sendmsg_locked+0x782/0xce0
tcp_sendmsg+0x27/0x40
sock_sendmsg+0x8b/0xa0
nvme_tcp_try_send_cmd_pdu+0x149/0x2a0
Then random panic may occur.
Fix that by serializing the usage of page_frag_cache.
Fixes: 4e893ca811 ("nvme_core: scan namespaces asynchronously")
Signed-off-by: Dmitry Bogdanov <d.bogdanov@yadro.com>
Signed-off-by: Daniel Wagner <wagi@kernel.org>
Signed-off-by: Keith Busch <kbusch@kernel.org>
Since commit b58da2d270 ("nvme: update keep alive interval when kato
is modified"), a Set Features (KATO) passthrough command lets userspace
start keep-alive on any transport. nvme_keep_alive_work() allocates with
BLK_MQ_REQ_RESERVED, but nvme_alloc_admin_tag_set() reserves admin tags
only for fabrics, so on other transports the allocation trips
WARN_ON_ONCE() in blk_mq_get_tag() and fails:
nvme nvme0: keep-alive failed: -11
Several Set Features change controller state the driver manages itself
and cannot react to when set behind its back. Reject these in
nvme_admin_cmd_allowed():
- KATO on non-fabrics (keep-alive is only armed for fabrics; on PCIe
it has no reserved tag and harms idle power states)
- Host Behavior Support, Host Memory Buffer, Number of Queues, and
Autonomous Power State Transition (all driver-managed)
Keep Alive on fabrics is unchanged; I/O commands are unaffected as the
check is confined to the admin path (ns == NULL).
Link: https://lore.kernel.org/linux-nvme/20260523225629.3964037-1-coshi036@gmail.com/
Fixes: b58da2d270 ("nvme: update keep alive interval when kato is modified")
Found by FuzzNvme.
Acked-by: Sungwoo Kim <iam@sung-woo.kim>
Acked-by: Dave Tian <daveti@purdue.edu>
Acked-by: Weidong Zhu <weizhu@fiu.edu>
Signed-off-by: Chao Shi <coshi036@gmail.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
When a host issues an Identify command with CNS 05h (I/O Command Set
specific Identify Namespace) and CSI 02h (ZNS) targeting a file-backed
namespace, nvmet_execute_identify_ns_zns() calls bdev_is_zoned() on
req->ns->bdev. A file-backed namespace has no block device, so
req->ns->bdev is NULL and bdev_is_zoned() dereferences it, oopsing.
The I/O command set is selected by the host-supplied CSI field and the
command is routed here whenever CONFIG_BLK_DEV_ZONED is enabled,
independent of the namespace backing type, so any file-backed namespace
is exposed.
Reject the command with Invalid Field when the namespace is not backed
by a block device.
Fixes: aaf2e048af ("nvmet: add ZBD over ZNS backend support")
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
nvmet_pci_epf_exec_iod_work() submits an I/O command with req->execute()
and then waits for the command to complete and transfers the data back
to the host. This wait is not needed for commands that do not transfer
data from the device to the host. To decide whether that wait is needed,
it reads iod->data_len and iod->dma_dir after calling req->execute().
However, once req->execute() is called, the command may complete
asynchronously on another CPU. For commands that do not require a
device-to-host data transfer, nvmet_pci_epf_queue_response() calls
nvmet_pci_epf_complete_iod() directly, which can free the iod before it
reads iod->data_len and iod->dma_dir, resulting in the KFENCE use-after-
free:
BUG: KFENCE: use-after-free read in nvmet_pci_epf_exec_iod_work+0x288/0x798 [nvmet_pci_epf]
Use-after-free read at 0x00000000fdfa6d03 (in kfence-#63):
nvmet_pci_epf_exec_iod_work+0x288/0x798 [nvmet_pci_epf]
process_one_work+0x15c/0x4f0
worker_thread+0x18c/0x30c
kthread+0x130/0x140
ret_from_fork+0x10/0x20
kfence-#63: 0x00000000e3de0e71-0x00000000c938ad62, size=712, cache=kmalloc-1k
allocated by task 10 on cpu 0 at 73.995480s (0.005122s ago):
mempool_kmalloc+0x1c/0x28
mempool_alloc_noprof+0x40/0x9c
nvmet_pci_epf_poll_sqs_work+0xd4/0x344 [nvmet_pci_epf]
process_one_work+0x15c/0x4f0
worker_thread+0x18c/0x30c
kthread+0x130/0x140
ret_from_fork+0x10/0x20
freed by task 131 on cpu 3 at 73.995521s (0.008385s ago):
mempool_kfree+0x10/0x20
mempool_free+0x44/0x64
nvmet_pci_epf_free_iod+0x88/0x98 [nvmet_pci_epf]
nvmet_pci_epf_cq_work+0xfc/0x280 [nvmet_pci_epf]
process_one_work+0x15c/0x4f0
worker_thread+0x18c/0x30c
kthread+0x130/0x140
ret_from_fork+0x10/0x20
Fix this by referring to iod->data_len and iod->dma_dir before calling
req->execute(). The remaining iod accesses such as iod->status are only
reached on the device-to-host read path. In this case,
nvmet_pci_epf_queue_response() signals iod->done instead of freeing the
iod, so the iod stays valid.
Fixes: 0faa0fe6f9 ("nvmet: New NVMe PCI endpoint function target driver")
Cc: stable@vger.kernel.org
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
nvmet_pci_epf_create_cq() calls nvmet_cq_create(), which takes a
reference on the controller and installs the completion queue. If the
subsequent PCI address-space mapping fails or returns a too-small partial
mapping, the function jumps to err_internal / err_unmap_queue without
calling nvmet_cq_put(). The matching put in nvmet_pci_epf_delete_cq() is
gated on NVMET_PCI_EPF_Q_LIVE, which is only set after the mapping
succeeds, so teardown never releases these references. A remote PCI host
that drives Create IO CQ commands with a failing PRP1/pci_addr therefore
leaks the CQ and a controller reference on each attempt.
Drop the CQ reference on the mapping-failure paths. The err_internal and
err_unmap_queue labels are only reachable after nvmet_cq_create() has
succeeded, so this pairs the create/put correctly.
Fixes: 0faa0fe6f9 ("nvmet: New NVMe PCI endpoint function target driver")
Cc: stable@vger.kernel.org
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Yifei Gao <gyf161023@gmail.com>
Signed-off-by: Keith Busch <kbusch@kernel.org>
Now that we program the DMA direction correctly the NULL check that used
to make commands fail passes. Another side effect of this bit was that
non-align buffers on the admin queue were silently allowed and that's
been fixed now as well and we this don't need this chicken bit anymore.
More importantly, starting with the firmware installed with macOS 15,
which is required for M4 but can also be installed on the previous SoCs,
the controller no longer exposes this control register and any access
SErrors instead. Just drop the write entirely.
Fixes: 5bd2927ace ("nvme-apple: Add initial Apple SoC NVMe driver")
Tested-by: Joshua Peisach <jpeisach@ubuntu.com>
Tested-by: Janne Grunau <j@jannau.net>
Tested-by: Nick Chan <towinchenmi@gmail.com>
Signed-off-by: Sven Peter <sven@kernel.org>
Now that we have a quick to align buffers on the admin queue to the NVMe
controller page size use it for Apple controllers. This fixes pre-M1
controllers, which always rejected unaligned requests, and also makes
this driver work for M4 SoCs and for M1/M2/M3 SoCs that have been
updated to the firmware shipped with macOS 15.
Fixes: 5bd2927ace ("nvme-apple: Add initial Apple SoC NVMe driver")
Tested-by: Joshua Peisach <jpeisach@ubuntu.com>
Tested-by: Janne Grunau <j@jannau.net>
Tested-by: Nick Chan <towinchenmi@gmail.com>
Signed-off-by: Sven Peter <sven@kernel.org>
