All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bart Van Assche <bvanassche@acm.org>
To: Jens Axboe <axboe@kernel.dk>
Cc: linux-block@vger.kernel.org, Eric Biggers <ebiggers@google.com>,
	Keith Busch <kbusch@kernel.org>,
	Christop Hellwig <hch@infradead.org>,
	Bart Van Assche <bvanassche@acm.org>
Subject: [PATCH 3/3] block: Rework splitting of encrypted bios
Date: Wed, 25 Jun 2025 16:42:59 -0700	[thread overview]
Message-ID: <20250625234259.1985366-4-bvanassche@acm.org> (raw)
In-Reply-To: <20250625234259.1985366-1-bvanassche@acm.org>

Modify splitting of encrypted bios as follows:
- Introduce a new block device flag BD_SPLITS_BIO_TO_LIMITS that allows
  bio-based drivers to report that they call bio_split_to_limits() for
  every bio.
- For request-based block drivers and for bio-based block drivers that call
  bio_split_to_limits(), call blk_crypto_bio_prep() after bio splitting
  happened instead of before bio splitting happened.
- For bio-based block drivers of which it is not known whether they call
  bio_split_to_limits(), call blk_crypto_bio_prep() before .submit_bio() is
  called.
- In blk_crypto_fallback_encrypt_bio(), prevent infinite recursion by only
  trying to split a bio if this function is not called from inside
  bio_split_to_limits().
- Since blk_crypto_fallback_encrypt_bio() may clear *bio_ptr, in its caller
  (__blk_crypto_bio_prep()), check whether or not this pointer is NULL.
- In bio_split_rw(), restrict the bio size to the smaller size of what is
  supported to the block driver and the crypto fallback code.

The advantages of these changes are as follows:
- This patch fixes write errors on zoned storage caused by out-of-order
  submission of bios. This out-of-order submission happens if both the
  crypto fallback code and bio_split_to_limits() split a bio.
- Less code duplication. The crypto fallback code now calls
  bio_split_to_limits() instead of open-coding it.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 block/bio.c                 |  3 ++
 block/blk-core.c            | 19 +++++++++----
 block/blk-crypto-fallback.c | 56 +++++++++++++++----------------------
 block/blk-crypto-internal.h |  7 +++++
 block/blk-crypto.c          |  3 +-
 block/blk-merge.c           | 13 +++++++--
 block/blk.h                 |  9 ++++++
 include/linux/blk_types.h   |  1 +
 8 files changed, 69 insertions(+), 42 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index 92c512e876c8..2103612171c0 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1688,6 +1688,9 @@ struct bio *bio_split(struct bio *bio, int sectors,
 	if (!split)
 		return ERR_PTR(-ENOMEM);
 
+	/* Prevent that bio_split_to_limits() calls itself recursively. */
+	bio_set_flag(split, BIO_HAS_BEEN_SPLIT);
+
 	split->bi_iter.bi_size = sectors << 9;
 
 	if (bio_integrity(split))
diff --git a/block/blk-core.c b/block/blk-core.c
index fdac48aec5ef..3a720ec8e6cc 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -626,23 +626,30 @@ static void __submit_bio(struct bio *bio)
 	/* If plug is not used, add new plug here to cache nsecs time. */
 	struct blk_plug plug;
 
-	if (unlikely(!blk_crypto_bio_prep(&bio)))
-		return;
-
 	blk_start_plug(&plug);
 
 	if (!bdev_test_flag(bio->bi_bdev, BD_HAS_SUBMIT_BIO)) {
 		blk_mq_submit_bio(bio);
 	} else if (likely(bio_queue_enter(bio) == 0)) {
 		struct gendisk *disk = bio->bi_bdev->bd_disk;
-	
-		if ((bio->bi_opf & REQ_POLLED) &&
-		    !(disk->queue->limits.features & BLK_FEAT_POLL)) {
+		const blk_features_t features = disk->queue->limits.features;
+
+		/*
+		 * Only call blk_crypto_bio_prep() before .submit_bio() if
+		 * the block driver won't call bio_split_to_limits().
+		 */
+		if (unlikely(!(features & BLK_FEAT_CALLS_BIO_SPLIT_TO_LIMITS) &&
+			     !blk_crypto_bio_prep(&bio)))
+			goto exit_queue;
+
+		if ((bio->bi_opf & REQ_POLLED) && !(features & BLK_FEAT_POLL)) {
 			bio->bi_status = BLK_STS_NOTSUPP;
 			bio_endio(bio);
 		} else {
 			disk->fops->submit_bio(bio);
 		}
+
+exit_queue:
 		blk_queue_exit(disk->queue);
 	}
 
diff --git a/block/blk-crypto-fallback.c b/block/blk-crypto-fallback.c
index 323f59b91a8f..8f9aee6a837a 100644
--- a/block/blk-crypto-fallback.c
+++ b/block/blk-crypto-fallback.c
@@ -213,7 +213,7 @@ blk_crypto_fallback_alloc_cipher_req(struct blk_crypto_keyslot *slot,
  * The encryption fallback code allocates bounce pages individually. Hence this
  * function that calculates an upper limit for the bio size.
  */
-static unsigned int blk_crypto_max_io_size(struct bio *bio)
+unsigned int blk_crypto_max_io_size(struct bio *bio)
 {
 	unsigned int i = 0;
 	unsigned int num_sectors = 0;
@@ -229,29 +229,6 @@ static unsigned int blk_crypto_max_io_size(struct bio *bio)
 	return num_sectors;
 }
 
-static bool blk_crypto_fallback_split_bio_if_needed(struct bio **bio_ptr)
-{
-	struct bio *bio = *bio_ptr;
-	unsigned int num_sectors;
-
-	num_sectors = blk_crypto_max_io_size(bio);
-	if (num_sectors < bio_sectors(bio)) {
-		struct bio *split_bio;
-
-		split_bio = bio_split(bio, num_sectors, GFP_NOIO,
-				      &crypto_bio_split);
-		if (IS_ERR(split_bio)) {
-			bio->bi_status = BLK_STS_RESOURCE;
-			return false;
-		}
-		bio_chain(split_bio, bio);
-		submit_bio_noacct(bio);
-		*bio_ptr = split_bio;
-	}
-
-	return true;
-}
-
 union blk_crypto_iv {
 	__le64 dun[BLK_CRYPTO_DUN_ARRAY_SIZE];
 	u8 bytes[BLK_CRYPTO_MAX_IV_SIZE];
@@ -270,8 +247,10 @@ static void blk_crypto_dun_to_iv(const u64 dun[BLK_CRYPTO_DUN_ARRAY_SIZE],
  * The crypto API fallback's encryption routine.
  * Allocate a bounce bio for encryption, encrypt the input bio using crypto API,
  * and replace *bio_ptr with the bounce bio. May split input bio if it's too
- * large. Returns true on success. Returns false and sets bio->bi_status on
- * error.
+ * large. Returns %true on success. On error, %false is returned and one of
+ * these two actions is taken:
+ * - Either @bio_ptr->bi_status is set and *@bio_ptr is not modified.
+ * - Or bio_endio() is called and *@bio_ptr is changed into %NULL.
  */
 static bool blk_crypto_fallback_encrypt_bio(struct bio **bio_ptr)
 {
@@ -288,11 +267,17 @@ static bool blk_crypto_fallback_encrypt_bio(struct bio **bio_ptr)
 	bool ret = false;
 	blk_status_t blk_st;
 
-	/* Split the bio if it's too big for single page bvec */
-	if (!blk_crypto_fallback_split_bio_if_needed(bio_ptr))
-		return false;
+	if (!bio_flagged(*bio_ptr, BIO_HAS_BEEN_SPLIT)) {
+		/* Split the bio if it's too big for single page bvec */
+		src_bio = bio_split_to_limits(*bio_ptr);
+		if (!src_bio) {
+			*bio_ptr = NULL;
+			return false;
+		}
+	} else {
+		src_bio = *bio_ptr;
+	}
 
-	src_bio = *bio_ptr;
 	bc = src_bio->bi_crypt_context;
 	data_unit_size = bc->bc_key->crypto_cfg.data_unit_size;
 
@@ -488,9 +473,8 @@ static void blk_crypto_fallback_decrypt_endio(struct bio *bio)
  * @bio_ptr: pointer to the bio to prepare
  *
  * If bio is doing a WRITE operation, this splits the bio into two parts if it's
- * too big (see blk_crypto_fallback_split_bio_if_needed()). It then allocates a
- * bounce bio for the first part, encrypts it, and updates bio_ptr to point to
- * the bounce bio.
+ * too big. It then allocates a bounce bio for the first part, encrypts it, and
+ * updates bio_ptr to point to the bounce bio.
  *
  * For a READ operation, we mark the bio for decryption by using bi_private and
  * bi_end_io.
@@ -508,6 +492,12 @@ bool blk_crypto_fallback_bio_prep(struct bio **bio_ptr)
 	struct bio_crypt_ctx *bc = bio->bi_crypt_context;
 	struct bio_fallback_crypt_ctx *f_ctx;
 
+	/*
+	 * Check whether blk_crypto_fallback_bio_prep() has already been called.
+	 */
+	if (bio->bi_end_io == blk_crypto_fallback_encrypt_endio)
+		return true;
+
 	if (WARN_ON_ONCE(!tfms_inited[bc->bc_key->crypto_cfg.crypto_mode])) {
 		/* User didn't call blk_crypto_start_using_key() first */
 		bio->bi_status = BLK_STS_IOERR;
diff --git a/block/blk-crypto-internal.h b/block/blk-crypto-internal.h
index ccf6dff6ff6b..443ba1fd82e6 100644
--- a/block/blk-crypto-internal.h
+++ b/block/blk-crypto-internal.h
@@ -223,6 +223,8 @@ bool blk_crypto_fallback_bio_prep(struct bio **bio_ptr);
 
 int blk_crypto_fallback_evict_key(const struct blk_crypto_key *key);
 
+unsigned int blk_crypto_max_io_size(struct bio *bio);
+
 #else /* CONFIG_BLK_INLINE_ENCRYPTION_FALLBACK */
 
 static inline int
@@ -245,6 +247,11 @@ blk_crypto_fallback_evict_key(const struct blk_crypto_key *key)
 	return 0;
 }
 
+static inline unsigned int blk_crypto_max_io_size(struct bio *bio)
+{
+	return UINT_MAX;
+}
+
 #endif /* CONFIG_BLK_INLINE_ENCRYPTION_FALLBACK */
 
 #endif /* __LINUX_BLK_CRYPTO_INTERNAL_H */
diff --git a/block/blk-crypto.c b/block/blk-crypto.c
index 4b1ad84d1b5a..76278e23193d 100644
--- a/block/blk-crypto.c
+++ b/block/blk-crypto.c
@@ -306,7 +306,8 @@ bool __blk_crypto_bio_prep(struct bio **bio_ptr)
 	if (blk_crypto_fallback_bio_prep(bio_ptr))
 		return true;
 fail:
-	bio_endio(*bio_ptr);
+	if (*bio_ptr)
+		bio_endio(*bio_ptr);
 	return false;
 }
 
diff --git a/block/blk-merge.c b/block/blk-merge.c
index e55a8ec219c9..df65231be543 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -9,6 +9,7 @@
 #include <linux/blk-integrity.h>
 #include <linux/part_stat.h>
 #include <linux/blk-cgroup.h>
+#include <linux/blk-crypto.h>
 
 #include <trace/events/block.h>
 
@@ -125,9 +126,14 @@ static struct bio *bio_submit_split(struct bio *bio, int split_sectors)
 				  bio->bi_iter.bi_sector);
 		WARN_ON_ONCE(bio_zone_write_plugging(bio));
 		submit_bio_noacct(bio);
-		return split;
+
+		bio = split;
 	}
 
+	WARN_ON_ONCE(!bio);
+	if (unlikely(!blk_crypto_bio_prep(&bio)))
+		return NULL;
+
 	return bio;
 error:
 	bio->bi_status = errno_to_blk_status(split_sectors);
@@ -356,9 +362,12 @@ EXPORT_SYMBOL_GPL(bio_split_rw_at);
 struct bio *bio_split_rw(struct bio *bio, const struct queue_limits *lim,
 		unsigned *nr_segs)
 {
+	u32 max_sectors =
+		min(get_max_io_size(bio, lim), blk_crypto_max_io_size(bio));
+
 	return bio_submit_split(bio,
 		bio_split_rw_at(bio, lim, nr_segs,
-			get_max_io_size(bio, lim) << SECTOR_SHIFT));
+				(u64)max_sectors << SECTOR_SHIFT));
 }
 
 /*
diff --git a/block/blk.h b/block/blk.h
index 1141b343d0b5..5f2b43b250df 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -376,6 +376,11 @@ static inline bool bio_may_need_split(struct bio *bio,
 		lim->min_segment_size;
 }
 
+static void bio_clear_split_flag(struct bio **bio)
+{
+	bio_clear_flag(*bio, BIO_HAS_BEEN_SPLIT);
+}
+
 /**
  * __bio_split_to_limits - split a bio to fit the queue limits
  * @bio:     bio to be split
@@ -392,6 +397,10 @@ 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)
 {
+	struct bio *clear_split_flag __cleanup(bio_clear_split_flag) = bio;
+
+	bio_set_flag(bio, BIO_HAS_BEEN_SPLIT);
+
 	switch (bio_op(bio)) {
 	case REQ_OP_READ:
 	case REQ_OP_WRITE:
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 09b99d52fd36..d235e894347a 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -308,6 +308,7 @@ enum {
 	BIO_REMAPPED,
 	BIO_ZONE_WRITE_PLUGGING, /* bio handled through zone write plugging */
 	BIO_EMULATES_ZONE_APPEND, /* bio emulates a zone append operation */
+	BIO_HAS_BEEN_SPLIT,
 	BIO_FLAG_LAST
 };
 

  parent reply	other threads:[~2025-06-25 23:43 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-25 23:42 [PATCH 0/3] Fix bio splitting in the crypto fallback code Bart Van Assche
2025-06-25 23:42 ` [PATCH 1/3] block: Split blk_crypto_fallback_split_bio_if_needed() Bart Van Assche
2025-06-25 23:42 ` [PATCH 2/3] block: Introduce the BLK_FEAT_CALLS_BIO_SPLIT_TO_LIMITS flag Bart Van Assche
2025-06-26  5:24   ` Christop Hellwig
2025-06-25 23:42 ` Bart Van Assche [this message]
2025-06-26  5:28   ` [PATCH 3/3] block: Rework splitting of encrypted bios Christop Hellwig
2025-07-10 18:04     ` Bart Van Assche

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250625234259.1985366-4-bvanassche@acm.org \
    --to=bvanassche@acm.org \
    --cc=axboe@kernel.dk \
    --cc=ebiggers@google.com \
    --cc=hch@infradead.org \
    --cc=kbusch@kernel.org \
    --cc=linux-block@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.