* Re: [PATCH v19 31/40] dept: assign unique dept_key to each distinct wait_for_completion() caller
From: Byungchul Park @ 2026-07-10 5:53 UTC (permalink / raw)
To: Gary Guo
Cc: linux-kernel, max.byungchul.park, kernel_team, torvalds,
damien.lemoal, linux-ide, adilger.kernel, linux-ext4, mingo,
peterz, will, tglx, rostedt, joel, sashal, daniel.vetter,
duyuyang, johannes.berg, tj, tytso, willy, david, amir73il,
gregkh, kernel-team, linux-mm, akpm, mhocko, minchan, hannes,
vdavydov.dev, sj, jglisse, dennis, cl, penberg, rientjes, vbabka,
ngupta, linux-block, josef, linux-fsdevel, jack, jlayton,
dan.j.williams, hch, djwong, dri-devel, rodrigosiqueiramelo,
melissa.srw, hamohammed.sa, harry.yoo, chris.p.wilson,
gwan-gyeong.mun, boqun.feng, longman, yunseong.kim, ysk,
yeoreum.yun, netdev, matthew.brost, her0gyugyu, corbet,
catalin.marinas, bp, x86, hpa, luto, sumit.semwal, gustavo,
christian.koenig, andi.shyti, arnd, lorenzo.stoakes, Liam.Howlett,
rppt, surenb, mcgrof, petr.pavlu, da.gomez, samitolvanen, paulmck,
frederic, neeraj.upadhyay, joelagnelf, josh, urezki,
mathieu.desnoyers, jiangshanlai, qiang.zhang, juri.lelli,
vincent.guittot, dietmar.eggemann, bsegall, mgorman, vschneid,
chuck.lever, neil, okorniev, Dai.Ngo, tom, trondmy, anna, kees,
bigeasy, clrkwllms, mark.rutland, ada.coupriediaz,
kristina.martsenko, wangkefeng.wang, broonie, kevin.brodsky, dwmw,
shakeel.butt, ast, ziy, yuzhao, baolin.wang, usamaarif642,
joel.granados, richard.weiyang, geert+renesas, tim.c.chen, linux,
alexander.shishkin, lillian, chenhuacai, francesco,
guoweikang.kernel, link, jpoimboe, masahiroy, brauner,
thomas.weissschuh, oleg, mjguzik, andrii, wangfushuai, linux-doc,
linux-arm-kernel, linux-media, linaro-mm-sig, linux-i2c,
linux-arch, linux-modules, rcu, linux-nfs, linux-rt-devel,
2407018371, dakr, miguel.ojeda.sandonis, neilb, bagasdotme,
wsa+renesas, dave.hansen, geert, ojeda, alex.gaynor, bjorn3_gh,
lossin, a.hindborg, aliceryhl, tmgross, rust-for-linux
In-Reply-To: <DJSEK3KA2ECQ.1512A4KGOBSCV@garyguo.net>
On Tue, Jul 07, 2026 at 03:18:29PM +0100, Gary Guo wrote:
> On Mon Jul 6, 2026 at 7:19 AM BST, Byungchul Park wrote:
> > wait_for_completion() can be used at various points in the code and it's
> > very hard to distinguish wait_for_completion()s between different usages.
> > Using a single dept_key for all the wait_for_completion()s could trigger
> > false positive reports.
> >
> > Assign unique dept_key to each distinct wait_for_completion() caller to
> > avoid false positive reports.
> >
> > While at it, add a rust helper for wait_for_completion() to avoid build
> > errors.
>
> This will cause Rust code to share the same dept_key, so it will have all the
> false positives that the change is trying to avoid.
Thank you for the input.
> In general it is easy to create Rust bindings for static inline C functions
> because it'll be just some computation, while creating bindings for C
> function-like macros that define additional statics can be challenging.
>
> Is dept_key similar to lock_class_key, where only the address matters? If so,
> the approach that I use in
> https://lore.kernel.org/rust-for-linux/DJP0CDOR98N5.29BK8PUFRWRUK@garyguo.net
Yes, dept_key is similar to lock_class_key. IIUC, the way you tried for
lock_class_key can be applied to DEPT too. I will do:
1) add 'key' parameter to sdt_might_sleep_start_timeout()
2) introduce init_completion_dkey() to allow custom keys
3) remove init_completion_dmap() and adjust the existing users
Just in case, it's worth noting that the custom keys must be
well-managed using e.g. dept_key_destroy() when the key gets freed.
Byungchul
> could be used for dept_key as well, then we can keep Rust `wait_for_completion`
> still a function; otherwise we have to turn it into a macro too on the Rust side
> to create such statics, which isn't ideal.
>
> Best,
> Gary
>
> >
> > Signed-off-by: Byungchul Park <byungchul@sk.com>
> > ---
> > include/linux/completion.h | 100 +++++++++++++++++++++++++++++++------
> > kernel/sched/completion.c | 60 +++++++++++-----------
> > rust/helpers/completion.c | 5 ++
> > 3 files changed, 120 insertions(+), 45 deletions(-)
^ permalink raw reply
* Re: [PATCH] drbd: reject data replies with an out-of-range payload size
From: Christoph Böhmwalder @ 2026-07-10 10:27 UTC (permalink / raw)
To: Michael Bommarito
Cc: Philipp Reisner, Lars Ellenberg, Jens Axboe, drbd-dev,
linux-block, linux-kernel
In-Reply-To: <20260710022837.3738461-1-michael.bommarito@gmail.com>
On Thu, Jul 09, 2026 at 10:28:37PM -0400, Michael Bommarito wrote:
>recv_dless_read() receives a P_DATA_REPLY from a peer into the bio of an
>outstanding read request. The peer-supplied payload length reaches it as
>the signed int data_size, and two peer-controlled inputs can make it
>negative. With a negotiated data-integrity-alg the digest length is
>subtracted first, so a reply whose payload is smaller than the digest
>underflows data_size. With no integrity algorithm (the default) data_size
>is assigned from the unsigned h95/h100 wire length and drbdd() never
>bounds it for a payload-carrying command, so a length above INT_MAX casts
>it negative; this path needs no non-default feature. The bio receive loop
>then computes expect = min_t(int, data_size, bv_len), which is negative,
>and drbd_recv_all_warn(mapped, expect) receives with a size_t of SIZE_MAX
>into the first mapped page.
>
>The sibling receive path read_in_block() is not affected: it uses an
>unsigned size and rejects it against DRBD_MAX_BIO_SIZE before receiving.
>Reject a data reply whose size is negative after the optional digest
>subtraction, covering both triggers.
>
>Impact: a malicious or man-in-the-middle DRBD peer copies attacker-chosen
>bytes past a bio page in the receiver, corrupting kernel memory. A node
>that reads from its peer (a diskless node, or read-balancing to the peer)
>is exposed in the default configuration; data-integrity-alg is not
>required.
>
>Fixes: b411b3637fa7 ("The DRBD driver")
>Cc: stable@vger.kernel.org
>Assisted-by: Codex:gpt-5-5-xhigh
>Assisted-by: Claude:claude-opus-4-8
>Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
Looks correct, thanks.
Note that DRBD usually chooses to trusts its peers by design, but I
agree that memory corruption is a bad enough consequence that we
should put the guard there regardless.
Reviewed-by: Christoph Böhmwalder <christoph.boehmwalder@linbit.com>
^ permalink raw reply
* Re: [PATCH] drbd: reject oversized DataReply before signed conversion
From: Christoph Böhmwalder @ 2026-07-10 10:34 UTC (permalink / raw)
To: Tianchu Chen
Cc: philipp.reisner, lars.ellenberg, drbd-dev, linux-block,
linux-kernel, axboe
In-Reply-To: <d4924afb559896a4a22258f14e4a97c8d12d8527@linux.dev>
On Tue, Jun 30, 2026 at 10:59:31AM +0000, Tianchu Chen wrote:
>From: Tianchu Chen <flynnnchen@tencent.com>
>
>Discovered by Atuin - Automated Vulnerability Discovery Engine.
>
>Reject DataReply payload lengths that cannot fit in recv_dless_read()'s
>signed size argument so a bogus remote peer cannot wrap the length negative
>and turn it into a huge heap OOB-write.
>
>Fixes: b411b3637fa7 ("The DRBD driver")
>Cc: stable@vger.kernel.org
>Signed-off-by: Tianchu Chen <flynnnchen@tencent.com>
This is similar to [0], but the mentioned patch is a more complete fix.
There is also a case in recv_dless_read where data_size can underflow
when a data-integrity-alg is configured.
The other patch catches this case as well by placing the guard later, so
I would prefer to apply that one instead of this.
Thanks,
Christoph
[0] https://lore.kernel.org/all/20260710022837.3738461-1-michael.bommarito@gmail.com/
^ permalink raw reply
* Re: [PATCH] drbd: Fix potential NULL pointer dereference in _drbd_set_state()
From: Christoph Böhmwalder @ 2026-07-10 11:13 UTC (permalink / raw)
To: Ваторопин Андрей
Cc: Philipp Reisner, Lars Ellenberg, Jens Axboe, Andreas Gruenbacher,
drbd-dev@lists.linbit.com, linux-block@vger.kernel.org,
linux-kernel@vger.kernel.org, lvc-project@linuxtesting.org,
stable@vger.kernel.org
In-Reply-To: <20260625050016.12004-1-a.vatoropin@crpt.ru>
Thanks for your patch.
On Thu, Jun 25, 2026 at 05:03:06AM +0000, Ваторопин Андрей wrote:
>From: Andrey Vatoropin <a.vatoropin@crpt.ru>
>
>The connection pointer receives a value in the _drbd_set_state()
>function, including through a call to the first_peer_device() function.
>This function returns a pointer to a list element. If the list is empty, it
>returns a NULL pointer, which is later assigned to the connection
>pointer. Subsequently, this pointer will be dereferenced.
Can the list actually be empty at this point?
The peer_device is linked into the list in drbd_create_device(), before
add_disk() and before the device is inserted into connection->peer_devices,
so no state change can reach the device earlier.
It is only unlinked again in drbd_destroy_device(), after the last kref
to the device is gone.
The connection itself is created together with the resource in
drbd_adm_new_resource() and lives until the resource is destroyed.
So for any device this function can be called on, first_peer_device()
returns a valid peer_device.
>
>Add a NULL check for the connection pointer to avoid dereferencing an
>invalid pointer.
>
>Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
>Fixes: a6b32bc3cebd ("drbd: Introduce "peer_device" object between "device" and "connection"")
>Cc: stable@vger.kernel.org
>Signed-off-by: Andrey Vatoropin <a.vatoropin@crpt.ru>
>---
> drivers/block/drbd/drbd_state.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
>diff --git a/drivers/block/drbd/drbd_state.c b/drivers/block/drbd/drbd_state.c
>index adcba7f1d8ea..ea982d48017e 100644
>--- a/drivers/block/drbd/drbd_state.c
>+++ b/drivers/block/drbd/drbd_state.c
>@@ -1281,6 +1281,11 @@ _drbd_set_state(struct drbd_device *device, union drbd_state ns,
> if (rv < SS_SUCCESS)
> return rv;
>
>+ if (!connection) {
>+ drbd_err(device, "No connection to peer, aborting!\n");
>+ return SS_ALREADY_STANDALONE;
>+ }
>+
Also, even if the condition could happen, its handling here would be
wrong. Since this check happens before handling hard state changes,
those could potentially be skipped, which is not allowed.
For example, after a local I/O error (drbd_chk_io_error), if this
condition would trigger, the detach state change would be silently
skipped. So in that circumstance, this patch would be actively harmful.
Also, SS_ALREADY_STANDALONE would map to the error message "Can not
disconnect a StandAlone device", which does not make any sense in this
context.
> if (!(flags & CS_HARD)) {
> /* pre-state-change checks ; only look at ns */
> /* See drbd_state_sw_errors in drbd_strings.c */
>--
>2.43.0
In summary, unless I missed something major: NAK.
Thanks,
Christoph
^ permalink raw reply
* [RESEND][SECURITY] block: double unpin in bounced direct reads
From: Neptune @ 2026-07-10 12:26 UTC (permalink / raw)
To: Jens Axboe, Christoph Hellwig; +Cc: linux-block, security
[-- Attachment #1: Type: text/plain, Size: 5247 bytes --]
Resending in plain text because my first message was rejected by the
mailing list due to its MIME format. The technical content is
unchanged.
Hi Jens, Christoph,
I found a double-unpin in the block-layer read-bounce path that can be
reached by an unprivileged process through XFS direct I/O. I
reproduced it as a uid-1000 to uid-0 privilege escalation on mainline
commit 0e35b9b6ec0f. The bug was introduced by e7b8b3c5b2a6 ("block:
align down bounces bios"), which first appears in v7.1-rc4.
AI-assisted tooling was used during the source review and validation.
I independently verified the bug against the source, reproduced the
complete privilege-escalation chain, and tested the proposed fix. I am
treating the finding as public for coordination purposes, but I have
not included the reproducer or exploit in this message.
bio_iov_iter_bounce_read() stores the bounce folio in bi_io_vec[0],
while the extracted user bvecs start at bi_io_vec[1]. bi_vcnt counts
only those user bvecs. bio_iov_iter_align_down(), however, treats
bi_vcnt as the length of an array starting at slot zero and selects
the last vector with bio->bi_io_vec[bio->bi_vcnt - 1].
I create the partial extraction with three iovecs: a one-byte mapped
segment, a 512-byte mapped segment, and a 511-byte PROT_NONE segment.
The complete 1024-byte read is aligned, but pinning the third segment
fails after the first two segments have already produced 513 bytes.
This leaves the following state immediately before trimming:
bi_size = 513
bi_vcnt = 2
len_align_mask = 511
vec[0] = bounce folio, len 513
vec[1] = first user page, len 1
vec[2] = second user page, len 512
The one-byte remainder should be removed from vec[2], but the helper
selects vec[1]. Because that bvec is exactly one byte long, it is
removed and its page is unpinned. bi_vcnt becomes one, and read-bounce
completion subsequently walks the user array from vec[1] and unpins
the same page again. Once the partial-extraction state has been
created, the two unpins are deterministic and do not depend on a race.
The in-tree path I confirmed is xfs_file_dio_read() with
IOMAP_DIO_BOUNCE set when mapping_stable_writes() is true. The trigger
needs a non-DAX XFS file backed by an allocated extent on a device
that either provides an integrity checksum or advertises
BLK_FEAT_STABLE_WRITES. It only requires read access to that file and
an aligned O_DIRECT vectored read; raw block-device access and
capabilities are not required. In my test setup, queue/stable_writes
was zero, but the NVMe namespace exposed T10-DIF-TYPE1-CRC, which also
makes bdev_stable_writes() true.
For the privilege-escalation test, I registered the page backing the
one-byte first iovec as an io_uring fixed buffer before triggering the
bug. The DIO pin and the fixed-buffer pin account for two
GUP_PIN_COUNTING_BIAS references. GDB showed a refcount of 2050 at the
first erroneous unpin, followed by 2050 -> 1026 in the trim path and
1026 -> 2 in read-bounce completion. After munmap() and memfd
teardown, the page is freed while io_uring still retains its bvec and
old PFN.
I then reclaimed that PFN as the page-cache page containing the entry
point of a root-owned setuid ELF. IORING_OP_WRITE_FIXED was used as a
read oracle to confirm the reuse, and IORING_OP_READ_FIXED replaced
the entry point with a position-independent
setuid(0)/execve("/bin/sh") payload. This does not require a kernel
address leak or kernel control-flow hijacking.
The vulnerable kernel produced:
$ id
uid=1000(ctf) gid=1000(ctf) groups=1000(ctf)
$ /bio_bounce_lpe
[*] uid=1000
[+] stale page hit
[+] exec
# id
uid=0(root) gid=0(root) groups=1000(ctf)
With the proposed fix, using the same config, initramfs, NVMe PI
namespace and XFS setup, the exploit no longer obtains a stale page:
$ /bio_bounce_lpe
[*] uid=1000
[-] stale page miss
$ id
uid=1000(ctf) gid=1000(ctf) groups=1000(ctf)
I repeated the fixed-kernel test on fresh images and obtained the same
result. The setuid target in the disposable VM was an unmodified
static BusyBox binary copied onto XFS and installed as root-owned mode
4755. It was only a convenient page-cache target: the exploit parses
the ELF entry point at runtime, does not depend on BusyBox behavior or
a hard-coded offset, and I also validated the same ending with a
dynamically linked PIE executable.
The attached patch changes bio_iov_iter_align_down() to accept the
index of the first vector it is allowed to trim. Existing callers pass
zero and read-bounce passes one. It also updates the bounce bvec
length after alignment and drops the bounce folio if alignment removes
all user data. I tested the resulting kernel in the same VM described
above.
I can provide the minimal trigger, full exploit, disposable initramfs,
exact QEMU command line, and the GDB transcript showing both unpins if
they would be useful for review.
Could you take a look at the analysis and patch? In particular, I
would appreciate your view on whether passing the first vector index
into the alignment helper is the right fix, or whether you would
prefer the read-bounce layout itself to be changed.
Regards,
0wnerD1ed
l7z@0b1t.tech
[-- Attachment #2: 0001-block-fix-bvec-offset-when-aligning-bounced-reads.patch --]
[-- Type: application/octet-stream, Size: 3538 bytes --]
From dae9025b3403bcecb2f1224306cb4b8b22a6c27c Mon Sep 17 00:00:00 2001
From: 0wnerD1ed <l7z@0b1t.tech>
Date: Fri, 10 Jul 2026 19:05:58 +0800
Subject: [PATCH] block: fix bvec offset when aligning bounced reads
bio_iov_iter_bounce_read() stores the bounce folio in bi_io_vec[0] and
extracts user bvecs starting at bi_io_vec[1]. bi_vcnt, however, counts
only the extracted user bvecs.
bio_iov_iter_align_down() assumes that the vectors it trims start at
index zero. Thus, with two extracted user bvecs it selects index one as
the last vector instead of index two. If the alignment remainder
consumes that vector, a pinned user page is released even though it is
still present in the completion-side vector array. Completion releases
the same page again.
Pass the first vector index to the alignment helper and use an offset of
one for bounced reads. After trimming, also synchronize the bounce
folio length with the aligned bio size. Release the bounce folio if
alignment removes all user data.
Fixes: e7b8b3c5b2a6 ("block: align down bounces bios")
Cc: stable@vger.kernel.org
Signed-off-by: 0wnerD1ed <l7z@0b1t.tech>
---
block/bio.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/block/bio.c b/block/bio.c
index f2a5f4d0a967..6b7d893df169 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1191,7 +1191,7 @@ void bio_iov_bvec_set(struct bio *bio, const struct iov_iter *iter)
* for the next iteration.
*/
static int bio_iov_iter_align_down(struct bio *bio, struct iov_iter *iter,
- unsigned len_align_mask)
+ unsigned int len_align_mask, unsigned int first_vec)
{
size_t nbytes = bio->bi_iter.bi_size & len_align_mask;
@@ -1201,7 +1201,8 @@ static int bio_iov_iter_align_down(struct bio *bio, struct iov_iter *iter,
iov_iter_revert(iter, nbytes);
bio->bi_iter.bi_size -= nbytes;
do {
- struct bio_vec *bv = &bio->bi_io_vec[bio->bi_vcnt - 1];
+ struct bio_vec *bv =
+ &bio->bi_io_vec[first_vec + bio->bi_vcnt - 1];
if (nbytes < bv->bv_len) {
bv->bv_len -= nbytes;
@@ -1276,7 +1277,7 @@ int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter,
if (is_pci_p2pdma_page(bio->bi_io_vec->bv_page))
bio->bi_opf |= REQ_NOMERGE;
- return bio_iov_iter_align_down(bio, iter, len_align_mask);
+ return bio_iov_iter_align_down(bio, iter, len_align_mask, 0);
}
static struct folio *folio_alloc_greedy(gfp_t gfp, size_t *size,
@@ -1360,7 +1361,7 @@ static int bio_iov_iter_bounce_write(struct bio *bio, struct iov_iter *iter,
if (!bio->bi_iter.bi_size)
return -ENOMEM;
- return bio_iov_iter_align_down(bio, iter, minsize - 1);
+ return bio_iov_iter_align_down(bio, iter, minsize - 1, 0);
}
static int bio_iov_iter_bounce_read(struct bio *bio, struct iov_iter *iter,
@@ -1368,6 +1369,7 @@ static int bio_iov_iter_bounce_read(struct bio *bio, struct iov_iter *iter,
{
size_t len = min3(iov_iter_count(iter), maxlen, SZ_1M);
struct folio *folio;
+ int ret;
folio = folio_alloc_greedy(GFP_KERNEL, &len, minsize);
if (!folio)
@@ -1398,7 +1400,13 @@ static int bio_iov_iter_bounce_read(struct bio *bio, struct iov_iter *iter,
bvec_set_folio(&bio->bi_io_vec[0], folio, bio->bi_iter.bi_size, 0);
if (iov_iter_extract_will_pin(iter))
bio_set_flag(bio, BIO_PAGE_PINNED);
- return bio_iov_iter_align_down(bio, iter, minsize - 1);
+ ret = bio_iov_iter_align_down(bio, iter, minsize - 1, 1);
+ if (ret) {
+ folio_put(folio);
+ return ret;
+ }
+ bio->bi_io_vec[0].bv_len = bio->bi_iter.bi_size;
+ return 0;
}
/**
--
2.34.1
^ permalink raw reply related
* Re: [PATCH V4 8/9] null_blk: serialize configfs attribute stores with device setup
From: Nilay Shroff @ 2026-07-10 12:30 UTC (permalink / raw)
To: Zizhi Wo, axboe, dlemoal, kch, johannes.thumshirn, kbusch,
bvanassche, linux-block
Cc: linux-kernel, yangerkun, chengzhihao1, wozizhi
In-Reply-To: <20260709100452.3520482-9-wozizhi@huaweicloud.com>
On 7/9/26 3:34 PM, Zizhi Wo wrote:
> From: Zizhi Wo <wozizhi@huawei.com>
>
> 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.
>
> Also reset ret to 0 after the input parsing so that within the locked
> section ret is purely a status code, rather than carrying the byte count
> returned by nullb_device_##TYPE##_attr_store(). The field is then written
> only on success via if (!ret), giving a single consistent rule for both the
> apply_fn and the APPLY=NULL paths.
>
> Fixes: 45919fbfe1c4 ("null_blk: Enable modifying 'submit_queues' after an instance has been configured")
> Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
Looks good to me.
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
^ permalink raw reply
* Re: [PATCH V4 9/9] null_blk: serialize configfs attribute shows with the file-scope lock
From: Nilay Shroff @ 2026-07-10 12:31 UTC (permalink / raw)
To: Zizhi Wo, axboe, dlemoal, kch, johannes.thumshirn, kbusch,
bvanassche, linux-block
Cc: linux-kernel, yangerkun, chengzhihao1, wozizhi
In-Reply-To: <20260709100452.3520482-10-wozizhi@huaweicloud.com>
On 7/9/26 3:34 PM, Zizhi Wo wrote:
> 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>
> Signed-off-by: Zizhi Wo<wozizhi@huawei.com>
Looks good to me.
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
^ permalink raw reply
* Re: [PATCH V4 1/9] null_blk: use DEFINE_MUTEX for the file-scope mutex
From: Nilay Shroff @ 2026-07-10 12:35 UTC (permalink / raw)
To: Zizhi Wo, axboe, dlemoal, kch, johannes.thumshirn, kbusch,
bvanassche, linux-block
Cc: linux-kernel, yangerkun, chengzhihao1, wozizhi
In-Reply-To: <20260709100452.3520482-2-wozizhi@huaweicloud.com>
On 7/9/26 3:34 PM, Zizhi Wo wrote:
> In null_init(), mutex_init(&lock) currently happens after
> configfs_register_subsystem(), which exposes the nullb subsystem to
> userspace. A racing mkdir() into/sys/kernel/config/nullb/ can reach
> null_find_dev_by_name() -> mutex_lock(&lock) before the mutex is
> initialized, trigger warning:
Looks good to me.
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
^ permalink raw reply
* Re: [PATCH V4 2/9] null_blk: register configfs subsystem after creating default devices
From: Nilay Shroff @ 2026-07-10 12:36 UTC (permalink / raw)
To: Zizhi Wo, axboe, dlemoal, kch, johannes.thumshirn, kbusch,
bvanassche, linux-block
Cc: linux-kernel, yangerkun, chengzhihao1, wozizhi
In-Reply-To: <20260709100452.3520482-3-wozizhi@huaweicloud.com>
On 7/9/26 3:34 PM, Zizhi Wo wrote:
> 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:
Looks good to me.
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
^ permalink raw reply
* Re: [PATCH V4 4/9] null_blk: free global tag_set on init error path
From: Nilay Shroff @ 2026-07-10 12:37 UTC (permalink / raw)
To: Zizhi Wo, axboe, dlemoal, kch, johannes.thumshirn, kbusch,
bvanassche, linux-block
Cc: linux-kernel, yangerkun, chengzhihao1, wozizhi
In-Reply-To: <20260709100452.3520482-5-wozizhi@huaweicloud.com>
On 7/9/26 3:34 PM, Zizhi Wo wrote:
> 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).
Looks good to me.
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
^ permalink raw reply
* Re: [PATCH V4 7/9] null_blk: reject per-device queue resize for shared tag set
From: Nilay Shroff @ 2026-07-10 12:34 UTC (permalink / raw)
To: Zizhi Wo, axboe, dlemoal, kch, johannes.thumshirn, kbusch,
bvanassche, linux-block
Cc: linux-kernel, yangerkun, chengzhihao1, wozizhi
In-Reply-To: <20260709100452.3520482-8-wozizhi@huaweicloud.com>
On 7/9/26 3:34 PM, Zizhi Wo wrote:
> From: Zizhi Wo <wozizhi@huawei.com>
>
> 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: 45919fbfe1c4 ("null_blk: Enable modifying 'submit_queues' after an instance has been configured")
> Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
> ---
> drivers/block/null_blk/main.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
> index 6d30591abb28..340ecc0a331e 100644
> --- a/drivers/block/null_blk/main.c
> +++ b/drivers/block/null_blk/main.c
> @@ -380,10 +380,19 @@ static int nullb_update_nr_hw_queues(struct nullb_device *dev,
> int ret, nr_hw_queues;
>
> if (!dev->nullb)
> return 0;
>
> + /*
> + * A shared tag_set is mapped via the module-wide queue counts, so a
> + * per-device resize is meaningless. On shrink it would also leave
> + * mq_map[] pointing at NULLed hctx slots, causing a NULL deref in
> + * blk_mq_map_swqueue(). Reject it.
> + */
> + if (dev->nullb->tag_set == &tag_set)
> + return -EINVAL;
> +
Wouldn't it be simpler to check dev->shared_tags here instead? Since the restriction
is specifically for devices configured with shared tags, that seems a bit easier to
reason about than checking whether dev->nullb->tag_set points to the global tag_set.
Something like this:
if (dev->shared_tags)
return -EINVAL;
Thanks,
--Nilay
^ permalink raw reply
* Re: [PATCH] block: remove redundant GD_NEED_PART_SCAN in add_disk_final()
From: Connor Williamson @ 2026-07-10 12:40 UTC (permalink / raw)
To: axboe; +Cc: linux-block
In-Reply-To: <20260615130715.53693-1-connordw@amazon.com>
Hi Jens,
Have you had a chance to look at this patch?
Thanks,
Connor
^ permalink raw reply
* Re: [PATCH V4 3/9] null_blk: move unregister_blkdev() after destroying dev in null_exit()
From: Nilay Shroff @ 2026-07-10 12:36 UTC (permalink / raw)
To: Zizhi Wo, axboe, dlemoal, kch, johannes.thumshirn, kbusch,
bvanassche, linux-block
Cc: linux-kernel, yangerkun, chengzhihao1, wozizhi
In-Reply-To: <20260709100452.3520482-4-wozizhi@huaweicloud.com>
On 7/9/26 3:34 PM, Zizhi Wo wrote:
> 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.
Looks good to me.
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
^ permalink raw reply
page: | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox