* Re: [PATCH] blk-mq: always take a queue reference in blk_mq_submit_bio
From: Keith Busch @ 2026-05-20 14:59 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: axboe, linux-block
In-Reply-To: <20260520084905.1092158-1-hch@lst.de>
On Wed, May 20, 2026 at 10:49:05AM +0200, Christoph Hellwig wrote:
> When submitting a bio to blk-mq, if the task should sleep after peeking
> a cached request, but before it pops it, the plug flushes and calls
> blk_mq_free_plug_rqs, freeing the cached_rqs. This creates a
> use-after-free bug.
>
> Fix this by alway grabbing a queue usage reference in blk_mq_submit_bio.
> While past commit claims that this is slow, we're better safe than
> fast as a first priority.
>
> The code had already warned of this possibility, and specifically popped
> the request before other known blocking calls, but it didn't handle a
> blocking GFP_NOIO alloc. Under memory pressure, allocating the split bio
> or the integrity payload are two such cases that can block. The blk-mq
> submit_bio function continues using the peeked request that was already
> freed and re-initialized, so the driver receives that request with a
> NULL'ed mq_hctx, and inevitably panics.
Thanks, this looks good to me!
Reviewed-by: Keith Busch <kbusch@kernel.org>
^ permalink raw reply
* Re: [PATCH] blk-throttle: schedule parent dispatch in tg_flush_bios()
From: Tao Cui @ 2026-05-20 14:20 UTC (permalink / raw)
To: shinichiro.kawasaki; +Cc: axboe, cgroups, cuitao, josef, linux-block, tj
In-Reply-To: <ag2owaQQoigp_fSV@shinmob>
Hello
Thank you for catching this.
I tested throtl/004 locally on the upstream
v7.1-rc4-next-20260519 kernel with the following results:
Without the patch:
throtl/004 (nullb) (delete disk while IO is throttled) [passed]
runtime 1.054s
throtl/004 (sdebug) (delete disk while IO is throttled) [passed]
runtime 1.212s
With the patch applied:
throtl/004 (nullb) (delete disk while IO is throttled) [passed]
runtime 0.824s
throtl/004 (sdebug) (delete disk while IO is throttled) [passed]
runtime 0.962s
I was unable to reproduce the failure.
Could you share your environment details (kernel .config, scsi_debug module parameters, blktests version)?
Thanks,
Tao
^ permalink raw reply
* Re: [PATCH] blk-throttle: schedule parent dispatch in tg_flush_bios()
From: Shin'ichiro Kawasaki @ 2026-05-20 12:33 UTC (permalink / raw)
To: Tao Cui; +Cc: tj, josef, axboe, cgroups, linux-block
In-Reply-To: <20260520062420.1762788-1-cuitao@kylinos.cn>
On May 20, 2026 / 14:24, Tao Cui wrote:
> tg_flush_bios() schedules pending_timer on the child tg's own
> service_queue, which causes throtl_pending_timer_fn() to dispatch from
> the child's pending_tree. For leaf cgroups this tree is empty, so the
> timer fires and exits without dispatching the throttled bio.
>
> The throttled bio sits in the parent's pending_tree with disptime set
> to jiffies (THROTL_TG_CANCELING zeroes all dispatch times), but the
> parent's timer is never explicitly rescheduled. The bio only gets
> dispatched when the parent timer eventually fires at its previously
> scheduled expiry.
>
> Fix by calling throtl_schedule_next_dispatch(sq->parent_sq, true)
> instead, matching what tg_set_limit() already does. This forces the
> parent's dispatch cycle to run immediately and flush all canceling
> bios without waiting for a stale timer.
>
> Signed-off-by: Tao Cui <cuitao@kylinos.cn>
Hello Tao,
Trial runs of blktests CI for this patch caught the failure of test case
throtl/004 using scsi_debug device. I tried to recreate the failure manually.
I applied the patch on v7.1-rc4 kernel, and ran the test case and got the
result below. Is this failure expected?
# ./check throtl/004
throtl/004 (nullb) (delete disk while IO is throttled) [passed]
runtime 1.546s ... 1.332s
throtl/004 (sdebug) (delete disk while IO is throttled) [failed]
runtime 2.654s ... 2.435s
--- tests/throtl/004.out 2026-03-20 14:25:50.478000000 +0900
+++ /home/shin/Blktests/blktests/results/nodev_sdebug/throtl/004.out.bad 2026-05-20 21:26:14.470000000 +0900
@@ -1,3 +1,2 @@
Running throtl/004
-Input/output error
Test complete
^ permalink raw reply
* Re: [PATCH v8 3/3] powerpc/pseries: PLPKS SED Opal keystore support
From: Dan Carpenter @ 2026-05-20 10:24 UTC (permalink / raw)
To: gjoyce
Cc: linux-block, axboe, linuxppc-dev, jonathan.derrick, brking,
msuchanek, mpe, nayna, akpm, ndesaulniers, nathan, jarkko,
okozina
In-Reply-To: <20231004201957.1451669-4-gjoyce@linux.vnet.ibm.com>
On Wed, Oct 04, 2023 at 03:19:57PM -0500, gjoyce@linux.vnet.ibm.com wrote:
> +int sed_read_key(char *keyname, char *key, u_int *keylen)
> +{
> + struct plpks_var var;
> + struct plpks_sed_object_data data;
> + int ret;
> + u_int len;
> +
> + plpks_init_var(&var, keyname);
> +
> + if (!plpks_sed_available)
> + return -EOPNOTSUPP;
> +
> + var.data = (u8 *)&data;
> + var.datalen = sizeof(data);
> +
> + ret = plpks_read_os_var(&var);
> + if (ret != 0)
> + return ret;
> +
> + len = min_t(u16, be32_to_cpu(data.key_len), var.datalen);
^^^^^^^^^^^
This isn't the correct limit. This is the number of bytes that we
copied into data. Probably it's sizeof(data) and, hopefully, it's
at least the offsetof(struct plpks_sed_object_data, key).
To me the temptation is the initialize data to zero and
s/var.datalen/sizeof(data.key)/.
> + memcpy(key, data.key, len);
^^^^^^^^
> + key[len] = '\0';
> + *keylen = len;
> +
> + return 0;
> +}
regards,
dan carpenter
^ permalink raw reply
* [PATCH] block, bfq: skip dispatch when selected queue has no next_rq
From: dayou5941 @ 2026-05-20 9:50 UTC (permalink / raw)
To: linux-block; +Cc: yukuai, axboe, liyouhong
From: liyouhong <liyouhong@kylinos.cn>
bfq_dispatch_rq_from_bfqq() dispatches bfqq->next_rq directly. Add a
guard in __bfq_dispatch_request() so that dispatch is skipped if the
selected queue unexpectedly has no next request.
This keeps behavior unchanged in normal paths while avoiding entering
the dispatch helper with a missing next_rq.
Signed-off-by: liyouhong <liyouhong@kylinos.cn>
diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 141c602d5e85..c3d9d02064b7 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -5226,7 +5226,7 @@ static struct request *__bfq_dispatch_request(struct blk_mq_hw_ctx *hctx)
goto exit;
bfqq = bfq_select_queue(bfqd);
- if (!bfqq)
+ if (!bfqq || !bfqq->next_rq)
goto exit;
rq = bfq_dispatch_rq_from_bfqq(bfqd, bfqq);
--
2.25.1
^ permalink raw reply related
* [PATCH] block: stack zoned resource limits
From: Yao Sang @ 2026-05-20 9:12 UTC (permalink / raw)
To: linux-block; +Cc: Jens Axboe, Yao Sang
This was found while debugging a zoned NVMe multipath setup, where the
namespace head reported 0/0 for max_open_zones and max_active_zones
while the live path still reported finite limits.
blk_stack_limits() already combines several zoned queue limits, but it
leaves max_open_zones and max_active_zones unchanged. Since 0 means "no
limit" for both values, stacked zoned devices can preserve bogus 0/0
limits even when the underlying queue advertises finite values.
Stack max_open_zones and max_active_zones with min_not_zero(), and
clear them when the resulting queue is not zoned.
Signed-off-by: Yao Sang <sangyao@kylinos.cn>
---
block/blk-settings.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/block/blk-settings.c b/block/blk-settings.c
index 78c83817b9d3..88b6c3703101 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -814,6 +814,10 @@ int blk_stack_limits(struct queue_limits *t, struct queue_limits *b,
t->max_hw_zone_append_sectors = min(t->max_hw_zone_append_sectors,
b->max_hw_zone_append_sectors);
+ t->max_open_zones = min_not_zero(t->max_open_zones,
+ b->max_open_zones);
+ t->max_active_zones = min_not_zero(t->max_active_zones,
+ b->max_active_zones);
t->seg_boundary_mask = min_not_zero(t->seg_boundary_mask,
b->seg_boundary_mask);
@@ -921,6 +925,8 @@ int blk_stack_limits(struct queue_limits *t, struct queue_limits *b,
t->zone_write_granularity = max(t->zone_write_granularity,
b->zone_write_granularity);
if (!(t->features & BLK_FEAT_ZONED)) {
+ t->max_open_zones = 0;
+ t->max_active_zones = 0;
t->zone_write_granularity = 0;
t->max_zone_append_sectors = 0;
}
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v2] loop: Fix NULL pointer dereference by synchronizing lo_release and loop_queue_rq
From: Ming Lei @ 2026-05-20 8:54 UTC (permalink / raw)
To: Tetsuo Handa
Cc: Bart Van Assche, Jens Axboe, Christoph Hellwig, Damien Le Moal,
linux-block, LKML, Andrew Morton
In-Reply-To: <1ab8c579-eb76-4227-8a72-6ec819135219@I-love.SAKURA.ne.jp>
On Wed, May 20, 2026 at 05:20:01PM +0900, Tetsuo Handa wrote:
> On 2026/05/20 16:49, Ming Lei wrote:
> > On Wed, May 20, 2026 at 03:36:12PM +0900, Tetsuo Handa wrote:
> >> On 2026/05/20 12:06, Ming Lei wrote:
> >>> The IO after close(loop) should be from writeback. rcu/sruc isn't necessary,
> >>
> >> Gemini's comment is that drain_workqueue() is not sufficient for waiting for
> >> do_req_filebacked(REQ_OP_WRITE) requests with cmd->use_aio == true case to complete.
> >
> > Anything cleared in __loop_clr_fd() is not used by lo_rw_aio_complete() & lo_complete_rq().
>
> "struct inode *inode = file_inode(iocb->ki_filp);" in kiocb_end_write() from
> lo_rw_aio_do_completion() can dereference "struct file *" with refcount == 0 (UAF)
> because fput() in __loop_clr_fd() can be the last reference to that file.
OK, you are right.
It can be handled by adding sync_blockdev(lo->lo_device) in __loop_clr_fd()
because IO can be from writeback only when loop disk is closed.
Please feel free to test the following change:
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 0000913f7efc..bbd15974a082 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1118,6 +1118,8 @@ static void __loop_clr_fd(struct loop_device *lo)
struct file *filp;
gfp_t gfp = lo->old_gfp_mask;
+ sync_blockdev(lo->lo_device);
+
spin_lock_irq(&lo->lo_lock);
filp = lo->lo_backing_file;
lo->lo_backing_file = NULL;
> >
> > So why isn't drain_workqueue() enough for cmd->use_aio?
>
> In addition to possible UAF above, the assumption at
> https://elixir.bootlin.com/linux/v7.1-rc4/source/drivers/block/loop.c#L1134
> is currently broken due to this race problem.
That shouldn't be one issue otherwise bio based driver can't work with updating
limits.
Thanks,
Ming
^ permalink raw reply related
* [PATCH] blk-mq: always take a queue reference in blk_mq_submit_bio
From: Christoph Hellwig @ 2026-05-20 8:49 UTC (permalink / raw)
To: axboe; +Cc: kbusch, linux-block
When submitting a bio to blk-mq, if the task should sleep after peeking
a cached request, but before it pops it, the plug flushes and calls
blk_mq_free_plug_rqs, freeing the cached_rqs. This creates a
use-after-free bug.
Fix this by alway grabbing a queue usage reference in blk_mq_submit_bio.
While past commit claims that this is slow, we're better safe than
fast as a first priority.
The code had already warned of this possibility, and specifically popped
the request before other known blocking calls, but it didn't handle a
blocking GFP_NOIO alloc. Under memory pressure, allocating the split bio
or the integrity payload are two such cases that can block. The blk-mq
submit_bio function continues using the peeked request that was already
freed and re-initialized, so the driver receives that request with a
NULL'ed mq_hctx, and inevitably panics.
Large parts of the commit message are stolen from an earlier attempt
to fix this issue by Keith Busch <kbusch@kernel.org>.
Fixes: b0077e269f6c1 ("blk-mq: make sure active queue usage is held for bio_integrity_prep()")
Fixes: 7b4f36cd22a65 ("block: ensure we hold a queue reference when using queue limits")
Reported-by: Keith Busch <kbusch@kernel.org>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
block/blk-mq.c | 65 +++++++++++++++++++-------------------------------
1 file changed, 24 insertions(+), 41 deletions(-)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index d0c37daf568f..a9af1a9faedc 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -3075,43 +3075,42 @@ static struct request *blk_mq_get_new_requests(struct request_queue *q,
}
/*
- * Check if there is a suitable cached request and return it.
+ * Check if there is a suitable cached request and use it if possible.
*/
-static struct request *blk_mq_peek_cached_request(struct blk_plug *plug,
- struct request_queue *q, blk_opf_t opf)
+static struct request *blk_mq_pop_cached_request(struct blk_plug *plug,
+ struct request_queue *q, struct bio *bio)
{
- enum hctx_type type = blk_mq_get_hctx_type(opf);
+ enum hctx_type type = blk_mq_get_hctx_type(bio->bi_opf);
struct request *rq;
if (!plug)
return NULL;
+
+ /*
+ * Note: we must not sleep between the peek of the cached_rqs list here
+ * and the pop below.
+ */
rq = rq_list_peek(&plug->cached_rqs);
if (!rq || rq->q != q)
return NULL;
if (type != rq->mq_hctx->type &&
(type != HCTX_TYPE_READ || rq->mq_hctx->type != HCTX_TYPE_DEFAULT))
return NULL;
- if (op_is_flush(rq->cmd_flags) != op_is_flush(opf))
+ if (op_is_flush(rq->cmd_flags) != op_is_flush(bio->bi_opf))
return NULL;
- return rq;
-}
-
-static void blk_mq_use_cached_rq(struct request *rq, struct blk_plug *plug,
- struct bio *bio)
-{
if (rq_list_pop(&plug->cached_rqs) != rq)
WARN_ON_ONCE(1);
- /*
- * If any qos ->throttle() end up blocking, we will have flushed the
- * plug and hence killed the cached_rq list as well. Pop this entry
- * before we throttle.
- */
rq_qos_throttle(rq->q, bio);
-
blk_mq_rq_time_init(rq, blk_time_get_ns());
rq->cmd_flags = bio->bi_opf;
INIT_LIST_HEAD(&rq->queuelist);
+
+ /*
+ * Drop the extra queue reference from the cached request.
+ */
+ blk_queue_exit(q);
+ return rq;
}
static bool bio_unaligned(const struct bio *bio, struct request_queue *q)
@@ -3149,11 +3148,6 @@ void blk_mq_submit_bio(struct bio *bio)
struct request *rq;
blk_status_t ret;
- /*
- * If the plug has a cached request for this queue, try to use it.
- */
- rq = blk_mq_peek_cached_request(plug, q, bio->bi_opf);
-
/*
* A BIO that was released from a zone write plug has already been
* through the preparation in this function, already holds a reference
@@ -3162,19 +3156,11 @@ void blk_mq_submit_bio(struct bio *bio)
*/
if (bio_zone_write_plugging(bio)) {
nr_segs = bio->__bi_nr_segments;
- if (rq)
- blk_queue_exit(q);
goto new_request;
}
- /*
- * The cached request already holds a q_usage_counter reference and we
- * don't have to acquire a new one if we use it.
- */
- if (!rq) {
- if (unlikely(bio_queue_enter(bio)))
- return;
- }
+ if (unlikely(bio_queue_enter(bio)))
+ return;
/*
* Device reconfiguration may change logical block size or reduce the
@@ -3210,9 +3196,11 @@ void blk_mq_submit_bio(struct bio *bio)
}
new_request:
- if (rq) {
- blk_mq_use_cached_rq(rq, plug, bio);
- } else {
+ /*
+ * If the plug has a cached request for this queue, try to use it.
+ */
+ rq = blk_mq_pop_cached_request(plug, q, bio);
+ if (!rq) {
rq = blk_mq_get_new_requests(q, plug, bio);
if (unlikely(!rq)) {
if (bio->bi_opf & REQ_NOWAIT)
@@ -3257,12 +3245,7 @@ void blk_mq_submit_bio(struct bio *bio)
return;
queue_exit:
- /*
- * Don't drop the queue reference if we were trying to use a cached
- * request and thus didn't acquire one.
- */
- if (!rq)
- blk_queue_exit(q);
+ blk_queue_exit(q);
}
#ifdef CONFIG_BLK_MQ_STACKING
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v3 04/10] block: introduce dma map backed bio type
From: Christoph Hellwig @ 2026-05-20 8:30 UTC (permalink / raw)
To: Pavel Begunkov
Cc: Christoph Hellwig, Jens Axboe, Keith Busch, Sagi Grimberg,
Alexander Viro, Christian Brauner, Andrew Morton, Sumit Semwal,
Christian König, linux-block, linux-kernel, linux-nvme,
linux-fsdevel, io-uring, linux-media, dri-devel, linaro-mm-sig,
Nitesh Shetty, Kanchan Joshi, Anuj Gupta, Tushar Gohad,
William Power, Phil Cayton, Jason Gunthorpe
In-Reply-To: <24833f76-2289-4859-86d1-9215b11a1258@gmail.com>
On Mon, May 18, 2026 at 11:29:54AM +0100, Pavel Begunkov wrote:
>>> BIO_ZONE_WRITE_PLUGGING, /* bio handled through zone write plugging */
>>> BIO_EMULATES_ZONE_APPEND, /* bio emulates a zone append operation */
>>> + BIO_DMABUF_MAP, /* Using premmaped dma buffers */
>>
>> Shouldn't this be a REQ_ flag as we should never mix and match bios with
>> and without this flag in a single request?
>
> Do you mean adding both and propagating it from bio to req? submit_bio()
> takes a bio, so we still need to set it there before it reaches blk-mq.
> And there might be bio-based drivers using it in the future.
I think I forgot to reply to this, so let's do this now.
REQ_ is actually used by both bios and requests, so if you set it in
bio->bi_opf it will automatically get propagated to the request, but
it can also always be tested on the bio, including by bio-based
drivers.
^ permalink raw reply
* Re: [PATCH v2] loop: Fix NULL pointer dereference by synchronizing lo_release and loop_queue_rq
From: Tetsuo Handa @ 2026-05-20 8:20 UTC (permalink / raw)
To: Ming Lei
Cc: Bart Van Assche, Jens Axboe, Christoph Hellwig, Damien Le Moal,
linux-block, LKML, Andrew Morton
In-Reply-To: <ag1nfIFcykmQHbkk@fedora>
On 2026/05/20 16:49, Ming Lei wrote:
> On Wed, May 20, 2026 at 03:36:12PM +0900, Tetsuo Handa wrote:
>> On 2026/05/20 12:06, Ming Lei wrote:
>>> The IO after close(loop) should be from writeback. rcu/sruc isn't necessary,
>>
>> Gemini's comment is that drain_workqueue() is not sufficient for waiting for
>> do_req_filebacked(REQ_OP_WRITE) requests with cmd->use_aio == true case to complete.
>
> Anything cleared in __loop_clr_fd() is not used by lo_rw_aio_complete() & lo_complete_rq().
"struct inode *inode = file_inode(iocb->ki_filp);" in kiocb_end_write() from
lo_rw_aio_do_completion() can dereference "struct file *" with refcount == 0 (UAF)
because fput() in __loop_clr_fd() can be the last reference to that file.
>
> So why isn't drain_workqueue() enough for cmd->use_aio?
In addition to possible UAF above, the assumption at
https://elixir.bootlin.com/linux/v7.1-rc4/source/drivers/block/loop.c#L1134
is currently broken due to this race problem.
^ permalink raw reply
* [PATCH] block: partitions: replace __get_free_page() with kmalloc()
From: Mike Rapoport (Microsoft) @ 2026-05-20 8:15 UTC (permalink / raw)
To: Jens Axboe; +Cc: Mike Rapoport, linux-block, linux-kernel, linux-mm
check_partition() allocates a buffer to use as backing buffer for
seq_buf.
This buffer can be allocated with kmalloc() as there's nothing special
about it to go directly to the page allocator.
Replace use of __get_free_page() with kmalloc() and free_page() with
kfree().
Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
This is a (tiny) part of larger work of replacing page allocator calls
with kmalloc:
Also in git:
https://git.kernel.org/pub/scm/linux/kernel/git/rppt/linux.git gfp-to-kmalloc/block
Signed-off-by: Mike Rapoport <rppt@kernel.org>
---
block/partitions/core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/block/partitions/core.c b/block/partitions/core.c
index 5d5332ce586b..b5c59b79ca7c 100644
--- a/block/partitions/core.c
+++ b/block/partitions/core.c
@@ -124,7 +124,7 @@ static struct parsed_partitions *check_partition(struct gendisk *hd)
state = allocate_partitions(hd);
if (!state)
return NULL;
- state->pp_buf.buffer = (char *)__get_free_page(GFP_KERNEL);
+ state->pp_buf.buffer = kmalloc(PAGE_SIZE, GFP_KERNEL);
if (!state->pp_buf.buffer) {
free_partitions(state);
return NULL;
@@ -154,7 +154,7 @@ static struct parsed_partitions *check_partition(struct gendisk *hd)
if (res > 0) {
printk(KERN_INFO "%s", seq_buf_str(&state->pp_buf));
- free_page((unsigned long)state->pp_buf.buffer);
+ kfree(state->pp_buf.buffer);
return state;
}
if (state->access_beyond_eod)
@@ -170,7 +170,7 @@ static struct parsed_partitions *check_partition(struct gendisk *hd)
printk(KERN_INFO "%s", seq_buf_str(&state->pp_buf));
}
- free_page((unsigned long)state->pp_buf.buffer);
+ kfree(state->pp_buf.buffer);
free_partitions(state);
return ERR_PTR(res);
}
---
base-commit: 5d6919055dec134de3c40167a490f33c74c12581
change-id: 20260520-block-25582753fd38
Best regards,
--
Sincerely yours,
Mike.
^ permalink raw reply related
* Re: [PATCH v2] loop: Fix NULL pointer dereference by synchronizing lo_release and loop_queue_rq
From: Ming Lei @ 2026-05-20 7:49 UTC (permalink / raw)
To: Tetsuo Handa
Cc: Bart Van Assche, Jens Axboe, Christoph Hellwig, Damien Le Moal,
linux-block, LKML, Andrew Morton
In-Reply-To: <94076bc9-2c09-4bb6-8468-b6b8af419cb9@I-love.SAKURA.ne.jp>
On Wed, May 20, 2026 at 03:36:12PM +0900, Tetsuo Handa wrote:
> On 2026/05/20 12:06, Ming Lei wrote:
> > The IO after close(loop) should be from writeback. rcu/sruc isn't necessary,
>
> Gemini's comment is that drain_workqueue() is not sufficient for waiting for
> do_req_filebacked(REQ_OP_WRITE) requests with cmd->use_aio == true case to complete.
Anything cleared in __loop_clr_fd() is not used by lo_rw_aio_complete() & lo_complete_rq().
So why isn't drain_workqueue() enough for cmd->use_aio?
Thanks,
Ming
^ permalink raw reply
* [PATCH] rust: add procedural macro for declaring configfs attributes
From: Malte Wechter @ 2026-05-20 7:40 UTC (permalink / raw)
To: Andreas Hindborg, Breno Leitao, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Jens Axboe
Cc: linux-kernel, rust-for-linux, linux-block, Malte Wechter
Implement `configfs_attrs!` as a procedural macro using `syn`, this
improves readability and maintainability. Remove the old macro and
replace all uses with the new macro. Add the new macro implementation
file to MAINTAINERS.
Signed-off-by: Malte Wechter <maltewechter@gmail.com>
---
MAINTAINERS | 1 +
drivers/block/rnull/configfs.rs | 2 +-
rust/kernel/configfs.rs | 251 ----------------------------------------
rust/macros/configfs_attrs.rs | 182 +++++++++++++++++++++++++++++
rust/macros/lib.rs | 85 ++++++++++++++
samples/rust/rust_configfs.rs | 2 +-
6 files changed, 270 insertions(+), 253 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 2fb1c75afd163..45f7a1ec93b45 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6464,6 +6464,7 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/a.hindborg/linux.git config
F: fs/configfs/
F: include/linux/configfs.h
F: rust/kernel/configfs.rs
+F: rust/macros/configfs_attrs.rs
F: samples/configfs/
F: samples/rust/rust_configfs.rs
diff --git a/drivers/block/rnull/configfs.rs b/drivers/block/rnull/configfs.rs
index 7c2eb5c0b7228..f28ec69d79846 100644
--- a/drivers/block/rnull/configfs.rs
+++ b/drivers/block/rnull/configfs.rs
@@ -4,8 +4,8 @@
use kernel::{
block::mq::gen_disk::{GenDisk, GenDiskBuilder},
configfs::{self, AttributeOperations},
- configfs_attrs,
fmt::{self, Write as _},
+ macros::configfs_attrs,
new_mutex,
page::PAGE_SIZE,
prelude::*,
diff --git a/rust/kernel/configfs.rs b/rust/kernel/configfs.rs
index 2339c6467325d..7a91e36677f52 100644
--- a/rust/kernel/configfs.rs
+++ b/rust/kernel/configfs.rs
@@ -791,254 +791,3 @@ fn as_ptr(&self) -> *const bindings::config_item_type {
self.item_type.get()
}
}
-
-/// Define a list of configfs attributes statically.
-///
-/// Invoking the macro in the following manner:
-///
-/// ```ignore
-/// let item_type = configfs_attrs! {
-/// container: configfs::Subsystem<Configuration>,
-/// data: Configuration,
-/// child: Child,
-/// attributes: [
-/// message: 0,
-/// bar: 1,
-/// ],
-/// };
-/// ```
-///
-/// Expands the following output:
-///
-/// ```ignore
-/// let item_type = {
-/// static CONFIGURATION_MESSAGE_ATTR: kernel::configfs::Attribute<
-/// 0,
-/// Configuration,
-/// Configuration,
-/// > = unsafe {
-/// kernel::configfs::Attribute::new({
-/// const S: &str = "message\u{0}";
-/// const C: &kernel::str::CStr = match kernel::str::CStr::from_bytes_with_nul(
-/// S.as_bytes()
-/// ) {
-/// Ok(v) => v,
-/// Err(_) => {
-/// core::panicking::panic_fmt(core::const_format_args!(
-/// "string contains interior NUL"
-/// ));
-/// }
-/// };
-/// C
-/// })
-/// };
-///
-/// static CONFIGURATION_BAR_ATTR: kernel::configfs::Attribute<
-/// 1,
-/// Configuration,
-/// Configuration
-/// > = unsafe {
-/// kernel::configfs::Attribute::new({
-/// const S: &str = "bar\u{0}";
-/// const C: &kernel::str::CStr = match kernel::str::CStr::from_bytes_with_nul(
-/// S.as_bytes()
-/// ) {
-/// Ok(v) => v,
-/// Err(_) => {
-/// core::panicking::panic_fmt(core::const_format_args!(
-/// "string contains interior NUL"
-/// ));
-/// }
-/// };
-/// C
-/// })
-/// };
-///
-/// const N: usize = (1usize + (1usize + 0usize)) + 1usize;
-///
-/// static CONFIGURATION_ATTRS: kernel::configfs::AttributeList<N, Configuration> =
-/// unsafe { kernel::configfs::AttributeList::new() };
-///
-/// {
-/// const N: usize = 0usize;
-/// unsafe { CONFIGURATION_ATTRS.add::<N, 0, _>(&CONFIGURATION_MESSAGE_ATTR) };
-/// }
-///
-/// {
-/// const N: usize = (1usize + 0usize);
-/// unsafe { CONFIGURATION_ATTRS.add::<N, 1, _>(&CONFIGURATION_BAR_ATTR) };
-/// }
-///
-/// static CONFIGURATION_TPE:
-/// kernel::configfs::ItemType<configfs::Subsystem<Configuration> ,Configuration>
-/// = kernel::configfs::ItemType::<
-/// configfs::Subsystem<Configuration>,
-/// Configuration
-/// >::new_with_child_ctor::<N,Child>(
-/// &THIS_MODULE,
-/// &CONFIGURATION_ATTRS
-/// );
-///
-/// &CONFIGURATION_TPE
-/// }
-/// ```
-#[macro_export]
-macro_rules! configfs_attrs {
- (
- container: $container:ty,
- data: $data:ty,
- attributes: [
- $($name:ident: $attr:literal),* $(,)?
- ] $(,)?
- ) => {
- $crate::configfs_attrs!(
- count:
- @container($container),
- @data($data),
- @child(),
- @no_child(x),
- @attrs($($name $attr)*),
- @eat($($name $attr,)*),
- @assign(),
- @cnt(0usize),
- )
- };
- (
- container: $container:ty,
- data: $data:ty,
- child: $child:ty,
- attributes: [
- $($name:ident: $attr:literal),* $(,)?
- ] $(,)?
- ) => {
- $crate::configfs_attrs!(
- count:
- @container($container),
- @data($data),
- @child($child),
- @no_child(),
- @attrs($($name $attr)*),
- @eat($($name $attr,)*),
- @assign(),
- @cnt(0usize),
- )
- };
- (count:
- @container($container:ty),
- @data($data:ty),
- @child($($child:ty)?),
- @no_child($($no_child:ident)?),
- @attrs($($aname:ident $aattr:literal)*),
- @eat($name:ident $attr:literal, $($rname:ident $rattr:literal,)*),
- @assign($($assign:block)*),
- @cnt($cnt:expr),
- ) => {
- $crate::configfs_attrs!(
- count:
- @container($container),
- @data($data),
- @child($($child)?),
- @no_child($($no_child)?),
- @attrs($($aname $aattr)*),
- @eat($($rname $rattr,)*),
- @assign($($assign)* {
- const N: usize = $cnt;
- // The following macro text expands to a call to `Attribute::add`.
-
- // SAFETY: By design of this macro, the name of the variable we
- // invoke the `add` method on below, is not visible outside of
- // the macro expansion. The macro does not operate concurrently
- // on this variable, and thus we have exclusive access to the
- // variable.
- unsafe {
- $crate::macros::paste!(
- [< $data:upper _ATTRS >]
- .add::<N, $attr, _>(&[< $data:upper _ $name:upper _ATTR >])
- )
- };
- }),
- @cnt(1usize + $cnt),
- )
- };
- (count:
- @container($container:ty),
- @data($data:ty),
- @child($($child:ty)?),
- @no_child($($no_child:ident)?),
- @attrs($($aname:ident $aattr:literal)*),
- @eat(),
- @assign($($assign:block)*),
- @cnt($cnt:expr),
- ) =>
- {
- $crate::configfs_attrs!(
- final:
- @container($container),
- @data($data),
- @child($($child)?),
- @no_child($($no_child)?),
- @attrs($($aname $aattr)*),
- @assign($($assign)*),
- @cnt($cnt),
- )
- };
- (final:
- @container($container:ty),
- @data($data:ty),
- @child($($child:ty)?),
- @no_child($($no_child:ident)?),
- @attrs($($name:ident $attr:literal)*),
- @assign($($assign:block)*),
- @cnt($cnt:expr),
- ) =>
- {
- $crate::macros::paste!{
- {
- $(
- // SAFETY: We are expanding `configfs_attrs`.
- static [< $data:upper _ $name:upper _ATTR >]:
- $crate::configfs::Attribute<$attr, $data, $data> =
- unsafe {
- $crate::configfs::Attribute::new(
- $crate::c_str!(::core::stringify!($name)),
- )
- };
- )*
-
-
- // We need space for a null terminator.
- const N: usize = $cnt + 1usize;
-
- // SAFETY: We are expanding `configfs_attrs`.
- static [< $data:upper _ATTRS >]:
- $crate::configfs::AttributeList<N, $data> =
- unsafe { $crate::configfs::AttributeList::new() };
-
- $($assign)*
-
- $(
- const [<$no_child:upper>]: bool = true;
-
- static [< $data:upper _TPE >] : $crate::configfs::ItemType<$container, $data> =
- $crate::configfs::ItemType::<$container, $data>::new::<N>(
- &THIS_MODULE, &[<$ data:upper _ATTRS >]
- );
- )?
-
- $(
- static [< $data:upper _TPE >]:
- $crate::configfs::ItemType<$container, $data> =
- $crate::configfs::ItemType::<$container, $data>::
- new_with_child_ctor::<N, $child>(
- &THIS_MODULE, &[<$ data:upper _ATTRS >]
- );
- )?
-
- & [< $data:upper _TPE >]
- }
- }
- };
-
-}
-
-pub use crate::configfs_attrs;
diff --git a/rust/macros/configfs_attrs.rs b/rust/macros/configfs_attrs.rs
new file mode 100644
index 0000000000000..f5c3a8d9bffbd
--- /dev/null
+++ b/rust/macros/configfs_attrs.rs
@@ -0,0 +1,182 @@
+// SPDX-License-Identifier: GPL-2.0
+
+use quote::{
+ quote, //
+ ToTokens,
+};
+
+use proc_macro2::{
+ Span, //
+};
+
+use syn::{
+ bracketed,
+ parse::{
+ Parse,
+ ParseStream, //
+ },
+ punctuated::Punctuated,
+ spanned::Spanned,
+ Ident,
+ LitInt,
+ Token,
+ Type, //
+};
+
+pub(crate) struct ConfigfsAttrs {
+ container: Type,
+ data: Type,
+ child: Option<Type>,
+ attributes: Vec<(Ident, LitInt)>,
+}
+
+fn parse_attribute_field(stream: ParseStream<'_>) -> syn::Result<(Ident, LitInt)> {
+ let id = stream.parse::<syn::Ident>()?;
+ let _colon = stream.parse::<Token![:]>()?;
+ let v = stream.parse::<LitInt>()?;
+ Ok((id, v))
+}
+
+fn parse_named_field(stream: ParseStream<'_>, name: &str) -> syn::Result<Type> {
+ let try_stream = stream.fork();
+ let name_id = try_stream.parse::<Ident>()?;
+ if name_id != name {
+ return Err(parse_field_error(name_id.span(), &name_id, name));
+ }
+ let _name_id = stream.parse::<Ident>()?;
+ let _colon = stream.parse::<Token![:]>()?;
+ let v = stream.parse::<Type>()?;
+ stream.parse::<Token![,]>()?;
+ Ok(v)
+}
+
+fn parse_attributes(stream: ParseStream<'_>) -> syn::Result<Vec<(Ident, LitInt)>> {
+ let attribute_id = stream.parse::<Ident>()?;
+ if attribute_id != "attributes" {
+ return Err(syn::Error::new(
+ attribute_id.span(),
+ format!(
+ "unexpected identifier: {}, expected: 'attributes'",
+ attribute_id
+ ),
+ ));
+ }
+ stream.parse::<Token![:]>()?;
+ let attr_stream;
+ let _bracket = bracketed!(attr_stream in stream);
+ let attributes = Punctuated::<(Ident, LitInt), Token![,]>::parse_terminated_with(
+ &attr_stream,
+ parse_attribute_field,
+ )?;
+ let attributes = attributes.into_iter().collect::<Vec<_>>();
+
+ stream.parse::<Option<Token![,]>>()?;
+ Ok(attributes)
+}
+
+fn parse_field_error(span: Span, name: &Ident, expected_name: &str) -> syn::Error {
+ syn::Error::new(
+ span,
+ format!("Unexpected field name, got: {name} expected: {expected_name}"),
+ )
+}
+
+impl Parse for ConfigfsAttrs {
+ fn parse(input: ParseStream<'_>) -> syn::Result<Self> {
+ let container = parse_named_field(input, "container")?;
+ let data = parse_named_field(input, "data")?;
+ let child = parse_named_field(input, "child").ok();
+ let attributes = parse_attributes(input)?;
+
+ Ok(ConfigfsAttrs {
+ container,
+ data,
+ child,
+ attributes,
+ })
+ }
+}
+
+fn make_static_ident<T: ToTokens>(ty: &T, suffix: &str) -> syn::Ident {
+ let raw_id = quote! { #ty }.to_string();
+
+ // Sanitizing syn::Type::Path, this is safe since it is
+ // only used as the identifier.
+ let normalized = raw_id
+ .split("::")
+ .map(|s| String::from(s.trim()))
+ .reduce(|a, b| format!("{a}_{b}"))
+ .expect("Cannot be empty")
+ .to_uppercase()
+ .replace(|c: char| !c.is_alphanumeric(), "_");
+
+ Ident::new(&format!("{}_{}", normalized, suffix), ty.span())
+}
+
+pub(crate) fn configfs_attrs(cfs_attrs: ConfigfsAttrs) -> proc_macro2::TokenStream {
+ let (container_ty, data_ty) = (&cfs_attrs.container, &cfs_attrs.data);
+
+ let data_tp_ident = make_static_ident(data_ty, "TPE");
+ let data_attr_ident = make_static_ident(data_ty, "ATTR_LIST");
+
+ let n = cfs_attrs.attributes.len() + 1;
+
+ let attr_list = quote! {
+ static #data_attr_ident: kernel::configfs::AttributeList<#n, #data_ty> =
+ // SAFETY: We are expanding `configfs_attrs`.
+ unsafe { kernel::configfs::AttributeList::new() };
+ };
+
+ let mut attrs = Vec::new();
+ for (attr_idx, (name, id)) in cfs_attrs.attributes.iter().enumerate() {
+ let name_with_attr = make_static_ident(name, "ATTR");
+
+ let id: u64 = match id.base10_parse::<u64>() {
+ Ok(v) => v,
+ Err(_) => {
+ return syn::Error::new(id.span(), "Could not parse attribute ID as a u64")
+ .to_compile_error();
+ }
+ };
+
+ attrs.push(quote! {
+ static #name_with_attr: kernel::configfs::Attribute<#id, #data_ty, #data_ty> =
+ // SAFETY: We are expanding `configfs_attrs`.
+ unsafe {
+ kernel::configfs::Attribute::new(kernel::c_str!(::core::stringify!(#name)))
+ };
+
+ // SAFETY: By design of this macro, the name of the variable we
+ // invoke the `add` method on below, is not visible outside of
+ // the macro expansion. The macro does not operate concurrently
+ // on this variable, and thus we have exclusive access to the
+ // variable.
+ unsafe { #data_attr_ident.add::<#attr_idx, #id, _>(&#name_with_attr) }
+ });
+ }
+
+ let has_child_code = if let Some(child) = cfs_attrs.child {
+ quote! { new_with_child_ctor::<#n, #child>}
+ } else {
+ quote! { new::<#n> }
+ };
+
+ let data_type = quote! {
+ {
+ static #data_tp_ident:
+ kernel::configfs::ItemType<#container_ty, #data_ty> =
+ kernel::configfs::ItemType::<#container_ty, #data_ty>::#has_child_code(
+ &THIS_MODULE, &#data_attr_ident
+ );
+ &#data_tp_ident
+ }
+ };
+
+ quote! {
+ {
+ #attr_list
+ #(#attrs)*
+ #data_type
+ }
+ }
+}
diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index 2cfd59e0f9e7c..745ba83c828b9 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -15,6 +15,8 @@
#![cfg_attr(not(CONFIG_RUSTC_HAS_SPAN_FILE), feature(proc_macro_span))]
mod concat_idents;
+#[cfg(CONFIG_CONFIGFS_FS)]
+mod configfs_attrs;
mod export;
mod fmt;
mod helpers;
@@ -489,3 +491,86 @@ pub fn kunit_tests(attr: TokenStream, input: TokenStream) -> TokenStream {
.unwrap_or_else(|e| e.into_compile_error())
.into()
}
+
+/// Define a list of configfs attributes statically.
+///
+/// # Examples
+///
+/// ```ignore
+/// let item_type = configfs_attrs! {
+/// container: configfs::Subsystem<Configuration>,
+/// data: Configuration,
+/// child: Child,
+/// attributes: [
+/// message: 0,
+/// bar: 1,
+/// ],
+/// };
+///```
+///
+/// Expands the following output:
+/// let item_type = {
+/// static CONFIGURATION_ATTR_LIST: kernel::configfs::AttributeList<
+/// 3usize,
+/// Configuration,
+/// > = unsafe { kernel::configfs::AttributeList::new() };
+/// static MESSAGE_ATTR: kernel::configfs::Attribute<
+/// 0u64,
+/// Configuration,
+/// Configuration,
+/// > = unsafe {
+/// kernel::configfs::Attribute::new({
+/// const S: &str = "message\u{0}";
+/// const C: &kernel::str::CStr = match kernel::str::CStr::from_bytes_with_nul(
+/// S.as_bytes(),
+/// ) {
+/// Ok(v) => v,
+/// Err(_) => {
+/// ::core::panicking::panic_fmt(
+/// format_args!("string contains interior NUL"),
+/// );
+/// }
+/// };
+/// C
+/// })
+/// };
+/// unsafe { CONFIGURATION_ATTR_LIST.add::<0usize, 0u64, _>(&MESSAGE_ATTR) }
+/// static BAR_ATTR: kernel::configfs::Attribute<
+/// 1u64,
+/// Configuration,
+/// Configuration,
+/// > = unsafe {
+/// kernel::configfs::Attribute::new({
+/// const S: &str = "bar\u{0}";
+/// const C: &kernel::str::CStr = match kernel::str::CStr::from_bytes_with_nul(
+/// S.as_bytes(),
+/// ) {
+/// Ok(v) => v,
+/// Err(_) => {
+/// ::core::panicking::panic_fmt(
+/// format_args!("string contains interior NUL"),
+/// );
+/// }
+/// };
+/// C
+/// })
+/// };
+/// unsafe { CONFIGURATION_ATTR_LIST.add::<1usize, 1u64, _>(&BAR_ATTR) }
+/// {
+/// static CONFIGURATION_TPE: kernel::configfs::ItemType<
+/// Subsystem<Configuration>,
+/// Configuration,
+/// > = kernel::configfs::ItemType::<
+/// Subsystem<Configuration>,
+/// Configuration,
+/// >::new_with_child_ctor::<3usize, Child>(&THIS_MODULE, &CONFIGURATION_ATTR_LIST);
+/// &CONFIGURATION_TPE
+/// }
+/// };
+///
+#[cfg(CONFIG_CONFIGFS_FS)]
+#[proc_macro]
+pub fn configfs_attrs(input: TokenStream) -> TokenStream {
+ configfs_attrs::configfs_attrs(parse_macro_input!(input as configfs_attrs::ConfigfsAttrs))
+ .into()
+}
diff --git a/samples/rust/rust_configfs.rs b/samples/rust/rust_configfs.rs
index a1bd9db6010da..876462f7789d1 100644
--- a/samples/rust/rust_configfs.rs
+++ b/samples/rust/rust_configfs.rs
@@ -4,7 +4,7 @@
use kernel::alloc::flags;
use kernel::configfs;
-use kernel::configfs::configfs_attrs;
+use kernel::macros::configfs_attrs;
use kernel::new_mutex;
use kernel::page::PAGE_SIZE;
use kernel::prelude::*;
---
base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
change-id: 20260417-configfs-syn-191e07130027
Best regards,
--
Malte Wechter <maltewechter@gmail.com>
^ permalink raw reply related
* Re: [PATCH RFC 5/5] block, nvme: add failed_bio callback for multipath bio failover
From: Christoph Hellwig @ 2026-05-20 7:27 UTC (permalink / raw)
To: Keith Busch
Cc: linux-block, linux-nvme, axboe, hch, tom.leiming, coshi036,
Igor.Achkinazi, dlemoal, Keith Busch
In-Reply-To: <20260519172326.3462354-6-kbusch@meta.com>
On Tue, May 19, 2026 at 10:23:26AM -0700, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
>
> The nvme driver has long utilized a zero capacity to indicate the path
> isn't reachable, which creates a race condition with IO dispatch when
> paths are being detached on a live system: when the block layer rejects
> a bio early due to a capacity check failure, drivers with multipath
> support using the original bio have no interception point to redirect
> the bio to another path.
Trying to reverse-engineer - the problem is that the block-layer
code catches being beyond the capacity and directly completes the bio,
right?
IMHO the right fix is to get rid of the capacity hacks, and have a flag
we can catch in the nvme driver and complete through the mechanisms.
Having a callback into the driver for a specific error codition seems
odd. And I think there's a bunch of cases where we could call this on
a bio the driver hasn't even seen yet, including from file system code.
^ permalink raw reply
* Re: [PATCH RFC 4/5] block: move bio validation into __bio_split_to_limits
From: Christoph Hellwig @ 2026-05-20 7:25 UTC (permalink / raw)
To: Keith Busch
Cc: linux-block, linux-nvme, axboe, hch, tom.leiming, coshi036,
Igor.Achkinazi, dlemoal, Keith Busch
In-Reply-To: <20260519172326.3462354-5-kbusch@meta.com>
Replying here, but at least the high-level issue applies to the
previous patch as well.
Moving the checking from submit_bio_noacct into __bio_split_to_limits
means that we lose it for bio-based drivers except for the few that
manuall call bio_split_to_limits. So we'll still need a helper for
them, which could be called just before calling into ->submit_bio.
> -static inline blk_status_t blk_check_zone_append(struct request_queue *q,
> - struct bio *bio)
> -{
Keeping a helper for this instead of doing a lot of work in a switch
statements feels nicer.
> -static blk_status_t blk_validate_atomic_write_op_size(struct request_queue *q,
> - struct bio *bio)
> -{
> - if (bio->bi_iter.bi_size > queue_atomic_write_unit_max_bytes(q))
> - return BLK_STS_INVAL;
> -
> - if (bio->bi_iter.bi_size % queue_atomic_write_unit_min_bytes(q))
> - return BLK_STS_INVAL;
> -
> - return BLK_STS_OK;
> -}
Similar for this one.
^ permalink raw reply
* Re: [PATCH RFC 3/5] block: validate bio bounds in the queue entered context
From: Damien Le Moal @ 2026-05-20 7:25 UTC (permalink / raw)
To: Keith Busch, linux-block, linux-nvme
Cc: axboe, hch, tom.leiming, coshi036, Igor.Achkinazi, Keith Busch
In-Reply-To: <03564da7-661e-40ea-9a11-8b96972f1048@kernel.org>
On 2026/05/20 9:22, Damien Le Moal wrote:
> On 2026/05/19 19:23, Keith Busch wrote:
>> From: Keith Busch <kbusch@kernel.org>
>>
>> bio_check_eod() in submit_bio_noacct() validates that a bio does not
>> extend beyond the partition's available sectors. This check runs before
>> bio_queue_enter(), so it is not serialized against queue limit updates.
>> A driver that freezes the queue, updates limits, changes the capacity,
>> and unfreezes can race with a bio that passed the early check under the
>> old capacity.
>>
>> Remove bio_check_eod() and replace it with a bounds check in
>> __bio_split_to_limits(), which runs after the queue usage reference has
>> been acquired. The check uses partition-aware arithmetic to validate
>> both partition bounds and disk capacity in a single comparison that
>> works correctly on the post-remap sector values.
>>
>> Signed-off-by: Keith Busch <kbusch@kernel.org>
>
> [...]
>
>> @@ -423,6 +423,17 @@ static inline bool bio_may_need_split(struct bio *bio,
>> static inline struct bio *__bio_split_to_limits(struct bio *bio,
>> const struct queue_limits *lim, unsigned int *nr_segs)
>> {
>> + if (unlikely(bio_end_sector(bio) > bdev_nr_sectors(bio->bi_bdev) +
>> + bio->bi_bdev->bd_start_sect)) {
>> + pr_info_ratelimited("%s: attempt to access beyond end of device\n"
>> + "%pg: rw=%d, sector=%llu, nr_sectors = %u limit=%llu\n",
>> + current->comm, bio->bi_bdev, bio->bi_opf,
>> + bio->bi_iter.bi_sector, bio_sectors(bio),
>> + bdev_nr_sectors(bio->bi_bdev) +
>> + bio->bi_bdev->bd_start_sect);
>
> Should this be a pr_err_ratelimited() ?
>
> Also, putting this check here means that we are going to redo the check for all
> the fragment of the large BIO being split, no ? It would be nice to be able to
> do this check only once when the large BIO is submitted.
>
> Moving this check to a helper and calling this new helper higher in the
> submission path could avoid that. But I am not 100% sure if a higher placed call
> can be a problem.
Replying to myself: We do not yet have called queue enter higher up. So moving
this check defeats your goal. Please ignore this comment :)
>> + goto ioerr;
>> + }
>> +
>> switch (bio_op(bio)) {
>> case REQ_OP_READ:
>> case REQ_OP_WRITE:
>> @@ -442,6 +453,9 @@ static inline struct bio *__bio_split_to_limits(struct bio *bio,
>> *nr_segs = 0;
>> return bio;
>> }
>> +ioerr:
>> + bio_io_error(bio);
>> + return NULL;
>> }
>>
>> /**
>
>
--
Damien Le Moal
Western Digital Research
^ permalink raw reply
* Re: [PATCH RFC 2/5] block: fix invalid zone append status
From: Christoph Hellwig @ 2026-05-20 7:23 UTC (permalink / raw)
To: Keith Busch
Cc: linux-block, linux-nvme, axboe, hch, tom.leiming, coshi036,
Igor.Achkinazi, dlemoal, Keith Busch
In-Reply-To: <20260519172326.3462354-3-kbusch@meta.com>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH RFC 1/5] blk-mq: fix status for unaligned bio
From: Christoph Hellwig @ 2026-05-20 7:22 UTC (permalink / raw)
To: Keith Busch
Cc: linux-block, linux-nvme, axboe, hch, tom.leiming, coshi036,
Igor.Achkinazi, dlemoal, Keith Busch
In-Reply-To: <20260519172326.3462354-2-kbusch@meta.com>
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
^ permalink raw reply
* Re: [PATCH RFC 3/5] block: validate bio bounds in the queue entered context
From: Damien Le Moal @ 2026-05-20 7:22 UTC (permalink / raw)
To: Keith Busch, linux-block, linux-nvme
Cc: axboe, hch, tom.leiming, coshi036, Igor.Achkinazi, Keith Busch
In-Reply-To: <20260519172326.3462354-4-kbusch@meta.com>
On 2026/05/19 19:23, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
>
> bio_check_eod() in submit_bio_noacct() validates that a bio does not
> extend beyond the partition's available sectors. This check runs before
> bio_queue_enter(), so it is not serialized against queue limit updates.
> A driver that freezes the queue, updates limits, changes the capacity,
> and unfreezes can race with a bio that passed the early check under the
> old capacity.
>
> Remove bio_check_eod() and replace it with a bounds check in
> __bio_split_to_limits(), which runs after the queue usage reference has
> been acquired. The check uses partition-aware arithmetic to validate
> both partition bounds and disk capacity in a single comparison that
> works correctly on the post-remap sector values.
>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
[...]
> @@ -423,6 +423,17 @@ static inline bool bio_may_need_split(struct bio *bio,
> static inline struct bio *__bio_split_to_limits(struct bio *bio,
> const struct queue_limits *lim, unsigned int *nr_segs)
> {
> + if (unlikely(bio_end_sector(bio) > bdev_nr_sectors(bio->bi_bdev) +
> + bio->bi_bdev->bd_start_sect)) {
> + pr_info_ratelimited("%s: attempt to access beyond end of device\n"
> + "%pg: rw=%d, sector=%llu, nr_sectors = %u limit=%llu\n",
> + current->comm, bio->bi_bdev, bio->bi_opf,
> + bio->bi_iter.bi_sector, bio_sectors(bio),
> + bdev_nr_sectors(bio->bi_bdev) +
> + bio->bi_bdev->bd_start_sect);
Should this be a pr_err_ratelimited() ?
Also, putting this check here means that we are going to redo the check for all
the fragment of the large BIO being split, no ? It would be nice to be able to
do this check only once when the large BIO is submitted.
Moving this check to a helper and calling this new helper higher in the
submission path could avoid that. But I am not 100% sure if a higher placed call
can be a problem.
> + goto ioerr;
> + }
> +
> switch (bio_op(bio)) {
> case REQ_OP_READ:
> case REQ_OP_WRITE:
> @@ -442,6 +453,9 @@ static inline struct bio *__bio_split_to_limits(struct bio *bio,
> *nr_segs = 0;
> return bio;
> }
> +ioerr:
> + bio_io_error(bio);
> + return NULL;
> }
>
> /**
--
Damien Le Moal
Western Digital Research
^ permalink raw reply
* Re: [PATCH RFC 2/5] block: fix invalid zone append status
From: Damien Le Moal @ 2026-05-20 6:58 UTC (permalink / raw)
To: Keith Busch, linux-block, linux-nvme
Cc: axboe, hch, tom.leiming, coshi036, Igor.Achkinazi, Keith Busch
In-Reply-To: <20260519172326.3462354-3-kbusch@meta.com>
On 2026/05/19 19:23, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
>
> A bio submitted to a zone block device that breaks the limits is invalid
> rather than an IO error. Fix up the return code to report it that way,
> as the previously used IOERR is considered a path failure.
>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
Looks good.
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Same comment as the previous patch. This could go in regardless of the other
fixes I think.
--
Damien Le Moal
Western Digital Research
^ permalink raw reply
* Re: [PATCH RFC 1/5] blk-mq: fix status for unaligned bio
From: Damien Le Moal @ 2026-05-20 6:56 UTC (permalink / raw)
To: Keith Busch, linux-block, linux-nvme
Cc: axboe, hch, tom.leiming, coshi036, Igor.Achkinazi, Keith Busch
In-Reply-To: <20260519172326.3462354-2-kbusch@meta.com>
On 2026/05/19 19:23, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
>
> A bio requesting sectors unaligned to the logical format is invalid
> rather than an IO error. Fix up the return code because there are some
> device mappers that care about distinguishing these kinds of errors.
>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
Looks good to me.
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
It seems that this one could go in regardless of this series (so I do not think
it needs to be RFC).
--
Damien Le Moal
Western Digital Research
^ permalink raw reply
* Re: [PATCH v2] loop: Fix NULL pointer dereference by synchronizing lo_release and loop_queue_rq
From: Tetsuo Handa @ 2026-05-20 6:36 UTC (permalink / raw)
To: Ming Lei
Cc: Bart Van Assche, Jens Axboe, Christoph Hellwig, Damien Le Moal,
linux-block, LKML, Andrew Morton
In-Reply-To: <ag0lS_CbKO9R5CV8@fedora>
On 2026/05/20 12:06, Ming Lei wrote:
> The IO after close(loop) should be from writeback. rcu/sruc isn't necessary,
Gemini's comment is that drain_workqueue() is not sufficient for waiting for
do_req_filebacked(REQ_OP_WRITE) requests with cmd->use_aio == true case to complete.
We could remove synchronize_rcu() prior to drain_workqueue() if we defer
lo->lo_state != Lo_bound check to workqueue context (or recheck in workqueue context).
But I still think that we need to guarantee that all "cmd->use_aio == true" requests (including
ones which had been issued before hitting "WRITE_ONCE(lo->lo_state, Lo_rundown);") have
completed before doing "lo->lo_backing_file = NULL;".
And I don't know whether it is safe to use
"blk_mq_unfreeze_queue(lo->lo_queue, blk_mq_freeze_queue(lo->lo_queue));"
immediately after drain_workqueue() because we are holding disk->open_mutex.
^ permalink raw reply
* [PATCH] blk-throttle: schedule parent dispatch in tg_flush_bios()
From: Tao Cui @ 2026-05-20 6:24 UTC (permalink / raw)
To: tj, josef, axboe, cgroups; +Cc: linux-block, Tao Cui
tg_flush_bios() schedules pending_timer on the child tg's own
service_queue, which causes throtl_pending_timer_fn() to dispatch from
the child's pending_tree. For leaf cgroups this tree is empty, so the
timer fires and exits without dispatching the throttled bio.
The throttled bio sits in the parent's pending_tree with disptime set
to jiffies (THROTL_TG_CANCELING zeroes all dispatch times), but the
parent's timer is never explicitly rescheduled. The bio only gets
dispatched when the parent timer eventually fires at its previously
scheduled expiry.
Fix by calling throtl_schedule_next_dispatch(sq->parent_sq, true)
instead, matching what tg_set_limit() already does. This forces the
parent's dispatch cycle to run immediately and flush all canceling
bios without waiting for a stale timer.
Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
block/blk-throttle.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/blk-throttle.c b/block/blk-throttle.c
index 2d39977ba9de..0ad30b688ce1 100644
--- a/block/blk-throttle.c
+++ b/block/blk-throttle.c
@@ -1652,7 +1652,7 @@ static void tg_flush_bios(struct throtl_grp *tg)
*/
tg_update_disptime(tg);
- throtl_schedule_pending_timer(sq, jiffies + 1);
+ throtl_schedule_next_dispatch(sq->parent_sq, true);
}
static void throtl_pd_offline(struct blkg_policy_data *pd)
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] block/loop: Fix NULL pointer dereference in lo_rw_aio()
From: Hongling Zeng @ 2026-05-20 3:20 UTC (permalink / raw)
To: Tetsuo Handa, Ming Lei
Cc: Hongling Zeng, axboe, ming.lei, linux-block, linux-kernel
In-Reply-To: <8f5602c6-52c6-4c96-9c85-892160b1718b@I-love.SAKURA.ne.jp>
Thanks, I'll review Tetsuo's analysis to better understand
the full scope of the issue.
在 2026年05月19日 20:37, Tetsuo Handa 写道:
> On 2026/05/19 20:28, Ming Lei wrote:
>>> This means |__loop_clr_fd()|runs while I/O is still active.
>>> Regression introduced by:
>>> 6050fa4c84cc ("loop: don't hold lo_mutex during __loop_clr_fd()")
>> Why do you conclude it is caused by above commit?
> Some commit in the merge window for 7.1 broke the loop driver.
> Even synchronize_rcu() + drain_workqueue(lo->workqueue) is not sufficient.
> Please see a thread at https://lkml.kernel.org/r/d43125ff-cc66-49b7-b16d-1b2650c68c23@I-love.SAKURA.ne.jp .
^ permalink raw reply
* Re: [PATCH v2] loop: Fix NULL pointer dereference by synchronizing lo_release and loop_queue_rq
From: Ming Lei @ 2026-05-20 3:06 UTC (permalink / raw)
To: Tetsuo Handa
Cc: Bart Van Assche, Jens Axboe, Christoph Hellwig, Damien Le Moal,
linux-block, LKML, Andrew Morton
In-Reply-To: <d43125ff-cc66-49b7-b16d-1b2650c68c23@I-love.SAKURA.ne.jp>
On Tue, May 19, 2026 at 06:27:11PM +0900, Tetsuo Handa wrote:
> On 2026/05/19 9:40, Andrew Morton wrote:
> > AI review asked a couple of questions:
> > https://sashiko.dev/#/patchset/9b2032d6-3f36-4d2b-8128-985c08a4fa37@I-love.SAKURA.ne.jp
>
> To: gemini/gemini-3.1-pro-preview
>
> Thank you for your valuable feedback. Your point about asynchronous I/O completing after drain_workqueue()
> and potentially causing a UAF at file_inode() from kiocb_end_write() from lo_rw_aio_do_completion() is correct.
> The drain_workqueue() alone does not wait for in-flight AIOs that have already returned -EIOCBQUEUED. However,
> I'm not convinced that use of blk_mq_freeze_queue() inside __loop_clr_fd() where disk->open_mutex was already
> held by bdev_release() is absolutely deadlock-free.
>
> 1. VFS and Block Layer Lock Contention:
> __loop_clr_fd() is exclusively invoked from the lo_release() path during the final close of the device.
> At this stage, the block layer is holding disk->open_mutex. If we call blk_mq_freeze_queue() here, it will
> synchronously wait for all in-flight AIOs to complete. However, the completion paths of those in-flight AIOs
> (or subsequent metadata processing in the underlying filesystem) may attempt to acquire resources or execute
> code paths that depend on the very same device state or open/close status. This creates a circular dependency,
> leading to an unrecoverable hang.
>
> 2. Memory Reclaim Deadlock:
> blk_mq_freeze_queue() blocks until the queue's usage counter drops to zero. If an in-flight AIO requires memory
> allocation for metadata updates upon completion, and the system is under heavy memory pressure, it can trigger
> direct memory reclaim. If the reclaim path attempts to sync other buffers or interact with the frozen loop
> device/queue, a circular deadlock occurs.
>
> Therefore, I would like to choose SRCU-based synchronization instead of blk_mq_freeze_queue().
>
> * Locking: We call srcu_read_lock(&loop_io_srcu) only for asynchronous paths (cmd->use_aio) immediately
> before submitting the I/O to the underlying filesystem in lo_rw_aio().
>
> * Unlocking: The reader lock is released via srcu_read_unlock() at the very end of the AIO completion handler
> (lo_rw_aio_do_completion()).
>
> * Synchronization: We place synchronize_srcu(&loop_io_srcu) immediately after drain_workqueue() in __loop_clr_fd().
>
> I think that this guarantees that __loop_clr_fd() safely blocks until all pending AIO callbacks are 100% completed,
> fully eliminating the UAF risk and ensuring the safety of the subsequent mapping_set_gfp_mask() and fput(), while
> remaining entirely deadlock-free.
>
> What do you think about this approach?
>
> drivers/block/loop.c | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index 0000913f7efc..7c3961f3cbc9 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -80,6 +80,7 @@ struct loop_cmd {
> struct list_head list_entry;
> bool use_aio; /* use AIO interface to handle I/O */
> atomic_t ref; /* only for aio */
> + int srcu_idx;
> long ret;
> struct kiocb iocb;
> struct bio_vec *bvec;
> @@ -93,6 +94,7 @@ struct loop_cmd {
> static DEFINE_IDR(loop_index_idr);
> static DEFINE_MUTEX(loop_ctl_mutex);
> static DEFINE_MUTEX(loop_validate_mutex);
> +DEFINE_SRCU(loop_io_srcu);
>
> /**
> * loop_global_lock_killable() - take locks for safe loop_validate_file() test
> @@ -327,6 +329,8 @@ static void lo_rw_aio_do_completion(struct loop_cmd *cmd)
> kiocb_end_write(&cmd->iocb);
> if (likely(!blk_should_fake_timeout(rq->q)))
> blk_mq_complete_request(rq);
> + if (cmd->use_aio)
> + srcu_read_unlock(&loop_io_srcu, cmd->srcu_idx);
> }
>
> static void lo_rw_aio_complete(struct kiocb *iocb, long ret)
> @@ -392,6 +396,7 @@ static int lo_rw_aio(struct loop_device *lo, struct loop_cmd *cmd,
> if (cmd->use_aio) {
> cmd->iocb.ki_complete = lo_rw_aio_complete;
> cmd->iocb.ki_flags = IOCB_DIRECT;
> + cmd->srcu_idx = srcu_read_lock(&loop_io_srcu);
> } else {
> cmd->iocb.ki_complete = NULL;
> cmd->iocb.ki_flags = 0;
> @@ -1118,6 +1123,22 @@ static void __loop_clr_fd(struct loop_device *lo)
> struct file *filp;
> gfp_t gfp = lo->old_gfp_mask;
>
> + /*
> + * Now that loop_queue_rq() sees lo->lo_state != Lo_bound,
> + * wait for already started loop_queue_rq() to complete.
> + */
> + synchronize_rcu();
> + /*
> + * Now that no more works are scheduled by loop_queue_rq(),
> + * wait for already scheduled works to complete.
> + */
> + drain_workqueue(lo->workqueue);
> + /*
> + * Now that no more AIO requests are scheduled by lo_rw_aio(),
> + * wait for already started AIO to complete.
> + */
> + synchronize_srcu(&loop_io_srcu);
The IO after close(loop) should be from writeback. rcu/sruc isn't necessary,
please see the patch posted in another thread:
https://lore.kernel.org/linux-block/agxJdUf1b0JSDAux@fedora/
in which the check on lo->lo_state is moved to loop_handle_cmd(), meantime
drain_workqueue() is added for draining in-flight workers.
Thanks,
Ming
^ permalink raw reply
page: next (older) | 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