* [PATCH RFC 4/5] block: move bio validation into __bio_split_to_limits
From: Keith Busch @ 2026-05-19 17:23 UTC (permalink / raw)
To: linux-block, linux-nvme
Cc: axboe, hch, tom.leiming, coshi036, Igor.Achkinazi, dlemoal,
Keith Busch
In-Reply-To: <20260519172326.3462354-1-kbusch@meta.com>
From: Keith Busch <kbusch@kernel.org>
The bio checks in submit_bio_noacct() compares queue limits to determine
whether operations like discard, write zeroes, zone append, and atomic
writes are supported and valid. These checks run before
bio_queue_enter(), so they race against any driver that updates queue
limits inside a freeze window.
Move all limit-dependent operation validation from submit_bio_noacct()
into __bio_split_to_limits(), which runs after the queue usage reference
has been acquired. This ensures that all checks are properly serialized
against limit updates.
The non-limit checks (crypto, fault injection, partition remap, and
flush flag handling) remain in submit_bio_noacct() as they do not
depend on queue limits.
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
block/blk-core.c | 118 -----------------------------------------------
block/blk.h | 75 ++++++++++++++++++++++++++++--
2 files changed, 72 insertions(+), 121 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index c200d0fc44fe7..8360c2b5efee5 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -519,25 +519,6 @@ static int __init fail_make_request_debugfs(void)
late_initcall(fail_make_request_debugfs);
#endif /* CONFIG_FAIL_MAKE_REQUEST */
-static inline void bio_check_ro(struct bio *bio)
-{
- if (op_is_write(bio_op(bio)) && bdev_read_only(bio->bi_bdev)) {
- if (op_is_flush(bio->bi_opf) && !bio_sectors(bio))
- return;
-
- if (bdev_test_flag(bio->bi_bdev, BD_RO_WARNED))
- return;
-
- bdev_set_flag(bio->bi_bdev, BD_RO_WARNED);
-
- /*
- * Use ioctl to set underlying disk of raid/dm to read-only
- * will trigger this.
- */
- pr_warn("Trying to write to read-only block-device %pg\n",
- bio->bi_bdev);
- }
-}
int should_fail_bio(struct bio *bio)
{
@@ -566,39 +547,6 @@ static int blk_partition_remap(struct bio *bio)
return 0;
}
-/*
- * Check write append to a zoned block device.
- */
-static inline blk_status_t blk_check_zone_append(struct request_queue *q,
- struct bio *bio)
-{
- int nr_sectors = bio_sectors(bio);
-
- /* Only applicable to zoned block devices */
- if (!bdev_is_zoned(bio->bi_bdev))
- return BLK_STS_NOTSUPP;
-
- /* The bio sector must point to the start of a sequential zone */
- if (!bdev_is_zone_start(bio->bi_bdev, bio->bi_iter.bi_sector))
- return BLK_STS_INVAL;
-
- /*
- * Not allowed to cross zone boundaries. Otherwise, the BIO will be
- * split and could result in non-contiguous sectors being written in
- * different zones.
- */
- if (nr_sectors > q->limits.chunk_sectors)
- return BLK_STS_INVAL;
-
- /* Make sure the BIO is small enough and will not get split */
- if (nr_sectors > q->limits.max_zone_append_sectors)
- return BLK_STS_INVAL;
-
- bio->bi_opf |= REQ_NOMERGE;
-
- return BLK_STS_OK;
-}
-
static void __submit_bio(struct bio *bio)
{
/* If plug is not used, add new plug here to cache nsecs time. */
@@ -731,18 +679,6 @@ void submit_bio_noacct_nocheck(struct bio *bio, bool split)
}
}
-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;
-}
-
/**
* submit_bio_noacct - re-submit a bio to the block device layer for I/O
* @bio: The bio describing the location in memory and on the device.
@@ -755,7 +691,6 @@ static blk_status_t blk_validate_atomic_write_op_size(struct request_queue *q,
void submit_bio_noacct(struct bio *bio)
{
struct block_device *bdev = bio->bi_bdev;
- struct request_queue *q = bdev_get_queue(bdev);
blk_status_t status = BLK_STS_IOERR;
might_sleep();
@@ -776,7 +711,6 @@ void submit_bio_noacct(struct bio *bio)
if (should_fail_bio(bio))
goto end_io;
- bio_check_ro(bio);
if (!bio_flagged(bio, BIO_REMAPPED)) {
if (bdev_is_partition(bdev) &&
unlikely(blk_partition_remap(bio)))
@@ -800,58 +734,6 @@ void submit_bio_noacct(struct bio *bio)
}
}
- switch (bio_op(bio)) {
- case REQ_OP_READ:
- break;
- case REQ_OP_WRITE:
- if (bio->bi_opf & REQ_ATOMIC) {
- status = blk_validate_atomic_write_op_size(q, bio);
- if (status != BLK_STS_OK)
- goto end_io;
- }
- break;
- case REQ_OP_FLUSH:
- /*
- * REQ_OP_FLUSH can't be submitted through bios, it is only
- * synthetized in struct request by the flush state machine.
- */
- goto not_supported;
- case REQ_OP_DISCARD:
- if (!bdev_max_discard_sectors(bdev))
- goto not_supported;
- break;
- case REQ_OP_SECURE_ERASE:
- if (!bdev_max_secure_erase_sectors(bdev))
- goto not_supported;
- break;
- case REQ_OP_ZONE_APPEND:
- status = blk_check_zone_append(q, bio);
- if (status != BLK_STS_OK)
- goto end_io;
- break;
- case REQ_OP_WRITE_ZEROES:
- if (!q->limits.max_write_zeroes_sectors)
- goto not_supported;
- break;
- case REQ_OP_ZONE_RESET:
- case REQ_OP_ZONE_OPEN:
- case REQ_OP_ZONE_CLOSE:
- case REQ_OP_ZONE_FINISH:
- case REQ_OP_ZONE_RESET_ALL:
- if (!bdev_is_zoned(bio->bi_bdev))
- goto not_supported;
- break;
- case REQ_OP_DRV_IN:
- case REQ_OP_DRV_OUT:
- /*
- * Driver private operations are only used with passthrough
- * requests.
- */
- fallthrough;
- default:
- goto not_supported;
- }
-
if (blk_throtl_bio(bio))
return;
submit_bio_noacct_nocheck(bio, false);
diff --git a/block/blk.h b/block/blk.h
index e70acb2d358e3..d3b897e9b5ee9 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -407,6 +407,22 @@ static inline bool bio_may_need_split(struct bio *bio,
return bv->bv_len + bv->bv_offset > lim->max_fast_segment_size;
}
+static inline void bio_check_ro(struct bio *bio)
+{
+ if (op_is_write(bio_op(bio)) && bdev_read_only(bio->bi_bdev)) {
+ if (op_is_flush(bio->bi_opf) && !bio_sectors(bio))
+ return;
+
+ if (bdev_test_flag(bio->bi_bdev, BD_RO_WARNED))
+ return;
+
+ bdev_set_flag(bio->bi_bdev, BD_RO_WARNED);
+
+ pr_warn("Trying to write to read-only block-device %pg\n",
+ bio->bi_bdev);
+ }
+}
+
/**
* __bio_split_to_limits - split a bio to fit the queue limits
* @bio: bio to be split
@@ -423,6 +439,8 @@ 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)
{
+ bio_check_ro(bio);
+
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"
@@ -435,24 +453,75 @@ static inline struct bio *__bio_split_to_limits(struct bio *bio,
}
switch (bio_op(bio)) {
- case REQ_OP_READ:
case REQ_OP_WRITE:
+ if (bio->bi_opf & REQ_ATOMIC) {
+ if (bio->bi_iter.bi_size > lim->atomic_write_unit_max ||
+ bio->bi_iter.bi_size % lim->atomic_write_unit_min)
+ goto invalid;
+ }
+ fallthrough;
+ case REQ_OP_READ:
if (bio_may_need_split(bio, lim))
return bio_split_rw(bio, lim, nr_segs);
*nr_segs = 1;
return bio;
case REQ_OP_ZONE_APPEND:
+ /* Only applicable to zoned block devices */
+ if (!(lim->features & BLK_FEAT_ZONED))
+ goto not_supported;
+
+ /* The bio sector must point to the start of a sequential zone */
+ if (!bdev_is_zone_start(bio->bi_bdev, bio->bi_iter.bi_sector))
+ goto invalid;
+
+ /*
+ * Not allowed to cross zone boundaries. Otherwise, the BIO
+ * will be split and could result in non-contiguous sectors
+ * being written in different zones.
+ */
+ if (bio_sectors(bio) > lim->chunk_sectors)
+ goto invalid;
+
+ /* Make sure the BIO is small enough and will not get split */
+ if (bio_sectors(bio) > lim->max_zone_append_sectors)
+ goto invalid;
+
+ bio->bi_opf |= REQ_NOMERGE;
return bio_split_zone_append(bio, lim, nr_segs);
case REQ_OP_DISCARD:
+ if (!lim->max_discard_sectors)
+ goto not_supported;
+ return bio_split_discard(bio, lim, nr_segs);
case REQ_OP_SECURE_ERASE:
+ if (!lim->max_secure_erase_sectors)
+ goto not_supported;
return bio_split_discard(bio, lim, nr_segs);
case REQ_OP_WRITE_ZEROES:
+ if (!lim->max_write_zeroes_sectors)
+ goto not_supported;
return bio_split_write_zeroes(bio, lim, nr_segs);
- default:
- /* other operations can't be split */
+ case REQ_OP_ZONE_RESET:
+ case REQ_OP_ZONE_OPEN:
+ case REQ_OP_ZONE_CLOSE:
+ case REQ_OP_ZONE_FINISH:
+ case REQ_OP_ZONE_RESET_ALL:
+ if (!(lim->features & BLK_FEAT_ZONED))
+ goto not_supported;
*nr_segs = 0;
return bio;
+ default:
+ WARN_ON_ONCE(1);
+ goto not_supported;
}
+
+invalid:
+ bio->bi_status = BLK_STS_INVAL;
+ bio_endio(bio);
+ return NULL;
+not_supported:
+ bio->bi_status = BLK_STS_NOTSUPP;
+ bio_endio(bio);
+ return NULL;
ioerr:
bio_io_error(bio);
return NULL;
--
2.53.0-Meta
^ permalink raw reply related
* Re: [RFC PATCH v2 0/4] mm/zsmalloc: reduce zs_free() latency on swap release path
From: Nhat Pham @ 2026-05-19 15:22 UTC (permalink / raw)
To: Wenchao Hao
Cc: Yosry Ahmed, Andrew Morton, Chengming Zhou, Jens Axboe,
Johannes Weiner, Minchan Kim, Sergey Senozhatsky, linux-block,
linux-kernel, linux-mm, Barry Song, Xueyuan Chen, Wenchao Hao
In-Reply-To: <CAOptpSMdFwQUxT7bP7s1GWyXvzUQrg3ROtGp3w_zYy2rSkDYRQ@mail.gmail.com>
On Sat, May 9, 2026 at 2:08 AM Wenchao Hao <haowenchao22@gmail.com> wrote:
>
> On Sat, May 9, 2026 at 7:33 AM Nhat Pham <nphamcs@gmail.com> wrote:
> >
> > On Wed, May 6, 2026 at 6:55 AM Wenchao Hao <haowenchao22@gmail.com> wrote:
> > >
> > > On Sat, May 2, 2026 at 3:21 PM Nhat Pham <nphamcs@gmail.com> wrote:
> > > >
> > > >
> > > > Oh man, you are eliminating pool lock here right? This would help my
> > > > other patch series a lot too :)
> > > >
> > > > https://lore.kernel.org/all/CAKEwX=M5YpR0cQrryX_y4pm_BuxyUWZ_8MbhWodwbf1Fe=gzew@mail.gmail.com/
> > > > https://lore.kernel.org/all/CAKEwX=PkFiP+u+ThrzjTKBi+usQf2uuhTZcfB2BNNA8RboOFDQ@mail.gmail.com/
> > > >
> > >
> > > Yes, exactly. With class_idx encoded in the obj value,
> > > zs_free() can determine the correct size_class without
> > > any pool-level lock. The lockless read gives a valid
> > > class_idx because it's invariant across migration (only
> > > PFN changes), and we re-read obj under class->lock to
> > > get a stable PFN.
> > >
> > > >
> > > > /*
> > > > * The pool->lock protects the race with zpage's migration
> > > > * so it's safe to get the page from handle.
> > > > */
> > > > read_lock(&pool->lock);
> > > > obj = handle_to_obj(handle);
> > > > obj_to_zpdesc(obj, &f_zpdesc);
> > > > zspage = get_zspage(f_zpdesc);
> > > > class = zspage_class(pool, zspage);
> > > > spin_lock(&class->lock);
> > > > read_unlock(&pool->lock);
> > > >
> > > > It's basically just this blob right?
> > > >
> > >
> > > Yes, that's the blob being replaced. On the
> > > ZS_OBJ_CLASS_IDX path (64-bit systems), it becomes:
> > >
> > > obj = handle_to_obj(handle);
> > > class = pool->size_class[obj_to_class_idx(obj)];
> > > spin_lock(&class->lock);
> > > obj = handle_to_obj(handle); /* re-read for stable PFN */
> > >
> > > No pool->lock at all. We've also added compile-time
> > > gating (#if BITS_PER_LONG >= 64) since 32-bit systems
> > > lack the spare bits in OBJ_INDEX to fit class_idx. On
> > > 32-bit, it falls back to the original pool->lock path.
> > >
> >
> > BTW, I've tested your idea with a hacky prototype, when I was playing
> > with my vswap series. It absolutely improves free time in the usemem
> > benchmark :) Idea is very promising - I won't scoop your work of
> > course, just letting you know that at least in my use case, it works
> > :) Look forward to seeing it submitted soon!!!
>
> Thanks, Nhat, that's great to hear.
>
> I've split this part out and posted it as its own series:
>
> https://lore.kernel.org/linux-mm/20260508061910.3882831-1-haowenchao@xiaomi.com
>
> Review there would be very welcome.
Huh I think I might have been unsubscribed from linux-mm again -.-
Weird - I wonder if this is because of Gmail shenanigans.
Can you cc me the thread next time just in case?
>
> Also, could you share the details of your usemem setup? I'd like
> to reproduce it locally on the same baseline.
Sure! I left some notes here:
https://lore.kernel.org/all/20260505153854.1612033-1-nphamcs@gmail.com/
But for your convenience, this is the benchmark I ran:
2. Usemem single-threaded: anonymous memory allocation (56GB) on a host
with 32GB RAM, 16 rounds.
I don't put a limit on the cgroup, relying on global pressure (per
Kairui's instructions).
I'm not on my work server right now so I don't have the exact command,
but hopefully that should be enough to show the wins with your patch
series! I wanted to run it for your patch series myself but I do not
have the cycles right now, unfortunately :(
>
> Thanks,
> Wenchao
^ permalink raw reply
* Re: 10x I/O await times in 6.12
From: Matt Fleming @ 2026-05-19 13:31 UTC (permalink / raw)
To: Jens Axboe; +Cc: Yu Kuai, linux-block, linux-kernel, kernel-team, yukuai (C)
In-Reply-To: <aepfV6Gp2uKbHi2N@matt-Precision-5490>
On Thu, Apr 23, 2026 at 07:18:39PM +0100, Matt Fleming wrote:
> On Mon, Apr 21, 2025 at 08:21:13PM +0800, Yu Kuai wrote:
> > If nsecs - plug->cur_ktime is still milliseconds, can you check if
> > the following patch can fix your problem?
> >
> > Thanks,
> > Kuai
> >
> > diff --git a/block/blk-mq.c b/block/blk-mq.c
> > index ae8494d88897..37197502147e 100644
> > --- a/block/blk-mq.c
> > +++ b/block/blk-mq.c
> > @@ -1095,7 +1095,9 @@ static inline void blk_account_io_start(struct request
> > *req)
> > return;
> >
> > req->rq_flags |= RQF_IO_STAT;
> > - req->start_time_ns = blk_time_get_ns();
> > +
> > + if (!current->plug)
> > + req->start_time_ns = blk_time_get_ns();
> >
> > /*
> > * All non-passthrough requests are created from a bio with one
> > @@ -2874,6 +2876,7 @@ void blk_mq_flush_plug_list(struct blk_plug *plug,
> > bool from_schedule)
> > {
> > struct request *rq;
> > unsigned int depth;
> > + u64 now;
> >
> > /*
> > * We may have been called recursively midway through handling
> > @@ -2887,6 +2890,10 @@ void blk_mq_flush_plug_list(struct blk_plug *plug,
> > bool from_schedule)
> > depth = plug->rq_count;
> > plug->rq_count = 0;
> >
> > + now = ktime_get_ns();
> > + rq_list_for_each(&plug->mq_list, rq)
> > + rq->start_time_ns = now;
> > +
> > if (!plug->multiple_queues && !plug->has_elevator && !from_schedule)
> > {
> > struct request_queue *q;
> >
>
> It's April again, and this time we're moving to 6.18.
>
> The I/O accounting bug reported at the start of this thread is still an
> issue for us, and we've been carrying the above patch for 12 months.
Hey Jens, any feedback on this patch?
^ permalink raw reply
* Re: [PATCH] block/loop: Fix NULL pointer dereference in lo_rw_aio()
From: Tetsuo Handa @ 2026-05-19 12:37 UTC (permalink / raw)
To: Ming Lei, Hongling Zeng
Cc: Hongling Zeng, axboe, ming.lei, linux-block, linux-kernel
In-Reply-To: <agxJdUf1b0JSDAux@fedora>
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
* [PATCH 4/4] dm crypt: batch all sectors of a bio per crypto request
From: Leonid Ravich @ 2026-05-19 12:00 UTC (permalink / raw)
To: Herbert Xu
Cc: David S . Miller, Mike Snitzer, Mikulas Patocka, Alasdair Kergon,
Ard Biesheuvel, Eric Biggers, Jens Axboe, Horia Geanta,
Gilad Ben-Yossef, linux-crypto, dm-devel, linux-block
In-Reply-To: <20260428101225.24316-1-lravich@amazon.com>
When the underlying skcipher driver advertises support for multiple
data units in a single request (CRYPTO_ALG_SKCIPHER_MULTI_DATA_UNIT),
configure the cipher with cc->sector_size as data_unit_size and
submit one request per bio instead of one request per sector. This
removes per-sector overhead in the crypto API hot path: request
allocation, callback dispatch, completion handling, and SG setup.
The optimisation is enabled automatically at table load when all
of the following hold:
- the cipher is non-aead (i.e. skcipher);
- tfms_count is 1 (interleaved per-sector keys would break batching);
- the IV mode is plain or plain64 (the only modes whose generator
produces a sequential 64-bit little-endian counter that the cipher
can extend by adding the data-unit index, matching the convention
documented in crypto_skcipher_set_data_unit_size());
- the iv_gen_ops->post() hook is unset (lmk and tcw use it; both are
already excluded by the IV-mode test, but the explicit check makes
the assumption durable against future IV modes);
- dm-integrity is not stacked (no integrity tag or integrity IV);
- the cipher driver advertises multi-data-unit support.
A new CRYPT_MULTI_DATA_UNIT cipher_flag, set once at construction
time, gates the multi-data-unit path. The existing per-sector path
in crypt_convert_block_skcipher() is unchanged; the new
crypt_convert_block_skcipher_multi() is reached from a small dispatch
in crypt_convert() and shares the same backlog/-EBUSY/-EINPROGRESS
flow control with the per-sector path.
Heap-allocated scatterlists are stashed in dm_crypt_request and freed
in crypt_free_req_skcipher() to avoid races between the synchronous-
success free path and async-completion reuse from the request pool.
On -ENOMEM during scatterlist allocation, the bio is requeued via
BLK_STS_DEV_RESOURCE rather than failed, matching the behaviour of
the existing -ENOMEM path for crypto request allocation.
Verified end-to-end with a byte-equivalence test: encrypted output of
plain64 dm-crypt with the multi-data-unit path matches output of the
single-data-unit path bit-for-bit over a 256 MB device.
Signed-off-by: Leonid Ravich <lravich@amazon.com>
---
drivers/md/dm-crypt.c | 248 ++++++++++++++++++++++++++++++++++++++++--
1 file changed, 241 insertions(+), 7 deletions(-)
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 5ef43231fe77..b35831d43f0e 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -98,6 +98,14 @@ struct dm_crypt_request {
struct scatterlist sg_in[4];
struct scatterlist sg_out[4];
u64 iv_sector;
+ /*
+ * Heap-allocated scatterlists used by the multi-data-unit path
+ * when one bio is processed in a single skcipher request. NULL
+ * when the inline sg_in[]/sg_out[] arrays above are sufficient
+ * (single-data-unit path). Freed in crypt_free_req_skcipher().
+ */
+ struct scatterlist *sg_in_ext;
+ struct scatterlist *sg_out_ext;
};
struct crypt_config;
@@ -149,6 +157,7 @@ enum cipher_flags {
CRYPT_IV_LARGE_SECTORS, /* Calculate IV from sector_size, not 512B sectors */
CRYPT_ENCRYPT_PREPROCESS, /* Must preprocess data for encryption (elephant) */
CRYPT_KEY_MAC_SIZE_SET, /* The integrity_key_size option was used */
+ CRYPT_MULTI_DATA_UNIT, /* Batch all sectors of a bio per crypto request */
};
/*
@@ -1501,12 +1510,139 @@ static int crypt_convert_block_skcipher(struct crypt_config *cc,
return r;
}
+/*
+ * Multi-data-unit variant of crypt_convert_block_skcipher. Submits all
+ * remaining sectors of the current bio in one skcipher request whose
+ * data_unit_size is cc->sector_size. The cipher walks the IV between
+ * data units (see crypto_skcipher_set_data_unit_size()).
+ *
+ * Returns the same set of values as crypt_convert_block_skcipher:
+ * 0 on synchronous success (full chunk processed),
+ * -EINPROGRESS / -EBUSY on asynchronous dispatch,
+ * -ENOMEM if scatterlist allocation fails (caller maps to
+ * BLK_STS_DEV_RESOURCE so the bio is requeued, not failed),
+ * negative errno otherwise.
+ *
+ * On success the bio iterators have been advanced by the chunk size.
+ */
+static int crypt_convert_block_skcipher_multi(struct crypt_config *cc,
+ struct convert_context *ctx,
+ struct skcipher_request *req,
+ unsigned int *out_processed)
+{
+ const unsigned int sector_size = cc->sector_size;
+ unsigned int total_in = ctx->iter_in.bi_size;
+ unsigned int total_out = ctx->iter_out.bi_size;
+ unsigned int total = min(total_in, total_out);
+ unsigned int n_sectors;
+ unsigned int n_sg_in = 0, n_sg_out = 0;
+ struct dm_crypt_request *dmreq = dmreq_of_req(cc, req);
+ struct scatterlist *sg_in = NULL, *sg_out = NULL;
+ struct bvec_iter iter_in, iter_out;
+ struct bio_vec bv;
+ u8 *iv, *org_iv;
+ int r;
+
+ if (unlikely(total < sector_size))
+ return -EIO;
+ n_sectors = total / sector_size;
+ total = n_sectors * sector_size;
+
+ /*
+ * Walk the bio_vec iterators to count how many SG entries we need
+ * for exactly @total bytes. bi_size of the iterators is at least
+ * @total by construction above.
+ */
+ iter_in = ctx->iter_in;
+ iter_in.bi_size = total;
+ __bio_for_each_segment(bv, ctx->bio_in, iter_in, iter_in)
+ n_sg_in++;
+
+ iter_out = ctx->iter_out;
+ iter_out.bi_size = total;
+ __bio_for_each_segment(bv, ctx->bio_out, iter_out, iter_out)
+ n_sg_out++;
+
+ sg_in = kmalloc_array(n_sg_in, sizeof(*sg_in), GFP_NOIO);
+ sg_out = (ctx->bio_in == ctx->bio_out) ? sg_in :
+ kmalloc_array(n_sg_out, sizeof(*sg_out), GFP_NOIO);
+ if (!sg_in || !sg_out) {
+ kfree(sg_in);
+ if (sg_out != sg_in)
+ kfree(sg_out);
+ return -ENOMEM;
+ }
+
+ sg_init_table(sg_in, n_sg_in);
+ {
+ unsigned int i = 0;
+
+ iter_in = ctx->iter_in;
+ iter_in.bi_size = total;
+ __bio_for_each_segment(bv, ctx->bio_in, iter_in, iter_in)
+ sg_set_page(&sg_in[i++], bv.bv_page, bv.bv_len,
+ bv.bv_offset);
+ }
+
+ if (sg_out != sg_in) {
+ unsigned int i = 0;
+
+ sg_init_table(sg_out, n_sg_out);
+ iter_out = ctx->iter_out;
+ iter_out.bi_size = total;
+ __bio_for_each_segment(bv, ctx->bio_out, iter_out, iter_out)
+ sg_set_page(&sg_out[i++], bv.bv_page, bv.bv_len,
+ bv.bv_offset);
+ }
+
+ /*
+ * Compute the IV for the first data unit. The cipher will derive
+ * IVs for subsequent data units by treating this one as a 128-bit
+ * little-endian counter and adding the data-unit index, which
+ * matches the layout produced by plain and plain64.
+ */
+ dmreq->iv_sector = ctx->cc_sector;
+ if (test_bit(CRYPT_IV_LARGE_SECTORS, &cc->cipher_flags))
+ dmreq->iv_sector >>= cc->sector_shift;
+ dmreq->ctx = ctx;
+
+ iv = iv_of_dmreq(cc, dmreq);
+ org_iv = org_iv_of_dmreq(cc, dmreq);
+ r = cc->iv_gen_ops->generator(cc, org_iv, dmreq);
+ if (r < 0)
+ goto out_free_sg;
+ memcpy(iv, org_iv, cc->iv_size);
+
+ /* Stash the SG arrays for cleanup on completion / free. */
+ dmreq->sg_in_ext = sg_in;
+ dmreq->sg_out_ext = (sg_out == sg_in) ? NULL : sg_out;
+
+ skcipher_request_set_crypt(req, sg_in, sg_out, total, iv);
+
+ if (bio_data_dir(ctx->bio_in) == WRITE)
+ r = crypto_skcipher_encrypt(req);
+ else
+ r = crypto_skcipher_decrypt(req);
+
+ *out_processed = total;
+ return r;
+
+out_free_sg:
+ kfree(sg_in);
+ if (sg_out != sg_in)
+ kfree(sg_out);
+ dmreq->sg_in_ext = NULL;
+ dmreq->sg_out_ext = NULL;
+ return r;
+}
+
static void kcryptd_async_done(void *async_req, int error);
static int crypt_alloc_req_skcipher(struct crypt_config *cc,
struct convert_context *ctx)
{
unsigned int key_index = ctx->cc_sector & (cc->tfms_count - 1);
+ struct dm_crypt_request *dmreq;
if (!ctx->r.req) {
ctx->r.req = mempool_alloc(&cc->req_pool, in_interrupt() ? GFP_ATOMIC : GFP_NOIO);
@@ -1516,6 +1652,18 @@ static int crypt_alloc_req_skcipher(struct crypt_config *cc,
skcipher_request_set_tfm(ctx->r.req, cc->cipher_tfm.tfms[key_index]);
+ /*
+ * Initialise the heap-allocated scatterlist pointers so that
+ * crypt_free_req_skcipher() does not read uninitialised memory
+ * for paths that don't take the multi-data-unit branch. The
+ * dmreq trailer lives in the per-bio data area which is not
+ * zeroed by the dm core, and the request is reused from the
+ * mempool across many bios.
+ */
+ dmreq = dmreq_of_req(cc, ctx->r.req);
+ dmreq->sg_in_ext = NULL;
+ dmreq->sg_out_ext = NULL;
+
/*
* Use REQ_MAY_BACKLOG so a cipher driver internally backlogs
* requests if driver request queue is full.
@@ -1562,6 +1710,12 @@ static void crypt_free_req_skcipher(struct crypt_config *cc,
struct skcipher_request *req, struct bio *base_bio)
{
struct dm_crypt_io *io = dm_per_bio_data(base_bio, cc->per_bio_data_size);
+ struct dm_crypt_request *dmreq = dmreq_of_req(cc, req);
+
+ kfree(dmreq->sg_in_ext);
+ dmreq->sg_in_ext = NULL;
+ kfree(dmreq->sg_out_ext);
+ dmreq->sg_out_ext = NULL;
if ((struct skcipher_request *)(io + 1) != req)
mempool_free(req, &cc->req_pool);
@@ -1590,7 +1744,9 @@ static void crypt_free_req(struct crypt_config *cc, void *req, struct bio *base_
static blk_status_t crypt_convert(struct crypt_config *cc,
struct convert_context *ctx, bool atomic, bool reset_pending)
{
- unsigned int sector_step = cc->sector_size >> SECTOR_SHIFT;
+ const unsigned int sector_step = cc->sector_size >> SECTOR_SHIFT;
+ const bool multi_du = test_bit(CRYPT_MULTI_DATA_UNIT, &cc->cipher_flags);
+ unsigned int processed;
int r;
/*
@@ -1611,8 +1767,13 @@ static blk_status_t crypt_convert(struct crypt_config *cc,
atomic_inc(&ctx->cc_pending);
+ processed = cc->sector_size;
if (crypt_integrity_aead(cc))
r = crypt_convert_block_aead(cc, ctx, ctx->r.req_aead, ctx->tag_offset);
+ else if (multi_du)
+ r = crypt_convert_block_skcipher_multi(cc, ctx,
+ ctx->r.req,
+ &processed);
else
r = crypt_convert_block_skcipher(cc, ctx, ctx->r.req, ctx->tag_offset);
@@ -1634,8 +1795,19 @@ static blk_status_t crypt_convert(struct crypt_config *cc,
* exit and continue processing in a workqueue
*/
ctx->r.req = NULL;
- ctx->tag_offset++;
- ctx->cc_sector += sector_step;
+ if (!multi_du) {
+ ctx->tag_offset++;
+ ctx->cc_sector += sector_step;
+ } else {
+ bio_advance_iter(ctx->bio_in,
+ &ctx->iter_in,
+ processed);
+ bio_advance_iter(ctx->bio_out,
+ &ctx->iter_out,
+ processed);
+ ctx->cc_sector +=
+ processed >> SECTOR_SHIFT;
+ }
return BLK_STS_DEV_RESOURCE;
}
} else {
@@ -1649,19 +1821,42 @@ static blk_status_t crypt_convert(struct crypt_config *cc,
*/
case -EINPROGRESS:
ctx->r.req = NULL;
- ctx->tag_offset++;
- ctx->cc_sector += sector_step;
+ if (!multi_du) {
+ ctx->tag_offset++;
+ ctx->cc_sector += sector_step;
+ } else {
+ bio_advance_iter(ctx->bio_in, &ctx->iter_in,
+ processed);
+ bio_advance_iter(ctx->bio_out, &ctx->iter_out,
+ processed);
+ ctx->cc_sector += processed >> SECTOR_SHIFT;
+ }
continue;
/*
* The request was already processed (synchronously).
*/
case 0:
atomic_dec(&ctx->cc_pending);
- ctx->cc_sector += sector_step;
- ctx->tag_offset++;
+ if (!multi_du) {
+ ctx->cc_sector += sector_step;
+ ctx->tag_offset++;
+ } else {
+ bio_advance_iter(ctx->bio_in, &ctx->iter_in,
+ processed);
+ bio_advance_iter(ctx->bio_out, &ctx->iter_out,
+ processed);
+ ctx->cc_sector += processed >> SECTOR_SHIFT;
+ }
if (!atomic)
cond_resched();
continue;
+ /*
+ * Out of memory for the multi-DU SG arrays — bounce back
+ * to the caller for requeue rather than failing the bio.
+ */
+ case -ENOMEM:
+ atomic_dec(&ctx->cc_pending);
+ return BLK_STS_DEV_RESOURCE;
/*
* There was a data integrity error.
*/
@@ -3142,6 +3337,45 @@ static int crypt_ctr_cipher(struct dm_target *ti, char *cipher_in, char *key)
}
}
+ /*
+ * Enable multi-data-unit batching when the cipher supports it and
+ * the IV layout is one we can derive per-DU from a single starting
+ * IV: plain or plain64 produce a sequential 64-bit little-endian
+ * counter, which matches the convention of
+ * crypto_skcipher_set_data_unit_size(). Restrict to the simple
+ * case (single tfm, no integrity, no per-sector post() callback)
+ * to keep the consumer path small; modes like essiv, lmk, tcw,
+ * eboiv, plain64be, random, null, benbi, and elephant are
+ * deliberately excluded because their generators or post-IV hooks
+ * cannot be re-derived by the cipher between data units.
+ */
+ if (!crypt_integrity_aead(cc) && cc->tfms_count == 1 &&
+ cc->iv_gen_ops &&
+ (cc->iv_gen_ops == &crypt_iv_plain_ops ||
+ cc->iv_gen_ops == &crypt_iv_plain64_ops) &&
+ !cc->iv_gen_ops->post &&
+ !cc->integrity_tag_size && !cc->integrity_iv_size &&
+ crypto_skcipher_supports_multi_data_unit(cc->cipher_tfm.tfms[0])) {
+ ret = crypto_skcipher_set_data_unit_size(cc->cipher_tfm.tfms[0],
+ cc->sector_size);
+ if (!ret) {
+ set_bit(CRYPT_MULTI_DATA_UNIT, &cc->cipher_flags);
+ DMINFO("Using multi-data-unit crypto offload (du=%u)",
+ cc->sector_size);
+ } else {
+ /*
+ * The driver advertised the capability via cra_flags
+ * but rejected the requested data unit size. This is
+ * a driver bug worth seeing in dmesg; fall back to
+ * the per-sector path so the device still activates.
+ */
+ DMWARN_LIMIT("multi-DU offload disabled: %s rejected du=%u (%d)",
+ crypto_skcipher_driver_name(cc->cipher_tfm.tfms[0]),
+ cc->sector_size, ret);
+ ret = 0;
+ }
+ }
+
/* wipe the kernel key payload copy */
if (cc->key_string)
memset(cc->key, 0, cc->key_size * sizeof(u8));
--
2.47.3
^ permalink raw reply related
* [PATCH 3/4] crypto: testmgr - exercise multi-data-unit path for skcipher
From: Leonid Ravich @ 2026-05-19 11:59 UTC (permalink / raw)
To: Herbert Xu
Cc: David S . Miller, Mike Snitzer, Mikulas Patocka, Alasdair Kergon,
Ard Biesheuvel, Eric Biggers, Jens Axboe, Horia Geanta,
Gilad Ben-Yossef, linux-crypto, dm-devel, linux-block
In-Reply-To: <20260428101225.24316-1-lravich@amazon.com>
Add a self-comparison test that runs whenever an skcipher algorithm
advertises CRYPTO_ALG_SKCIPHER_MULTI_DATA_UNIT in cra_flags. The test
encrypts the same random plaintext two ways:
1. as one batched request with data_unit_size set, and
2. as N back-to-back single-data-unit requests with IVs derived from
the original IV by adding the data-unit index (treated as a
128-bit little-endian counter, matching the convention documented
in crypto_skcipher_set_data_unit_size()).
Both encrypts must produce byte-identical ciphertext, otherwise the
algorithm's multi-DU implementation is inconsistent with its single-DU
behaviour. Iterates over a fixed set of typical data unit sizes
(512, 1024, 2048, 4096) which cover the dm-crypt sector-size range.
The test is gated on ivsize == 16 (XTS, the only multi-DU consumer in
the kernel today) and on the algorithm advertising the capability,
so it costs nothing for the existing fleet of skcipher drivers.
Signed-off-by: Leonid Ravich <lravich@amazon.com>
---
crypto/testmgr.c | 129 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 129 insertions(+)
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 6a490aaa71b9..45cc7acc85ee 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -3217,6 +3217,123 @@ static int test_skcipher(int enc, const struct cipher_test_suite *suite,
return 0;
}
+/*
+ * For algorithms that advertise CRYPTO_ALG_SKCIPHER_MULTI_DATA_UNIT,
+ * verify that one request batching N data units produces the same
+ * ciphertext as N back-to-back single-data-unit requests with IVs
+ * derived from the original IV by adding the data-unit index (treated
+ * as a 128-bit little-endian counter).
+ *
+ * This is a self-comparison: it does not depend on test-vector
+ * authoritativeness, only on the algorithm being internally consistent
+ * between its single-DU and multi-DU paths.
+ */
+#define TEST_MDU_NR_UNITS 4
+static int test_skcipher_multi_du(struct crypto_skcipher *tfm,
+ unsigned int du_size)
+{
+ const char *driver = crypto_skcipher_driver_name(tfm);
+ const unsigned int ivsize = crypto_skcipher_ivsize(tfm);
+ const unsigned int total = du_size * TEST_MDU_NR_UNITS;
+ struct skcipher_request *req = NULL;
+ struct scatterlist sg_in, sg_out;
+ DECLARE_CRYPTO_WAIT(wait);
+ u8 iv_orig[16] = {0};
+ u8 iv_work[16];
+ u8 *plain = NULL, *batched = NULL, *unit = NULL;
+ unsigned int i;
+ int err;
+
+ if (ivsize != 16)
+ return 0;
+
+ plain = kmalloc(total, GFP_KERNEL);
+ batched = kmalloc(total, GFP_KERNEL);
+ unit = kmalloc(total, GFP_KERNEL);
+ req = skcipher_request_alloc(tfm, GFP_KERNEL);
+ if (!plain || !batched || !unit || !req) {
+ err = -ENOMEM;
+ goto out;
+ }
+
+ get_random_bytes(plain, total);
+ get_random_bytes(iv_orig, ivsize);
+
+ /* Pass 1: one batched encrypt with data_unit_size set. */
+ err = crypto_skcipher_set_data_unit_size(tfm, du_size);
+ if (err) {
+ pr_err("alg: skcipher: %s set_data_unit_size(%u) failed: %d\n",
+ driver, du_size, err);
+ goto out;
+ }
+ memcpy(batched, plain, total);
+ memcpy(iv_work, iv_orig, ivsize);
+ sg_init_one(&sg_in, batched, total);
+ sg_out = sg_in;
+ skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
+ CRYPTO_TFM_REQ_MAY_SLEEP,
+ crypto_req_done, &wait);
+ skcipher_request_set_crypt(req, &sg_in, &sg_out, total, iv_work);
+ err = crypto_wait_req(crypto_skcipher_encrypt(req), &wait);
+ if (err) {
+ pr_err("alg: skcipher: %s multi-DU batched encrypt failed: %d\n",
+ driver, err);
+ goto out_clear_du;
+ }
+
+ /* Pass 2: TEST_MDU_NR_UNITS single-DU encrypts with derived IVs. */
+ err = crypto_skcipher_set_data_unit_size(tfm, 0);
+ if (err)
+ goto out;
+ memcpy(unit, plain, total);
+ memcpy(iv_work, iv_orig, ivsize);
+ for (i = 0; i < TEST_MDU_NR_UNITS; i++) {
+ sg_init_one(&sg_in, unit + i * du_size, du_size);
+ sg_out = sg_in;
+ skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
+ CRYPTO_TFM_REQ_MAY_SLEEP,
+ crypto_req_done, &wait);
+ skcipher_request_set_crypt(req, &sg_in, &sg_out, du_size,
+ iv_work);
+ err = crypto_wait_req(crypto_skcipher_encrypt(req), &wait);
+ if (err) {
+ pr_err("alg: skcipher: %s single-DU[%u] encrypt failed: %d\n",
+ driver, i, err);
+ goto out;
+ }
+ /* Increment iv_work as a 128-bit little-endian counter. */
+ {
+ __le64 lo_le, hi_le;
+ u64 lo;
+
+ memcpy(&lo_le, iv_work, 8);
+ memcpy(&hi_le, iv_work + 8, 8);
+ lo = le64_to_cpu(lo_le) + 1;
+ lo_le = cpu_to_le64(lo);
+ memcpy(iv_work, &lo_le, 8);
+ if (lo == 0) {
+ hi_le = cpu_to_le64(le64_to_cpu(hi_le) + 1);
+ memcpy(iv_work + 8, &hi_le, 8);
+ }
+ }
+ }
+
+ if (memcmp(batched, unit, total) != 0) {
+ pr_err("alg: skcipher: %s multi-DU mismatch (du=%u, n=%u)\n",
+ driver, du_size, TEST_MDU_NR_UNITS);
+ err = -EINVAL;
+ }
+
+out_clear_du:
+ (void)crypto_skcipher_set_data_unit_size(tfm, 0);
+out:
+ skcipher_request_free(req);
+ kfree(unit);
+ kfree(batched);
+ kfree(plain);
+ return err;
+}
+
static int alg_test_skcipher(const struct alg_test_desc *desc,
const char *driver, u32 type, u32 mask)
{
@@ -3265,6 +3382,18 @@ static int alg_test_skcipher(const struct alg_test_desc *desc,
if (err)
goto out;
+ if (crypto_skcipher_supports_multi_data_unit(tfm)) {
+ static const unsigned int du_sizes[] = { 512, 1024, 2048, 4096 };
+ unsigned int j;
+
+ for (j = 0; j < ARRAY_SIZE(du_sizes); j++) {
+ err = test_skcipher_multi_du(tfm, du_sizes[j]);
+ if (err)
+ goto out;
+ cond_resched();
+ }
+ }
+
err = test_skcipher_vs_generic_impl(desc->generic_driver, req, tsgls);
out:
free_cipher_test_sglists(tsgls);
--
2.47.3
^ permalink raw reply related
* [PATCH 1/4] crypto: skcipher - add per-tfm data_unit_size for batched requests
From: Leonid Ravich @ 2026-05-19 11:59 UTC (permalink / raw)
To: Herbert Xu
Cc: David S . Miller, Mike Snitzer, Mikulas Patocka, Alasdair Kergon,
Ard Biesheuvel, Eric Biggers, Jens Axboe, Horia Geanta,
Gilad Ben-Yossef, linux-crypto, dm-devel, linux-block
In-Reply-To: <20260428101225.24316-1-lravich@amazon.com>
Add a per-tfm data_unit_size and an algorithm capability flag that
together allow a caller to submit several data units in a single
skcipher request. The IV passed in the request applies to the first
data unit; the algorithm advances the tweak between data units
according to the mode specification (e.g., LE128 multiply for XTS per
IEEE 1619).
This mirrors the data_unit_size concept already exposed by
struct blk_crypto_config for inline encryption hardware, but at the
software skcipher layer. The first user is dm-crypt, which today
issues one request per sector and so pays a per-sector cost in
request allocation, IV generation, callback dispatch, and completion
handling. Allowing the cipher to consume a whole bio per request
removes that overhead for drivers that can chain across data units
internally.
The data_unit_size lives on struct crypto_skcipher rather than on
struct skcipher_request because it does not change between requests
for any plausible consumer: dm-crypt picks one sector size per
mapped target at table load time; fscrypt would pick one per master
key. Anchoring it to the tfm also lets the driver validate it once
at setkey() time and avoids per-request initialisation hazards on
mempool-recycled requests.
Capability is advertised with CRYPTO_ALG_SKCIPHER_MULTI_DATA_UNIT
in cra_flags (type-specific high-byte range, mirroring the
CRYPTO_AHASH_ALG_* convention). This makes the capability visible
in /proc/crypto and lets templates OR it into their derived
algorithms.
crypto_skcipher_set_data_unit_size() returns -EOPNOTSUPP if the
algorithm does not advertise the flag, and accepts 0 (the default)
unconditionally so callers can re-disable batching cheaply.
crypto_skcipher_encrypt()/decrypt() reject requests whose cryptlen
is not a multiple of the configured data_unit_size with -EINVAL.
The check is gated on data_unit_size != 0 so it costs nothing for
the common single-data-unit case.
No in-tree algorithm advertises the flag yet; subsequent patches
add the generic xts() template, arm64, and x86 producers as well
as the dm-crypt consumer.
Signed-off-by: Leonid Ravich <lravich@amazon.com>
---
crypto/skcipher.c | 120 +++++++++++++++++++++++++++++
include/crypto/internal/skcipher.h | 34 ++++++++
include/crypto/skcipher.h | 85 ++++++++++++++++++++
3 files changed, 239 insertions(+)
diff --git a/crypto/skcipher.c b/crypto/skcipher.c
index 8fa5d9686d08..9155a4d9ea6d 100644
--- a/crypto/skcipher.c
+++ b/crypto/skcipher.c
@@ -183,13 +183,119 @@ int crypto_skcipher_setkey(struct crypto_skcipher *tfm, const u8 *key,
}
EXPORT_SYMBOL_GPL(crypto_skcipher_setkey);
+int crypto_skcipher_set_data_unit_size(struct crypto_skcipher *tfm,
+ unsigned int data_unit_size)
+{
+ unsigned int blocksize;
+
+ if (!data_unit_size) {
+ tfm->data_unit_size = 0;
+ return 0;
+ }
+
+ if (!crypto_skcipher_supports_multi_data_unit(tfm))
+ return -EOPNOTSUPP;
+
+ blocksize = crypto_skcipher_blocksize(tfm);
+ if (data_unit_size < blocksize || data_unit_size % blocksize)
+ return -EINVAL;
+
+ tfm->data_unit_size = data_unit_size;
+ return 0;
+}
+EXPORT_SYMBOL_GPL(crypto_skcipher_set_data_unit_size);
+
+static int crypto_skcipher_check_data_unit_size(struct crypto_skcipher *tfm,
+ struct skcipher_request *req)
+{
+ unsigned int du = tfm->data_unit_size;
+
+ if (likely(!du))
+ return 0;
+ if (req->cryptlen % du)
+ return -EINVAL;
+ return 0;
+}
+
+/*
+ * Increment a 16-byte little-endian counter held in @iv. See
+ * crypto_skcipher_set_data_unit_size() for the convention.
+ */
+static inline void skcipher_iv_inc_le128(u8 *iv)
+{
+ __le64 lo_le, hi_le;
+ u64 lo;
+
+ memcpy(&lo_le, iv, 8);
+ memcpy(&hi_le, iv + 8, 8);
+ lo = le64_to_cpu(lo_le) + 1;
+ lo_le = cpu_to_le64(lo);
+ memcpy(iv, &lo_le, 8);
+ if (unlikely(lo == 0)) {
+ hi_le = cpu_to_le64(le64_to_cpu(hi_le) + 1);
+ memcpy(iv + 8, &hi_le, 8);
+ }
+}
+
+int skcipher_walk_data_units(struct skcipher_request *req,
+ int (*body)(struct skcipher_request *))
+{
+ struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
+ const unsigned int du = tfm->data_unit_size;
+ const unsigned int total = req->cryptlen;
+ struct scatterlist *orig_src = req->src;
+ struct scatterlist *orig_dst = req->dst;
+ struct scatterlist src_sg[2], dst_sg[2];
+ u8 iv_save[16];
+ unsigned int off;
+ int err = 0;
+
+ if (likely(!du))
+ return body(req);
+
+ /*
+ * Registration of an algorithm advertising
+ * CRYPTO_ALG_SKCIPHER_MULTI_DATA_UNIT enforces ivsize == 16
+ * (see skcipher_prepare_alg_common()), so this is purely
+ * defensive against algorithm-registration bugs.
+ */
+ if (WARN_ON_ONCE(crypto_skcipher_ivsize(tfm) != 16))
+ return -EINVAL;
+
+ memcpy(iv_save, req->iv, 16);
+
+ for (off = 0; off < total; off += du) {
+ req->cryptlen = du;
+ req->src = scatterwalk_ffwd(src_sg, orig_src, off);
+ req->dst = (orig_src == orig_dst) ? req->src :
+ scatterwalk_ffwd(dst_sg, orig_dst, off);
+
+ err = body(req);
+ if (err)
+ break;
+
+ skcipher_iv_inc_le128(iv_save);
+ memcpy(req->iv, iv_save, 16);
+ }
+
+ req->src = orig_src;
+ req->dst = orig_dst;
+ req->cryptlen = total;
+ return err;
+}
+EXPORT_SYMBOL_GPL(skcipher_walk_data_units);
+
int crypto_skcipher_encrypt(struct skcipher_request *req)
{
struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
struct skcipher_alg *alg = crypto_skcipher_alg(tfm);
+ int err;
if (crypto_skcipher_get_flags(tfm) & CRYPTO_TFM_NEED_KEY)
return -ENOKEY;
+ err = crypto_skcipher_check_data_unit_size(tfm, req);
+ if (err)
+ return err;
if (alg->co.base.cra_type != &crypto_skcipher_type)
return crypto_lskcipher_encrypt_sg(req);
return alg->encrypt(req);
@@ -200,9 +306,13 @@ int crypto_skcipher_decrypt(struct skcipher_request *req)
{
struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
struct skcipher_alg *alg = crypto_skcipher_alg(tfm);
+ int err;
if (crypto_skcipher_get_flags(tfm) & CRYPTO_TFM_NEED_KEY)
return -ENOKEY;
+ err = crypto_skcipher_check_data_unit_size(tfm, req);
+ if (err)
+ return err;
if (alg->co.base.cra_type != &crypto_skcipher_type)
return crypto_lskcipher_decrypt_sg(req);
return alg->decrypt(req);
@@ -432,6 +542,16 @@ int skcipher_prepare_alg_common(struct skcipher_alg_common *alg)
(alg->ivsize + alg->statesize) > PAGE_SIZE / 2)
return -EINVAL;
+ /*
+ * Algorithms advertising multi-data-unit support must use the
+ * 16-byte little-endian counter convention documented in
+ * crypto_skcipher_set_data_unit_size(); see also
+ * skcipher_walk_data_units().
+ */
+ if ((base->cra_flags & CRYPTO_ALG_SKCIPHER_MULTI_DATA_UNIT) &&
+ alg->ivsize != 16)
+ return -EINVAL;
+
if (!alg->chunksize)
alg->chunksize = base->cra_blocksize;
diff --git a/include/crypto/internal/skcipher.h b/include/crypto/internal/skcipher.h
index d5aa535263f6..bfabc97f34ef 100644
--- a/include/crypto/internal/skcipher.h
+++ b/include/crypto/internal/skcipher.h
@@ -22,6 +22,40 @@
*/
#define CRYPTO_ALG_SKCIPHER_REQSIZE_LARGE CRYPTO_ALG_OPTIONAL_KEY
+/**
+ * skcipher_walk_data_units - dispatch a request as one body call per data unit
+ * @req: the caller's skcipher request
+ * @body: the algorithm's single-data-unit encrypt or decrypt function
+ *
+ * When tfm->data_unit_size is zero this is a tail call into @body with
+ * @req unchanged. Otherwise the request is split into
+ * cryptlen / data_unit_size sub-ranges and @body is called once per
+ * sub-range with req->cryptlen, req->src, req->dst, and req->iv adjusted
+ * for that sub-range. The IV passed to data unit n is the caller-
+ * supplied IV plus n, where + is a 128-bit little-endian add — this
+ * matches the convention documented in
+ * crypto_skcipher_set_data_unit_size().
+ *
+ * Many single-data-unit XTS bodies modify the IV buffer in place during
+ * processing (the tweak is walked block by block). This helper saves
+ * the caller's IV before each call and rewrites the next data unit's
+ * IV from the saved value, so the body always sees a fresh per-DU IV
+ * regardless of any in-place mutation it performs.
+ *
+ * The body MUST run to completion synchronously. Drivers that use this
+ * helper therefore advertise CRYPTO_ALG_SKCIPHER_MULTI_DATA_UNIT only
+ * for synchronous configurations.
+ *
+ * After the call returns, the contents of req->iv are unspecified per
+ * the documented contract. src/dst/cryptlen are restored to the
+ * caller's values to keep skcipher request post-conditions intact.
+ *
+ * Return: 0 on success, or the body's negative errno on the first
+ * data unit that returned non-zero.
+ */
+int skcipher_walk_data_units(struct skcipher_request *req,
+ int (*body)(struct skcipher_request *));
+
struct aead_request;
struct rtattr;
diff --git a/include/crypto/skcipher.h b/include/crypto/skcipher.h
index 9e5853464345..c4112c57f6a2 100644
--- a/include/crypto/skcipher.h
+++ b/include/crypto/skcipher.h
@@ -26,6 +26,15 @@
/* Set this bit if the skcipher operation is not final. */
#define CRYPTO_SKCIPHER_REQ_NOTFINAL 0x00000002
+/*
+ * Set in cra_flags by an skcipher algorithm that supports processing
+ * multiple data units in a single request. See
+ * crypto_skcipher_set_data_unit_size().
+ *
+ * Type-specific flag in the 0xff000000 reserved range.
+ */
+#define CRYPTO_ALG_SKCIPHER_MULTI_DATA_UNIT 0x01000000
+
struct scatterlist;
/**
@@ -53,6 +62,22 @@ struct skcipher_request {
struct crypto_skcipher {
unsigned int reqsize;
+ /*
+ * Number of bytes in one data unit when batching multiple data units
+ * per request. 0 means "single data unit per request" (legacy
+ * behaviour). Set via crypto_skcipher_set_data_unit_size().
+ *
+ * When non-zero, cryptlen must be a multiple of data_unit_size. The
+ * IV passed in skcipher_request::iv applies to the first data unit;
+ * the algorithm advances the tweak between data units according to
+ * the mode specification (e.g., LE128 multiply for XTS per
+ * IEEE 1619).
+ *
+ * Only algorithms that advertise CRYPTO_ALG_SKCIPHER_MULTI_DATA_UNIT
+ * in cra_flags accept a non-zero value.
+ */
+ unsigned int data_unit_size;
+
struct crypto_tfm base;
};
@@ -491,6 +516,66 @@ static inline unsigned int crypto_lskcipher_chunksize(
return crypto_lskcipher_alg(tfm)->co.chunksize;
}
+/**
+ * crypto_skcipher_supports_multi_data_unit() - test multi-data-unit support
+ * @tfm: cipher handle
+ *
+ * Return: true if the algorithm advertises that it can process multiple
+ * data units in a single skcipher_request.
+ */
+static inline bool
+crypto_skcipher_supports_multi_data_unit(struct crypto_skcipher *tfm)
+{
+ return crypto_skcipher_alg_common(tfm)->base.cra_flags &
+ CRYPTO_ALG_SKCIPHER_MULTI_DATA_UNIT;
+}
+
+/**
+ * crypto_skcipher_set_data_unit_size() - set data unit size for the tfm
+ * @tfm: cipher handle
+ * @data_unit_size: data unit size in bytes; 0 disables multi-data-unit mode
+ *
+ * Configure the tfm to process multiple data units per request. When set
+ * to a non-zero value, every subsequent encrypt/decrypt request must have
+ * cryptlen that is a multiple of @data_unit_size. Each data unit is
+ * processed as if it were a separate request whose IV is derived from the
+ * preceding data unit's IV by the algorithm-specific tweak update rule:
+ * the implementation treats the caller-supplied IV as a 128-bit
+ * little-endian counter and adds the data-unit index for each subsequent
+ * data unit.
+ *
+ * The contents of req->iv after a multi-data-unit request returns are
+ * unspecified — callers MUST NOT rely on it being either the original
+ * value or the final-data-unit value. Set a fresh IV before every
+ * request.
+ *
+ * The algorithm must advertise CRYPTO_ALG_SKCIPHER_MULTI_DATA_UNIT in its
+ * cra_flags. @data_unit_size must be a positive multiple of the
+ * algorithm's cra_blocksize, otherwise -EINVAL is returned.
+ *
+ * Setting @data_unit_size to 0 reverts the tfm to single-data-unit
+ * behaviour and is always permitted.
+ *
+ * Return: 0 on success; -EOPNOTSUPP if the algorithm does not advertise
+ * multi-data-unit support; -EINVAL if @data_unit_size is not a
+ * positive multiple of the cipher block size.
+ */
+int crypto_skcipher_set_data_unit_size(struct crypto_skcipher *tfm,
+ unsigned int data_unit_size);
+
+/**
+ * crypto_skcipher_data_unit_size() - obtain data unit size
+ * @tfm: cipher handle
+ *
+ * Return: configured data unit size in bytes; 0 if multi-data-unit mode
+ * is disabled.
+ */
+static inline unsigned int
+crypto_skcipher_data_unit_size(struct crypto_skcipher *tfm)
+{
+ return tfm->data_unit_size;
+}
+
/**
* crypto_skcipher_statesize() - obtain state size
* @tfm: cipher handle
--
2.47.3
^ permalink raw reply related
* [PATCH 2/4] crypto: xts - support multiple data units per request in template
From: Leonid Ravich @ 2026-05-19 11:59 UTC (permalink / raw)
To: Herbert Xu
Cc: David S . Miller, Mike Snitzer, Mikulas Patocka, Alasdair Kergon,
Ard Biesheuvel, Eric Biggers, Jens Axboe, Horia Geanta,
Gilad Ben-Yossef, linux-crypto, dm-devel, linux-block
In-Reply-To: <20260428101225.24316-1-lravich@amazon.com>
Teach the generic xts() template to consume cryptlen larger than one
data unit when the caller has configured a non-zero data_unit_size on
the tfm. Each data unit is processed with its own IV, derived from
the caller-supplied IV by treating it as a 128-bit little-endian
counter and adding the data-unit index. This matches the
sector-indexed XTS used by dm-crypt's plain64 IV mode and by typical
inline-encryption hardware.
The single-data-unit body is unchanged and is now reached via a thin
xts_crypt_multi() dispatcher that skips straight to the body when
data_unit_size is zero (the legacy default), so existing users see
no extra cost.
Advertise CRYPTO_ALG_SKCIPHER_MULTI_DATA_UNIT in cra_flags only when
the inner cipher is synchronous. An async inner cipher would require
a per-DU completion chain which is out of scope for the slow software
template; consumers that need multi-DU on async hardware will use one
of the arch-specific drivers added later in this series.
Signed-off-by: Leonid Ravich <lravich@amazon.com>
---
crypto/xts.c | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/crypto/xts.c b/crypto/xts.c
index 3da8f5e053d6..2b7233311dad 100644
--- a/crypto/xts.c
+++ b/crypto/xts.c
@@ -258,7 +258,7 @@ static int xts_init_crypt(struct skcipher_request *req,
return 0;
}
-static int xts_encrypt(struct skcipher_request *req)
+static int xts_encrypt_one(struct skcipher_request *req)
{
struct xts_request_ctx *rctx = skcipher_request_ctx(req);
struct skcipher_request *subreq = &rctx->subreq;
@@ -275,7 +275,7 @@ static int xts_encrypt(struct skcipher_request *req)
return xts_cts_final(req, crypto_skcipher_encrypt);
}
-static int xts_decrypt(struct skcipher_request *req)
+static int xts_decrypt_one(struct skcipher_request *req)
{
struct xts_request_ctx *rctx = skcipher_request_ctx(req);
struct skcipher_request *subreq = &rctx->subreq;
@@ -292,6 +292,16 @@ static int xts_decrypt(struct skcipher_request *req)
return xts_cts_final(req, crypto_skcipher_decrypt);
}
+static int xts_encrypt(struct skcipher_request *req)
+{
+ return skcipher_walk_data_units(req, xts_encrypt_one);
+}
+
+static int xts_decrypt(struct skcipher_request *req)
+{
+ return skcipher_walk_data_units(req, xts_decrypt_one);
+}
+
static int xts_init_tfm(struct crypto_skcipher *tfm)
{
struct skcipher_instance *inst = skcipher_alg_instance(tfm);
@@ -427,6 +437,17 @@ static int xts_create(struct crypto_template *tmpl, struct rtattr **tb)
inst->alg.base.cra_alignmask = alg->base.cra_alignmask |
(__alignof__(u64) - 1);
+ /*
+ * Advertise multi-data-unit support only when the inner cipher is
+ * synchronous. The dispatcher in skcipher_walk_data_units() calls
+ * the single-DU body in a loop and assumes synchronous completion;
+ * supporting async would require a per-DU callback chain, which
+ * the slow software template does not need.
+ */
+ if (!(alg->base.cra_flags & CRYPTO_ALG_ASYNC))
+ inst->alg.base.cra_flags |=
+ CRYPTO_ALG_SKCIPHER_MULTI_DATA_UNIT;
+
inst->alg.ivsize = XTS_BLOCK_SIZE;
inst->alg.min_keysize = alg->min_keysize * 2;
inst->alg.max_keysize = alg->max_keysize * 2;
--
2.47.3
^ permalink raw reply related
* [PATCH 0/4] crypto: skcipher - per-tfm multi-data-unit batching
From: Leonid Ravich @ 2026-05-19 11:59 UTC (permalink / raw)
To: Herbert Xu
Cc: David S . Miller, Mike Snitzer, Mikulas Patocka, Alasdair Kergon,
Ard Biesheuvel, Eric Biggers, Jens Axboe, Horia Geanta,
Gilad Ben-Yossef, linux-crypto, dm-devel, linux-block
In-Reply-To: <20260428101225.24316-1-lravich@amazon.com>
This implements the multi-data-unit skcipher request flow proposed in
the RFC thread [1], following Herbert's ack of the IPsec-friendly
shape and the proof-of-concept performance numbers I posted in [2]
(+19% throughput / -40% CPU on a single-core arm64 system with a
hardware XTS-AES-256 accelerator running fio 4 KiB sequential writes
through dm-crypt).
The series adds a per-tfm "data unit size" to the skcipher API so a
caller can submit several data units in one crypto request, mirroring
the data_unit_size concept already exposed by struct blk_crypto_config
for inline encryption hardware.
The first user is dm-crypt, which today issues one skcipher request
per sector and so pays a per-sector cost in request allocation,
callback dispatch, completion handling, and scatterlist setup.
Allowing the cipher to consume a whole bio per request removes that
overhead. As shown in [2], the per-sector cost dominates the profile
(~25% of CPU cycles) on a hardware accelerator where AES rounds
themselves are nearly free.
[1] https://lore.kernel.org/linux-crypto/... (RFC: crypto: skcipher
multi-data-unit requests for dm-crypt)
[2] Message-Id: 20260428101225.24316-1-lravich@amazon.com
Design overview
---------------
* Patch 1 adds an `unsigned int data_unit_size` field to
`struct crypto_skcipher` (per-tfm: invariant for the consumer's
lifetime, set once via `crypto_skcipher_set_data_unit_size()`),
plus a capability flag CRYPTO_ALG_SKCIPHER_MULTI_DATA_UNIT in
`cra_flags` (type-specific high-byte range, mirroring the
CRYPTO_AHASH_ALG_BLOCK_ONLY precedent). `crypto_skcipher_encrypt()`
and `crypto_skcipher_decrypt()` validate that `cryptlen` is a
positive multiple of `data_unit_size`. The setter rejects
sub-blocksize values; algorithm registration rejects the flag for
algorithms with `ivsize != 16`.
Also exposes `skcipher_walk_data_units()` in
<crypto/internal/skcipher.h> as a default per-DU dispatcher for
drivers that don't want to roll their own.
* Patch 2 lets the generic `xts(...)` template advertise the flag
when the inner cipher is synchronous. This is the in-tree
software producer of the new capability.
* Patch 3 extends `testmgr` with a self-comparison test that fires
automatically for every alg advertising the flag. The test
encrypts random plaintext two ways - one batched request vs N
back-to-back single-DU requests with derived IVs - and rejects
the algorithm if the ciphertexts differ.
* Patch 4 turns dm-crypt on automatically when all of the following
hold at table load: skcipher (not aead), `tfms_count == 1`, IV
mode is plain or plain64, no per-sector `iv_gen_ops->post()`, no
dm-integrity stacking, and the underlying cipher advertises the
capability. Heap-allocated scatterlists are stashed in
`dm_crypt_request` and freed in `crypt_free_req_skcipher()`,
initialised to NULL on every request alloc to keep the free path
safe on the per-sector code path that does not use them.
This series intentionally does NOT add the capability flag to any
arch crypto driver. Arch maintainers can opt in independently by
wrapping their xts(aes) entry points with skcipher_walk_data_units()
or, for hardware engines, by submitting one HW command for the whole
multi-DU request. The contract documented in
crypto_skcipher_set_data_unit_size() is the only obligation.
Why per-tfm and why cra_flags
-----------------------------
`data_unit_size` is invariant for the tfm's lifetime in every
plausible consumer. dm-crypt picks one sector size per mapped
target at table load. fscrypt would pick one per master key.
IPsec would pick one per SA. Putting the field on
`crypto_skcipher` (rather than on every `skcipher_request`) avoids
growing a hot per-request struct used by fscrypt, IPsec ESP,
AF_ALG, etc. It also lets the driver validate the value once in
`setkey()` and keeps the encrypt/decrypt fast path single-branch
(`likely(!data_unit_size)`).
The capability lives in `cra_flags` for consistency with existing
skcipher capabilities, so it surfaces in `/proc/crypto` and templates
can OR it into derived algorithms.
IV semantics
------------
The contract documented in `crypto_skcipher_set_data_unit_size()`:
the algorithm treats the caller-supplied IV as a 128-bit
little-endian counter and adds the data-unit index for each
subsequent data unit. This is what dm-crypt's plain and plain64
generators already produce, so no IV translation is needed at the
boundary. For modes that don't fit (essiv, lmk, tcw, eboiv,
plain64be, random, null, benbi, elephant) dm-crypt falls back to the
existing per-sector path.
Verification
------------
* checkpatch.pl --strict: clean on all 4 patches.
* Builds clean on x86_64 and arm64.
* QEMU boots; existing xts-aes-aesni / xts-aes-ce / xts-aes-neon
crypto self-tests pass.
* In-kernel testmgr self-comparison passes for any algorithm
advertising the flag.
* dm-crypt round-trip with plain64: encrypt+decrypt produces correct
data through both the existing per-sector path and the multi-DU
path (the latter exercised against an out-of-tree arm64 / x86 xts
enablement during development).
* dm-crypt activation gating: plain -> enabled, plain64 -> enabled,
essiv:sha256 -> fallback (correctly rejected), plain64be ->
fallback.
* Byte-equivalence: 256 MB of ciphertext written through the
multi-DU path is bit-identical to ciphertext written through the
per-sector path (sha256
4913910b1aa6f8859fcb8f4adec20230274993a3ade8f4dd0140a323dc43efc0
on plain64+xts-aes). The on-disk format is unchanged.
Leonid Ravich (4):
crypto: skcipher - add per-tfm data_unit_size for batched requests
crypto: xts - support multiple data units per request in template
crypto: testmgr - exercise multi-data-unit path for skcipher
dm crypt: batch all sectors of a bio per crypto request
crypto/skcipher.c | 120 ++++++++++++++
crypto/testmgr.c | 129 +++++++++++++++
crypto/xts.c | 25 ++-
drivers/md/dm-crypt.c | 248 ++++++++++++++++++++++++++++-
include/crypto/internal/skcipher.h | 34 ++++
include/crypto/skcipher.h | 85 ++++++++++
6 files changed, 632 insertions(+), 9 deletions(-)
--
2.47.3
^ permalink raw reply
* Re: [PATCH] block/loop: Fix NULL pointer dereference in lo_rw_aio()
From: Ming Lei @ 2026-05-19 11:28 UTC (permalink / raw)
To: Hongling Zeng
Cc: Hongling Zeng, axboe, ming.lei, linux-block, linux-kernel,
Tetsuo Handa
In-Reply-To: <6A0C29F6.5080704@126.com>
On Tue, May 19, 2026 at 05:14:30PM +0800, Hongling Zeng wrote:
> Hi Ming,
>
> Tetsuo’s syzkaller trace confirms the root cause:
>
> Timeline:
> T6142: lo_rw_aio(loop3) starting read with raw_refcnt=0x0, refcnt=1
> T44: lo_rw_aio(loop3) starting read with raw_refcnt=0x0, refcnt=1
> T6148: __loop_clr_fd(loop3) clearing lo_backing_file with raw_refcnt=0x0,
> refcnt=1
> T180: lo_rw_aio(loop3) starting write with NULL file (already cleared?)
> T180: CRASH - null-ptr-deref
IO could be from writeback or early close, so the following patch should help:
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 0000913f7efc..24654a03db71 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;
+ drain_workqueue(lo->workqueue);
+
spin_lock_irq(&lo->lo_lock);
filp = lo->lo_backing_file;
lo->lo_backing_file = NULL;
@@ -1857,9 +1859,6 @@ static blk_status_t loop_queue_rq(struct blk_mq_hw_ctx *hctx,
blk_mq_start_request(rq);
- if (data_race(READ_ONCE(lo->lo_state)) != Lo_bound)
- return BLK_STS_IOERR;
-
switch (req_op(rq)) {
case REQ_OP_FLUSH:
case REQ_OP_DISCARD:
@@ -1901,6 +1900,11 @@ static void loop_handle_cmd(struct loop_cmd *cmd)
int ret = 0;
struct mem_cgroup *old_memcg = NULL;
+ if (data_race(READ_ONCE(lo->lo_state)) != Lo_bound) {
+ ret = -EIO;
+ goto failed;
+ }
+
if (write && (lo->lo_flags & LO_FLAGS_READ_ONLY)) {
ret = -EIO;
goto failed;
>
> Key anomaly: |raw_refcnt=0|while |refcnt=1|.
> 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?
Thanks,
Ming
^ permalink raw reply related
* Re: [PATCH] sched: flush plug in schedule_preempt_disabled() to prevent deadlock
From: Michael Wu @ 2026-05-19 10:30 UTC (permalink / raw)
To: Xiaosen, Ming Lei, Peter Zijlstra
Cc: Tejun Heo, Jens Axboe, linux-block, linux-kernel, Ingo Molnar,
Juri Lelli, Vincent Guittot, Thomas Gleixner
In-Reply-To: <6611f3ea-b70f-45c7-9332-235647643a49@oss.qualcomm.com>
The patch you mentioned also solves my problem; I've verified and tested
it on my platform.
On 5/15/2026 2:18 PM, Xiaosen wrote:
> https://lore.kernel.org/lkml/20260427183848.698551-2-jstultz@google.com/
> The above change can resolve the deadlock I reported before by setting
> task's state to TASK_RUNNING before switching context.
>
> There is the likely alternative fix below.
> https://lore.kernel.org/lkml/20260512025635.2840817-1-jstultz@google.com/
>
> Regards,
> Xiaosen
>
> On 5/13/2026 4:08 PM, Ming Lei wrote:
>> On Wed, May 13, 2026 at 09:30:39AM +0200, Peter Zijlstra wrote:
>>> On Wed, May 13, 2026 at 10:07:03AM +0800, Ming Lei wrote:
>>>> On Tue, May 12, 2026 at 07:16:36AM -1000, Tejun Heo wrote:
>>>>> Hello, Ming.
>>>>>
>>>>> On Tue, May 12, 2026 at 11:45:14PM +0800, Ming Lei wrote:
>>>>>> On Tue, May 12, 2026 at 02:40:21PM +0200, Peter Zijlstra wrote:
>>>>>>> On Tue, May 12, 2026 at 02:04:32PM +0200, Peter Zijlstra wrote:
>>>>>>>> On Tue, May 12, 2026 at 04:59:39PM +0800, Ming Lei wrote:
>>>>>>>>> On preemptible kernels, a deadlock can occur when a task with plugged IO
>>>>>>>>> calls schedule_preempt_disabled():
>>>>>>>>>
>>>>>>>>> schedule_preempt_disabled()
>>>>>>>>> sched_preempt_enable_no_resched() // preemption now enabled
>>>>>>>>> schedule() // <-- preemption can happen here
>>>>>>>>> sched_submit_work()
>>>>>>>>> blk_flush_plug()
>>>>>>>>>
>>>>>>>>> After sched_preempt_enable_no_resched() re-enables preemption, the task
>>>>>>>>> can be preempted (e.g., by a higher-priority RT task) before reaching
>>>>>>>>> blk_flush_plug() in sched_submit_work(). Since the task's state is
>>>>>>>>> already TASK_UNINTERRUPTIBLE (set by the mutex/rwsem slowpath caller),
>>>>>>>>> requests in current->plug remain unflushed for an unbounded time.
>>>>>>>>>
>>>>>>>>> If another task depends on those plugged requests to make progress (e.g.,
>>>>>>>>> to release a lock the sleeping task needs), a deadlock results:
>>>>>>>>>
>>>>>>>>> - Task A (writeback worker): holds plugged IO, preempted before
>>>>>>>>> flushing, stuck on run queue behind higher-priority work
>>>>>>>>> - Task B: waiting for IO completion from Task A's plug, holds a lock
>>>>>>>>> that Task A needs to be woken up
>>>>>
>>>>> My memory is hazy around io_schedule but the above reads really weird to me.
>>>>> A task, regardless of its current state stays on the runqueue when
>>>>> preempted, so the condition is temporary. As soon as the preempted task can
>>>>> get CPU, it should unwind the situation. That's not a deadlock. Is the
>>>>> problem that there can be preemption-induced delay in flushing the plugs?
>>>>
>>>> IMO, preempting a `!TASK_RUNNING` task can be thought as effective sleep,
>>>
>>> No it cannot be. Preemption ignores task state.
>>
>> Yeah, I get similar conclusion too with AI's assistance.
>>
>> But both two reports show that the preempted task aren't switched back for
>> long enough time, can you share any idea for Michael & Xiaosen to investigate
>> further from scheduler side?
>>
>> https://lore.kernel.org/linux-block/20260417082744.30124-1-michael@allwinnertech.com/
>>
>> https://lore.kernel.org/linux-block/5660795d-87de-46f5-add4-7729a02225ef@oss.qualcomm.com/
>>
>>
>> Thanks,
>> Ming
--
Regards,
Michael Wu
^ permalink raw reply
* Re: [PATCH v2] loop: Fix NULL pointer dereference by synchronizing lo_release and loop_queue_rq
From: Tetsuo Handa @ 2026-05-19 9:27 UTC (permalink / raw)
To: Bart Van Assche, Jens Axboe, Christoph Hellwig, Damien Le Moal
Cc: linux-block, LKML, Andrew Morton
In-Reply-To: <20260518174013.4b72dd50a5bcb89daaed1f62@linux-foundation.org>
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);
+
spin_lock_irq(&lo->lo_lock);
filp = lo->lo_backing_file;
lo->lo_backing_file = NULL;
^ permalink raw reply related
* Re: [PATCH v3 05/10] lib: add dmabuf token infrastructure
From: Christoph Hellwig @ 2026-05-19 9:25 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: <9933142a-4ce2-4219-9574-73da30edd74e@gmail.com>
On Tue, May 19, 2026 at 08:55:32AM +0100, Pavel Begunkov wrote:
> On 5/19/26 07:56, Christoph Hellwig wrote:
>> On Mon, May 18, 2026 at 03:23:53PM +0100, Pavel Begunkov wrote:
>>> To be fair, it's not that dma-buf specific. This lib/ code only
>>> does some resv locking, fence waiting and queuing fences,
>>
>> But all the dma resv/fence stuff is pretty tied into the dma-buf
>> ecosystem. I don't think it would really apply to something not
>> doing DMA at all.
>
> The point is that those can be separated to reuse the rest.
Are you actually doing this right now? If so please share what you
have, otherwise let's keep the dma-buf bits together and move things
if new abstractions emerge.
>> But none of that really sits in the current lib/ code anyway?
>
> It's about naming. E.g. passing a DMABUF_ITER that doesn't have a
> dma-buf would be confusing, and then it'll need renaming at all
> layers to support the use case.
Again, if you concretely are doing this right now, find a good
name and place based on those abstractions. If not let's ignore
it and move it if needed.
>> drivers/dma-buf is a pretty natural place for it, I could not thing
>
> _If_ there is no dma mappings, drivers/dma-buf would definitely
> be an awkward spot.
Yes. But that's not the case right now. And from looking at the
handwaiving for ublk/fuse probably not anytime soon, but maybe I'm
mistaken on the latter.
^ permalink raw reply
* Re: [PATCH v3 04/10] block: introduce dma map backed bio type
From: David Laight @ 2026-05-19 9:21 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, 18 May 2026 11:29:54 +0100
Pavel Begunkov <asml.silence@gmail.com> wrote:
> On 5/13/26 09:19, Christoph Hellwig wrote:
> >> + if (!bio_flagged(bio_src, BIO_DMABUF_MAP)) {
> >> + bio->bi_io_vec = bio_src->bi_io_vec;
> >> + } else {
> >> + bio->dmabuf_map = bio_src->dmabuf_map;
> >> + bio_set_flag(bio, BIO_DMABUF_MAP);
> >> + }
> >
> > This is backwards, please avoid pointless negations:
>
> I can flip it, but compilers tend to prefer the true branch. E.g. this
>
> if (cond) A; else B;
> C;
>
> can get compiled into:
>
> jmpcc cond B
> A: ...
> C:
> return;
> B: ...
> jmp C;
I'm pretty sure gcc completely ignores the order.
Some very old compilers didn't - to the extreme of generating a short
conditional branch around a long unconditional branch to get past a
large 'true' code block.
likely() and unlikely() can change things, they are definitely useful
comments to a human (or Human) reader.
I'm not sure you can rely on the branch predictor to do anything sensible
outside of a loop.
-- David
^ permalink raw reply
* Re: [PATCH] block/loop: Fix NULL pointer dereference in lo_rw_aio()
From: Ming Lei @ 2026-05-19 8:33 UTC (permalink / raw)
To: Hongling Zeng; +Cc: Hongling Zeng, axboe, ming.lei, linux-block, linux-kernel
In-Reply-To: <6A0BFDE2.5000808@126.com>
On Tue, May 19, 2026 at 02:06:26PM +0800, Hongling Zeng wrote:
> Hi,
>
> Thank you for the review.
>
> 在 2026年05月19日 12:42, Ming Lei 写道:
> > On Tue, May 19, 2026 at 12:01:03PM +0800, Hongling Zeng wrote:
> > > lo->lo_backing_file can be NULL when the loop device is being cleared,
> > > causing NULL pointer dereference in lo_rw_aio(). Add a defensive check
> > > to prevent kernel crash.
> > >
> > > In v7.1-rc tree, lo->lo_backing_file is cleared in __loop_clr_fd(), which
> > > is called from lo_release() only, when it is guaranteed that there isn't
> > > inflight IO.
> > >
> > > So care to share your stack trace?
> Syzkaller has managed to trigger NULL pointer dereference in lo_rw_aio().
> Tetsuo Handa encountered the same issue (commit 1a83d226268cf), but his
> patch only adds diagnostic logging and is marked "not for upstream".
> > > Also fix loop_attr_backing_file_show() to use PTR_ERR_OR_ZERO()
> > > for correct NULL pointer handling.
> > >
> > > Fixes: bc07c10a3603a ("block: loop: support DIO & AIO")
> > Can you explain how the issue is introduced in above commit?
> The issue was introduced when lo_rw_aio() was added in that commit,
> which dereferences lo->lo_backing_file without NULL checks. While the
> design assumes no inflight IO during __loop_clr_fd(), fuzzers have
> found edge cases where this assumption doesn't hold.
What are the edge cases?
>
> > > Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> > > ---
> > > drivers/block/loop.c | 5 ++++-
> > > 1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> > > index 0000913f7efc..d8db1b0d8018 100644
> > > --- a/drivers/block/loop.c
> > > +++ b/drivers/block/loop.c
> > > @@ -397,6 +397,9 @@ static int lo_rw_aio(struct loop_device *lo, struct loop_cmd *cmd,
> > > cmd->iocb.ki_flags = 0;
> > > }
> > > + if (!file)
> > > + return -EIO;
> > If lo->lo_backing_file may become NULL, it isn't enough to add check here
> > only, cause it can be observed in lo_req_flush() & lo_fallocate() too.
> >
> >
> > Thanks,
> > Ming
> You're absolutely right. Tetsuo's approach only adds logging to help
> diagnose the issue (and he marked it "not for upstream"), but it doesn't
> prevent the crash.
>
> To properly fix this, I need to add NULL checks to all functions that
> dereference lo->lo_backing_file:
> - lo_rw_aio()
> - lo_req_flush()
> - lo_fallocate()
> - loop_update_limits()
> - lo_can_use_dio()
>
> What's your preferred approach?
> 1. Simple NULL checks in all functions
> 2. A helper function with proper locking
> 3. Your recommendation
>
> Let me know which approach you think is best, and I'll update the patch
> accordingly.
It isn't supposed to happen, so please root-cause first before sending
any workaround.
Thanks,
Ming
^ permalink raw reply
* Re: [PATCH v3 05/10] lib: add dmabuf token infrastructure
From: Pavel Begunkov @ 2026-05-19 7:55 UTC (permalink / raw)
To: Christoph Hellwig
Cc: 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: <20260519065653.GB8173@lst.de>
On 5/19/26 07:56, Christoph Hellwig wrote:
> On Mon, May 18, 2026 at 03:23:53PM +0100, Pavel Begunkov wrote:
>> To be fair, it's not that dma-buf specific. This lib/ code only
>> does some resv locking, fence waiting and queuing fences,
>
> But all the dma resv/fence stuff is pretty tied into the dma-buf
> ecosystem. I don't think it would really apply to something not
> doing DMA at all.
The point is that those can be separated to reuse the rest.
>> otherwise
>> all the attaching is done by the driver behind callbacks. Switching
>> it to some memfd could be pretty simple. But The main thing it'd
>> need to share is iterator handling like forwarding in the block
>> layer, and it should be fine as it's already passed as a completely
>> opaque object with no knowledge about pages / dma / etc. for the
>> middle layers.
>
> But none of that really sits in the current lib/ code anyway?
It's about naming. E.g. passing a DMABUF_ITER that doesn't have a
dma-buf would be confusing, and then it'll need renaming at all
layers to support the use case.
>>> lib/ is most certainly the wrong place for something that absolutely
>>> is not library functionality but directly interacts with a few
>>> subsystems.
>>
>> It only interacts with dma-buf, and even for dma-buf attachments
>> are created by the driver. Block, nvme, io_uring are users, either
>> using the helpers or implementing callbacks.
>>
>> Ok. Let's assume for the argument's sake it's not dma-buf
>> specific, if not lib/, where would you put it? I was also
>> assuming that dma-buf being under drivers/ is rather a relic
>> of the past rather than the desired location, hmm?
>
> drivers/dma-buf is a pretty natural place for it, I could not thing
_If_ there is no dma mappings, drivers/dma-buf would definitely
be an awkward spot. Just trying to understand your criteria for
placement, let's say of a generic buffer registration code assuming
there is no dma-buf involved at all. Again, just a hypothetical.
> where else you'd place dma-buffers. I'm not sure how hmm has anything
> to do with it.
Looks there is some confusion. It's was meant as an interjection
with an open question, I didn't mention the HMM subsystem.
--
Pavel Begunkov
^ permalink raw reply
* [PATCH] tracing/blktrace: Use sysfs_emit() for sysfs show callbacks
From: Yu Peng @ 2026-05-19 7:50 UTC (permalink / raw)
To: Jens Axboe, Steven Rostedt, Masami Hiramatsu
Cc: Mathieu Desnoyers, linux-block, linux-kernel, linux-trace-kernel,
Yu Peng
Use sysfs_emit() and sysfs_emit_at() instead of sprintf() when
formatting blktrace sysfs show output.
No functional change intended.
Signed-off-by: Yu Peng <pengyu@kylinos.cn>
---
kernel/trace/blktrace.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index 8cd2520b4c99e..1eda8158883ca 100644
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -2025,11 +2025,11 @@ static ssize_t blk_trace_mask2str(char *buf, int mask)
for (i = 0; i < ARRAY_SIZE(mask_maps); i++) {
if (mask & mask_maps[i].mask) {
- p += sprintf(p, "%s%s",
+ p += sysfs_emit_at(buf, p - buf, "%s%s",
(p == buf) ? "" : ",", mask_maps[i].str);
}
}
- *p++ = '\n';
+ p += sysfs_emit_at(buf, p - buf, "\n");
return p - buf;
}
@@ -2048,20 +2048,20 @@ static ssize_t sysfs_blk_trace_attr_show(struct device *dev,
bt = rcu_dereference_protected(q->blk_trace,
lockdep_is_held(&q->debugfs_mutex));
if (attr == &dev_attr_enable) {
- ret = sprintf(buf, "%u\n", !!bt);
+ ret = sysfs_emit(buf, "%u\n", !!bt);
goto out_unlock_bdev;
}
if (bt == NULL)
- ret = sprintf(buf, "disabled\n");
+ ret = sysfs_emit(buf, "disabled\n");
else if (attr == &dev_attr_act_mask)
ret = blk_trace_mask2str(buf, bt->act_mask);
else if (attr == &dev_attr_pid)
- ret = sprintf(buf, "%u\n", bt->pid);
+ ret = sysfs_emit(buf, "%u\n", bt->pid);
else if (attr == &dev_attr_start_lba)
- ret = sprintf(buf, "%llu\n", bt->start_lba);
+ ret = sysfs_emit(buf, "%llu\n", bt->start_lba);
else if (attr == &dev_attr_end_lba)
- ret = sprintf(buf, "%llu\n", bt->end_lba);
+ ret = sysfs_emit(buf, "%llu\n", bt->end_lba);
out_unlock_bdev:
blk_debugfs_unlock_nomemrestore(q);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v3 05/10] lib: add dmabuf token infrastructure
From: Christoph Hellwig @ 2026-05-19 6:56 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: <ea47051e-697f-4017-a514-be6ef7c110e9@gmail.com>
On Mon, May 18, 2026 at 03:23:53PM +0100, Pavel Begunkov wrote:
> To be fair, it's not that dma-buf specific. This lib/ code only
> does some resv locking, fence waiting and queuing fences,
But all the dma resv/fence stuff is pretty tied into the dma-buf
ecosystem. I don't think it would really apply to something not
doing DMA at all.
> otherwise
> all the attaching is done by the driver behind callbacks. Switching
> it to some memfd could be pretty simple. But The main thing it'd
> need to share is iterator handling like forwarding in the block
> layer, and it should be fine as it's already passed as a completely
> opaque object with no knowledge about pages / dma / etc. for the
> middle layers.
But none of that really sits in the current lib/ code anyway?
>> lib/ is most certainly the wrong place for something that absolutely
>> is not library functionality but directly interacts with a few
>> subsystems.
>
> It only interacts with dma-buf, and even for dma-buf attachments
> are created by the driver. Block, nvme, io_uring are users, either
> using the helpers or implementing callbacks.
>
> Ok. Let's assume for the argument's sake it's not dma-buf
> specific, if not lib/, where would you put it? I was also
> assuming that dma-buf being under drivers/ is rather a relic
> of the past rather than the desired location, hmm?
drivers/dma-buf is a pretty natural place for it, I could not thing
where else you'd place dma-buffers. I'm not sure how hmm has anything
to do with it.
>
>
> --
> Pavel Begunkov
---end quoted text---
^ permalink raw reply
* Re: [PATCH] nvme-multipath: set BIO_REMAPPED on bios remapped to per-path namespace disks
From: hch @ 2026-05-19 6:53 UTC (permalink / raw)
To: Keith Busch
Cc: Achkinazi, Igor, hch@lst.de, sagi@grimberg.me, axboe@kernel.dk,
linux-nvme@lists.infradead.org, linux-block@vger.kernel.org,
linux-kernel@vger.kernel.org, stable@vger.kernel.org
In-Reply-To: <agtnJb5a5uIqH-65@kbusch-mbp>
On Mon, May 18, 2026 at 01:23:17PM -0600, Keith Busch wrote:
> On Mon, May 18, 2026 at 02:52:56PM +0000, Achkinazi, Igor wrote:
> > @@ -511,6 +511,13 @@ static void nvme_ns_head_submit_bio(struct bio *bio)
> > ns = nvme_find_path(head);
> > if (likely(ns)) {
> > bio_set_dev(bio, ns->disk->part0);
> > + /*
> > + * Mark the bio as remapped to the per-path namespace disk so
> > + * that bio_check_eod() is skipped on resubmission (e.g. from
> > + * bio splitting in blk_mq_submit_bio). The EOD check already
> > + * passed on the multipath head disk.
> > + */
> > + bio_set_flag(bio, BIO_REMAPPED);
>
> Any reason nvme multipath can't call submit_bio_noacct_nocheck()
> directly instead? If it's safe to skip the eod check here, then it
> looks safe to skip everything else too.
We really shouldn't expose that, and it doesn't quite to the
right checks here.
But I think the proper fix is to stop using bio_set_dev entirely.
We do not want to associate the bio with a new blkg as those are
basically not going to exist for the underlying devices. I'm also
not sure we want to allow another round of BPF throttling either.
^ permalink raw reply
* Re: [PATCHv2] blk-mq: check for stale cached request in blk_mq_submit_bio
From: Christoph Hellwig @ 2026-05-19 6:47 UTC (permalink / raw)
To: Keith Busch; +Cc: Keith Busch, axboe, hch, linux-block
In-Reply-To: <agslvk4DLD0OOVLR@kbusch-mbp>
On Mon, May 18, 2026 at 08:44:14AM -0600, Keith Busch wrote:
> On Fri, May 01, 2026 at 10:41:19AM -0700, Keith Busch 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 ensuring the cached_rqs still contains
> > our peeked request, and retry the bio submission without it if the
> > request had been freed.
>
> Any thoughts on this one? I know it's an old bug, but I've only recently
> started seeing it happen for some reason.
I still think the only proper fix is to always hold a queue ref over
the checks. Correctness first and then look into optimizing that
with hazard pointers or whatever. Below is the patch I did during
LSF/MM, refeshed using a commit log mostly stolen from you:
---
From e1bff09d6b594f777624240cefd70861137c45b4 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <hch@lst.de>
Date: Tue, 19 May 2026 06:49:12 +0200
Subject: blk-mq: always take a queue reference in blk_mq_submit_bio
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 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
* [bug report] drbd: replace genl_magic with explicit netlink serialization
From: Dan Carpenter @ 2026-05-19 6:24 UTC (permalink / raw)
To: Christoph Böhmwalder; +Cc: drbd-dev, linux-block
Hello Christoph Böhmwalder,
Commit 8098eeb693c4 ("drbd: replace genl_magic with explicit netlink
serialization") from May 6, 2026 (linux-next), leads to the following
Smatch static checker warning:
drivers/block/drbd/drbd_nl_gen.c:1859 drbd_cfg_reply_to_skb()
warn: unsigned 's->info_text_len' is never less than zero.
drivers/block/drbd/drbd_nl_gen.c
1852 int drbd_cfg_reply_to_skb(struct sk_buff *skb, struct drbd_cfg_reply *s)
1853 {
1854 struct nlattr *tla = nla_nest_start(skb, DRBD_NLA_CFG_REPLY);
1855
1856 if (!tla)
1857 goto nla_put_failure;
1858
--> 1859 if (nla_put(skb, DRBD_A_DRBD_CFG_REPLY_INFO_TEXT, min_t(int, 0,
1860 s->info_text_len + (s->info_text_len < 0)), s->info_text))
^^^^^^^^^^^^^^^^^^^^
This is a u32 so it can't be negative.
1861 goto nla_put_failure;
1862
1863 nla_nest_end(skb, tla);
1864 return 0;
1865
1866 nla_put_failure:
1867 if (tla)
1868 nla_nest_cancel(skb, tla);
1869 return -EMSGSIZE;
1870 }
This email is a free service from the Smatch-CI project [smatch.sf.net].
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH] block/loop: Fix NULL pointer dereference in lo_rw_aio()
From: Hongling Zeng @ 2026-05-19 6:06 UTC (permalink / raw)
To: Ming Lei, Hongling Zeng; +Cc: axboe, ming.lei, linux-block, linux-kernel
In-Reply-To: <agvqK_4EelxnXUvf@fedora>
Hi,
Thank you for the review.
在 2026年05月19日 12:42, Ming Lei 写道:
> On Tue, May 19, 2026 at 12:01:03PM +0800, Hongling Zeng wrote:
>> lo->lo_backing_file can be NULL when the loop device is being cleared,
>> causing NULL pointer dereference in lo_rw_aio(). Add a defensive check
>> to prevent kernel crash.
>>
>> In v7.1-rc tree, lo->lo_backing_file is cleared in __loop_clr_fd(), which
>> is called from lo_release() only, when it is guaranteed that there isn't
>> inflight IO.
>>
>> So care to share your stack trace?
Syzkaller has managed to trigger NULL pointer dereference in lo_rw_aio().
Tetsuo Handa encountered the same issue (commit 1a83d226268cf), but his
patch only adds diagnostic logging and is marked "not for upstream".
>> Also fix loop_attr_backing_file_show() to use PTR_ERR_OR_ZERO()
>> for correct NULL pointer handling.
>>
>> Fixes: bc07c10a3603a ("block: loop: support DIO & AIO")
> Can you explain how the issue is introduced in above commit?
The issue was introduced when lo_rw_aio() was added in that commit,
which dereferences lo->lo_backing_file without NULL checks. While the
design assumes no inflight IO during __loop_clr_fd(), fuzzers have
found edge cases where this assumption doesn't hold.
>> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
>> ---
>> drivers/block/loop.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
>> index 0000913f7efc..d8db1b0d8018 100644
>> --- a/drivers/block/loop.c
>> +++ b/drivers/block/loop.c
>> @@ -397,6 +397,9 @@ static int lo_rw_aio(struct loop_device *lo, struct loop_cmd *cmd,
>> cmd->iocb.ki_flags = 0;
>> }
>>
>> + if (!file)
>> + return -EIO;
> If lo->lo_backing_file may become NULL, it isn't enough to add check here
> only, cause it can be observed in lo_req_flush() & lo_fallocate() too.
>
>
> Thanks,
> Ming
You're absolutely right. Tetsuo's approach only adds logging to help
diagnose the issue (and he marked it "not for upstream"), but it doesn't
prevent the crash.
To properly fix this, I need to add NULL checks to all functions that
dereference lo->lo_backing_file:
- lo_rw_aio()
- lo_req_flush()
- lo_fallocate()
- loop_update_limits()
- lo_can_use_dio()
What's your preferred approach?
1. Simple NULL checks in all functions
2. A helper function with proper locking
3. Your recommendation
Let me know which approach you think is best, and I'll update the patch
accordingly.
Regards,
Hongling
~
^ permalink raw reply
* Re: [PATCH] block/loop: Fix NULL pointer dereference in lo_rw_aio()
From: Ming Lei @ 2026-05-19 4:42 UTC (permalink / raw)
To: Hongling Zeng; +Cc: axboe, ming.lei, linux-block, linux-kernel, zhongling0719
In-Reply-To: <20260519040103.22776-1-zenghongling@kylinos.cn>
On Tue, May 19, 2026 at 12:01:03PM +0800, Hongling Zeng wrote:
> lo->lo_backing_file can be NULL when the loop device is being cleared,
> causing NULL pointer dereference in lo_rw_aio(). Add a defensive check
> to prevent kernel crash.
In v7.1-rc tree, lo->lo_backing_file is cleared in __loop_clr_fd(), which
is called from lo_release() only, when it is guaranteed that there isn't
inflight IO.
So care to share your stack trace?
>
> Also fix loop_attr_backing_file_show() to use PTR_ERR_OR_ZERO()
> for correct NULL pointer handling.
>
> Fixes: bc07c10a3603a ("block: loop: support DIO & AIO")
Can you explain how the issue is introduced in above commit?
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> ---
> drivers/block/loop.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index 0000913f7efc..d8db1b0d8018 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -397,6 +397,9 @@ static int lo_rw_aio(struct loop_device *lo, struct loop_cmd *cmd,
> cmd->iocb.ki_flags = 0;
> }
>
> + if (!file)
> + return -EIO;
If lo->lo_backing_file may become NULL, it isn't enough to add check here
only, cause it can be observed in lo_req_flush() & lo_fallocate() too.
Thanks,
Ming
^ permalink raw reply
* [PATCH] block/loop: Fix NULL pointer dereference in lo_rw_aio()
From: Hongling Zeng @ 2026-05-19 4:01 UTC (permalink / raw)
To: axboe, ming.lei; +Cc: linux-block, linux-kernel, zhongling0719, Hongling Zeng
lo->lo_backing_file can be NULL when the loop device is being cleared,
causing NULL pointer dereference in lo_rw_aio(). Add a defensive check
to prevent kernel crash.
Also fix loop_attr_backing_file_show() to use PTR_ERR_OR_ZERO()
for correct NULL pointer handling.
Fixes: bc07c10a3603a ("block: loop: support DIO & AIO")
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
drivers/block/loop.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 0000913f7efc..d8db1b0d8018 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -397,6 +397,9 @@ static int lo_rw_aio(struct loop_device *lo, struct loop_cmd *cmd,
cmd->iocb.ki_flags = 0;
}
+ if (!file)
+ return -EIO;
+
if (rw == ITER_SOURCE) {
kiocb_start_write(&cmd->iocb);
ret = file->f_op->write_iter(&cmd->iocb, &iter);
@@ -662,7 +665,7 @@ static ssize_t loop_attr_backing_file_show(struct loop_device *lo, char *buf)
spin_unlock_irq(&lo->lo_lock);
if (IS_ERR_OR_NULL(p))
- ret = PTR_ERR(p);
+ ret = PTR_ERR_OR_ZERO(p);
else {
ret = strlen(p);
memmove(buf, p, ret);
--
2.25.1
^ permalink raw reply related
* Re: [PATCHv2] blk-mq: check for stale cached request in blk_mq_submit_bio
From: Ming Lei @ 2026-05-19 3:36 UTC (permalink / raw)
To: Keith Busch; +Cc: Keith Busch, axboe, hch, linux-block
In-Reply-To: <agvT6mMM5Tc2ABbk@kbusch-mbp>
On Mon, May 18, 2026 at 09:07:22PM -0600, Keith Busch wrote:
> On Tue, May 19, 2026 at 09:24:50AM +0800, Ming Lei wrote:
> > On Fri, May 01, 2026 at 10:41:19AM -0700, Keith Busch wrote:
> > >
> > > new_request:
> > > if (rq) {
> > > - blk_mq_use_cached_rq(rq, plug, bio);
> > > + if (!blk_mq_use_cached_rq(rq, plug, bio)) {
> > > + struct bio_integrity_payload *bip = bio_integrity(bio);
> > > +
> > > + if (bip && (bip->bip_flags & BIP_BLOCK_INTEGRITY))
> > > + bio_integrity_free(bio);
> > > + rq = NULL;
> > > + goto retry;
> > > + }
> >
> > This way still looks fragile, given rq isn't guaranteed to be live.
>
> Yeah. The current usage in this patch looks like it should work out, but
> not necessarily by design, so yes, fragile.
>
> > Just wondering why not pop it from plug->cached_rqs from beginning, and
> > move it back in case of `queue_exit:`?
>
> Not sure. I guess it's an optimization when the bio merges with a
> previous request in the plug? If we popped initially, a successful merge
> has to push it back. Maybe there's a better reason I'm not seeing.
Yes, this way may avoid potential perf regression for heavy merge cases.
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