public inbox for linux-btrfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vacek <neelx@suse.com>
To: Chris Mason <clm@fb.com>, Josef Bacik <josef@toxicpanda.com>,
	Eric Biggers <ebiggers@kernel.org>,
	"Theodore Y. Ts'o" <tytso@mit.edu>,
	Jaegeuk Kim <jaegeuk@kernel.org>, Jens Axboe <axboe@kernel.dk>,
	David Sterba <dsterba@suse.com>
Cc: linux-block@vger.kernel.org, Daniel Vacek <neelx@suse.com>,
	linux-fscrypt@vger.kernel.org, linux-btrfs@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH v6 33/43] btrfs: implement read repair for encryption
Date: Fri,  6 Feb 2026 19:23:05 +0100	[thread overview]
Message-ID: <20260206182336.1397715-34-neelx@suse.com> (raw)
In-Reply-To: <20260206182336.1397715-1-neelx@suse.com>

From: Josef Bacik <josef@toxicpanda.com>

In order to do read repair we will allocate sectorsize bio's and read
them one at a time, repairing any sectors that don't match their csum.
In order to do this we re-submit the IO's after it's failed, and at this
point we still need the fscrypt_extent_info for these new bio's.

Add the fscrypt_extent_info to the read part of the union in the
btrfs_bio, and then pass this through all the places where we do reads.
Additionally add the orig_start, because we need to be able to put the
correct extent offset for the encryption context.

With these in place we can utilize the normal read repair path.  The
only exception is that the actual repair of the bad copies has to be
triggered from the ->process_bio callback, because this is the encrypted
data.  If we waited until the end_io we would have the decrypted data
and we don't want to write that to the disk.  This is the only change to
the normal read repair path, we trigger the fixup of the broken sectors
in ->process_bio, and then we skip that part if we successfully repair
the sector in ->process_bio once we get to the endio.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Daniel Vacek <neelx@suse.com>
---

v5: https://lore.kernel.org/linux-btrfs/310c0ebdc78613b6f379595e160206013f75b6dc.1706116485.git.josef@toxicpanda.com/
 * Fixed UAF bug with !ordered case doing bbio->end_io(bbio) >>>
   fscrypt_put_extent_info(bbio->fscrypt_info).
   - We can simply put the fscrypt_info first and then end the bio.
   - Also no need to clear the bbio->fscrypt_info pointer as bbio is
     just going to be freed.  That cleans up the code a bit.
 * Adapted to bs > ps changes.
 * Updated and re-wrap the comments.
 * Moved the dio-related changes from inode.c to direct-io.c
   as upstream refactored in the meantime.
---
 fs/btrfs/bio.c         | 75 +++++++++++++++++++++++++++++++++++++-----
 fs/btrfs/bio.h         | 10 +++++-
 fs/btrfs/compression.c |  2 ++
 fs/btrfs/direct-io.c   |  2 ++
 fs/btrfs/extent_io.c   |  2 ++
 5 files changed, 82 insertions(+), 9 deletions(-)

diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
index 69dc32cb4ed6..a89f49dac0f2 100644
--- a/fs/btrfs/bio.c
+++ b/fs/btrfs/bio.c
@@ -97,6 +97,9 @@ static struct btrfs_bio *btrfs_split_bio(struct btrfs_fs_info *fs_info,
 		bbio->ordered = orig_bbio->ordered;
 		bbio->orig_logical = orig_bbio->orig_logical;
 		orig_bbio->orig_logical += map_length;
+	} else if (is_data_bbio(bbio)) {
+		bbio->fscrypt_info = fscrypt_get_extent_info(orig_bbio->fscrypt_info);
+		bbio->orig_start = orig_bbio->orig_start;
 	}
 
 	bbio->csum_search_commit_root = orig_bbio->csum_search_commit_root;
@@ -124,6 +127,8 @@ void btrfs_bio_end_io(struct btrfs_bio *bbio, blk_status_t status)
 		/* Free bio that was never submitted to the underlying device. */
 		if (bbio_has_ordered_extent(bbio))
 			btrfs_put_ordered_extent(bbio->ordered);
+		else if (is_data_bbio(bbio))
+			fscrypt_put_extent_info(bbio->fscrypt_info);
 		bio_put(&bbio->bio);
 
 		bbio = orig_bbio;
@@ -147,6 +152,8 @@ void btrfs_bio_end_io(struct btrfs_bio *bbio, blk_status_t status)
 			bbio->end_io(bbio);
 			btrfs_put_ordered_extent(ordered);
 		} else {
+			if (is_data_bbio(bbio))
+				fscrypt_put_extent_info(bbio->fscrypt_info);
 			bbio->end_io(bbio);
 		}
 	}
@@ -174,6 +181,23 @@ static void btrfs_repair_done(struct btrfs_failed_bio *fbio)
 	}
 }
 
+static void handle_repair(struct btrfs_bio *repair_bbio, phys_addr_t *paddrs)
+{
+	struct btrfs_failed_bio *fbio = repair_bbio->private;
+	struct btrfs_inode *inode = repair_bbio->inode;
+	struct btrfs_fs_info *fs_info = inode->root->fs_info;
+	const u32 step = min(fs_info->sectorsize, PAGE_SIZE);
+	const u64 logical = repair_bbio->saved_iter.bi_sector << SECTOR_SHIFT;
+	int mirror = repair_bbio->mirror_num;
+
+	do {
+		mirror = prev_repair_mirror(fbio, mirror);
+		btrfs_repair_io_failure(fs_info, btrfs_ino(inode),
+				  repair_bbio->file_offset, fs_info->sectorsize,
+				  logical, paddrs, step, mirror);
+	} while (mirror != fbio->bbio->mirror_num);
+}
+
 static void btrfs_end_repair_bio(struct btrfs_bio *repair_bbio,
 				 struct btrfs_device *dev)
 {
@@ -186,7 +210,6 @@ static void btrfs_end_repair_bio(struct btrfs_bio *repair_bbio,
 	 */
 	struct bvec_iter saved_iter = repair_bbio->saved_iter;
 	const u32 step = min(fs_info->sectorsize, PAGE_SIZE);
-	const u64 logical = repair_bbio->saved_iter.bi_sector << SECTOR_SHIFT;
 	const u32 nr_steps = repair_bbio->saved_iter.bi_size / step;
 	int mirror = repair_bbio->mirror_num;
 	phys_addr_t paddrs[BTRFS_MAX_BLOCKSIZE / PAGE_SIZE];
@@ -202,6 +225,13 @@ static void btrfs_end_repair_bio(struct btrfs_bio *repair_bbio,
 		slot++;
 	}
 
+	/*
+	 * If we got here from the encrypted path with ->csum_ok set then
+	 * we've already csumed and repaired this sector, we're all done.
+	 */
+	if (repair_bbio->csum_ok)
+		goto done;
+
 	if (repair_bbio->bio.bi_status ||
 	    !btrfs_data_csum_ok(repair_bbio, dev, 0, paddrs)) {
 		bio_reset(&repair_bbio->bio, NULL, REQ_OP_READ);
@@ -214,17 +244,17 @@ static void btrfs_end_repair_bio(struct btrfs_bio *repair_bbio,
 			goto done;
 		}
 
+		btrfs_set_bio_crypt_ctx_from_extent(&repair_bbio->bio,
+						    repair_bbio->inode,
+						    repair_bbio->fscrypt_info,
+						    repair_bbio->file_offset -
+						    repair_bbio->orig_start);
+
 		btrfs_submit_bbio(repair_bbio, mirror);
 		return;
 	}
 
-	do {
-		mirror = prev_repair_mirror(fbio, mirror);
-		btrfs_repair_io_failure(fs_info, btrfs_ino(inode),
-				  repair_bbio->file_offset, fs_info->sectorsize,
-				  logical, paddrs, step, mirror);
-	} while (mirror != fbio->bbio->mirror_num);
-
+	handle_repair(repair_bbio, paddrs);
 done:
 	btrfs_repair_done(fbio);
 	bio_put(&repair_bbio->bio);
@@ -293,6 +323,13 @@ static struct btrfs_failed_bio *repair_one_sector(struct btrfs_bio *failed_bbio,
 	repair_bbio = btrfs_bio(repair_bio);
 	btrfs_bio_init(repair_bbio, failed_bbio->inode, failed_bbio->file_offset + bio_offset,
 		       NULL, fbio);
+	repair_bbio->fscrypt_info = fscrypt_get_extent_info(failed_bbio->fscrypt_info);
+	repair_bbio->orig_start = failed_bbio->orig_start;
+
+	btrfs_set_bio_crypt_ctx_from_extent(repair_bio, repair_bbio->inode,
+					    failed_bbio->fscrypt_info,
+					    repair_bbio->file_offset -
+					    failed_bbio->orig_start);
 
 	mirror = next_repair_mirror(fbio, failed_bbio->mirror_num);
 	btrfs_debug(fs_info, "submitting repair read to mirror %d", mirror);
@@ -324,7 +361,29 @@ blk_status_t btrfs_check_encrypted_read_bio(struct btrfs_bio *bbio, struct bio *
 	if (!btrfs_data_csum_ok(bbio, dev, 0, paddrs))
 			return BLK_STS_IOERR;
 
+	/*
+	 * Read repair is slightly different for encrypted bio's.  This
+	 * callback is before we decrypt the bio in the block crypto layer,
+	 * we're not actually in the endio handler.
+	 *
+	 * We don't trigger the repair process here either, that is handled
+	 * in the actual endio path because we don't want to create another
+	 * pseudo endio path through this callback.  This is because when we
+	 * call btrfs_repair_done() we want to call the endio for the original
+	 * bbio. Short circuiting that for the encrypted case would be ugly.
+	 * We really want to the repair case to be handled generically.
+	 *
+	 * However for the actual repair part we need to use this page
+	 * pre-decrypted, which is why we call the btrfs_repair_io_failure()
+	 * code from this path.  The repair path is synchronous so we are
+	 * safe there.  Then we simply mark the repair bbio as completed so
+	 * the actual btrfs_end_repair_bio() code can skip the repair part.
+	 */
+	if (bbio->bio.bi_pool == &btrfs_repair_bioset)
+		handle_repair(bbio, paddrs);
 	bbio->csum_ok = true;
+	fscrypt_put_extent_info(bbio->fscrypt_info);
+	bbio->fscrypt_info = NULL;
 	return BLK_STS_OK;
 }
 
diff --git a/fs/btrfs/bio.h b/fs/btrfs/bio.h
index 456d32db9e9e..7a8ff4378cba 100644
--- a/fs/btrfs/bio.h
+++ b/fs/btrfs/bio.h
@@ -15,6 +15,7 @@
 struct btrfs_bio;
 struct btrfs_fs_info;
 struct btrfs_inode;
+struct fscrypt_extent_info;
 
 #define BTRFS_BIO_INLINE_CSUM_SIZE	64
 
@@ -38,13 +39,20 @@ struct btrfs_bio {
 	union {
 		/*
 		 * For data reads: checksumming and original I/O information.
-		 * (for internal use in the btrfs_submit_bbio() machinery only)
+		 * (for internal use in the btrfs_submit_bbio() machinery only).
+		 *
+		 * The fscrypt context is used for read repair, this is the
+		 * only thing not internal to btrfs_submit_bbio() machinery.
 		 */
 		struct {
 			u8 *csum;
 			u8 csum_inline[BTRFS_BIO_INLINE_CSUM_SIZE];
 			bool csum_ok;
 			struct bvec_iter saved_iter;
+
+			/* Used for read repair. */
+			struct fscrypt_extent_info *fscrypt_info;
+			u64 orig_start;
 		};
 
 		/*
diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index cd7b245bedb8..15bf03be3fd5 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -606,6 +606,8 @@ void btrfs_submit_compressed_read(struct btrfs_bio *bbio)
 	cb->compress_type = btrfs_extent_map_compression(em);
 	cb->orig_bbio = bbio;
 	cb->bbio.csum_search_commit_root = bbio->csum_search_commit_root;
+	cb->bbio.fscrypt_info = fscrypt_get_extent_info(em->fscrypt_info);
+	cb->bbio.orig_start = 0;
 
 	btrfs_set_bio_crypt_ctx_from_extent(&cb->bbio.bio, inode, em->fscrypt_info, 0);
 	btrfs_free_extent_map(em);
diff --git a/fs/btrfs/direct-io.c b/fs/btrfs/direct-io.c
index d20a5b99bdde..c59e13fce764 100644
--- a/fs/btrfs/direct-io.c
+++ b/fs/btrfs/direct-io.c
@@ -765,6 +765,8 @@ static void btrfs_dio_submit_io(const struct iomap_iter *iter, struct bio *bio,
 	} else {
 		fscrypt_info = dio_data->fscrypt_info;
 		offset = file_offset - dio_data->orig_start;
+		bbio->fscrypt_info = fscrypt_get_extent_info(fscrypt_info);
+		bbio->orig_start = dio_data->orig_start;
 	}
 
 	btrfs_set_bio_crypt_ctx_from_extent(&bbio->bio, bbio->inode, fscrypt_info, offset);
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 3273b7e3b4b0..094855b77768 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -796,6 +796,8 @@ static void alloc_new_bio(struct btrfs_inode *inode,
 	} else {
 		fscrypt_info = bio_ctrl->fscrypt_info;
 		offset = file_offset - bio_ctrl->orig_start;
+		bbio->fscrypt_info = fscrypt_get_extent_info(fscrypt_info);
+		bbio->orig_start = bio_ctrl->orig_start;
 	}
 
 	btrfs_set_bio_crypt_ctx_from_extent(&bbio->bio, inode, fscrypt_info, offset);
-- 
2.51.0


  parent reply	other threads:[~2026-02-06 18:25 UTC|newest]

Thread overview: 77+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-06 18:22 [PATCH v6 00/43] btrfs: add fscrypt support Daniel Vacek
2026-02-06 18:22 ` [PATCH v6 01/43] fscrypt: add per-extent encryption support Daniel Vacek
2026-02-21 22:11   ` Eric Biggers
2026-02-06 18:22 ` [PATCH v6 02/43] fscrypt: allow inline encryption for extent based encryption Daniel Vacek
2026-02-06 18:22 ` [PATCH v6 03/43] fscrypt: add a __fscrypt_file_open helper Daniel Vacek
2026-02-06 18:22 ` [PATCH v6 04/43] fscrypt: conditionally don't wipe mk secret until the last active user is done Daniel Vacek
2026-02-06 18:22 ` [PATCH v6 05/43] blk-crypto: add a process_bio callback Daniel Vacek
2026-02-06 18:22 ` [PATCH v6 06/43] fscrypt: add a process_bio hook to fscrypt_operations Daniel Vacek
2026-02-06 18:22 ` [PATCH v6 07/43] fscrypt: expose fscrypt_nokey_name Daniel Vacek
2026-02-06 18:22 ` [PATCH v6 08/43] fscrypt: add documentation about extent encryption Daniel Vacek
2026-02-06 18:43   ` Randy Dunlap
2026-02-17 14:48     ` Daniel Vacek
2026-02-06 18:22 ` [PATCH v6 09/43] btrfs: add infrastructure for safe em freeing Daniel Vacek
2026-02-06 18:22 ` [PATCH v6 10/43] btrfs: start using fscrypt hooks Daniel Vacek
2026-02-08 15:44   ` Chris Mason
2026-02-17 15:26     ` Daniel Vacek
2026-02-06 18:22 ` [PATCH v6 11/43] btrfs: add inode encryption contexts Daniel Vacek
2026-02-08 15:36   ` Chris Mason
2026-02-18 13:18     ` Daniel Vacek
2026-02-06 18:22 ` [PATCH v6 12/43] btrfs: add new FEATURE_INCOMPAT_ENCRYPT flag Daniel Vacek
2026-02-06 18:22 ` [PATCH v6 13/43] btrfs: adapt readdir for encrypted and nokey names Daniel Vacek
2026-02-08 15:35   ` Chris Mason
2026-02-18 14:05     ` Daniel Vacek
2026-02-06 18:22 ` [PATCH v6 14/43] btrfs: handle " Daniel Vacek
2026-02-08 15:28   ` Chris Mason
2026-02-18 14:50     ` Daniel Vacek
2026-02-06 18:22 ` [PATCH v6 15/43] btrfs: implement fscrypt ioctls Daniel Vacek
2026-02-06 18:22 ` [PATCH v6 16/43] btrfs: select encryption dependencies if FS_ENCRYPTION Daniel Vacek
2026-02-08 15:22   ` Chris Mason
2026-02-18 15:02     ` Daniel Vacek
2026-02-06 18:22 ` [PATCH v6 17/43] btrfs: add get_devices hook for fscrypt Daniel Vacek
2026-02-06 18:22 ` [PATCH v6 18/43] btrfs: set file extent encryption excplicitly Daniel Vacek
2026-02-06 18:22 ` [PATCH v6 19/43] btrfs: add fscrypt_info and encryption_type to extent_map Daniel Vacek
2026-02-06 18:22 ` [PATCH v6 20/43] btrfs: add fscrypt_info and encryption_type to ordered_extent Daniel Vacek
2026-02-08 15:18   ` Chris Mason
2026-02-18 15:29     ` Daniel Vacek
2026-02-18 15:50       ` Chris Mason
2026-02-18 16:11         ` Daniel Vacek
2026-02-06 18:22 ` [PATCH v6 21/43] btrfs: plumb through setting the fscrypt_info for ordered extents Daniel Vacek
2026-02-06 18:22 ` [PATCH v6 22/43] btrfs: populate the ordered_extent with the fscrypt context Daniel Vacek
2026-02-06 18:22 ` [PATCH v6 23/43] btrfs: keep track of fscrypt info and orig_start for dio reads Daniel Vacek
2026-02-06 18:22 ` [PATCH v6 24/43] btrfs: add extent encryption context tree item type Daniel Vacek
2026-02-08 15:16   ` Chris Mason
2026-02-18 17:25     ` Daniel Vacek
2026-02-06 18:22 ` [PATCH v6 25/43] btrfs: pass through fscrypt_extent_info to the file extent helpers Daniel Vacek
2026-02-06 18:22 ` [PATCH v6 26/43] btrfs: implement the fscrypt extent encryption hooks Daniel Vacek
2026-02-06 18:22 ` [PATCH v6 27/43] btrfs: setup fscrypt_extent_info for new extents Daniel Vacek
2026-02-06 18:23 ` [PATCH v6 28/43] btrfs: populate ordered_extent with the orig offset Daniel Vacek
2026-02-08 15:12   ` Chris Mason
2026-03-03 13:42     ` Daniel Vacek
2026-02-06 18:23 ` [PATCH v6 29/43] btrfs: set the bio fscrypt context when applicable Daniel Vacek
2026-02-06 18:23 ` [PATCH v6 30/43] btrfs: add a bio argument to btrfs_csum_one_bio Daniel Vacek
2026-02-06 18:23 ` [PATCH v6 31/43] btrfs: limit encrypted writes to 256 segments Daniel Vacek
2026-02-06 18:23 ` [PATCH v6 32/43] btrfs: implement process_bio cb for fscrypt Daniel Vacek
2026-02-08 15:10   ` Chris Mason
2026-03-24  9:36     ` Daniel Vacek
2026-02-06 18:23 ` Daniel Vacek [this message]
2026-02-08 15:08   ` [PATCH v6 33/43] btrfs: implement read repair for encryption Chris Mason
2026-03-25 14:17     ` Daniel Vacek
2026-02-06 18:23 ` [PATCH v6 34/43] btrfs: add test_dummy_encryption support Daniel Vacek
2026-02-06 18:23 ` [PATCH v6 35/43] btrfs: make btrfs_ref_to_path handle encrypted filenames Daniel Vacek
2026-02-08 15:02   ` Chris Mason
2026-03-25 15:27     ` Daniel Vacek
2026-02-06 18:23 ` [PATCH v6 36/43] btrfs: deal with encrypted symlinks in send Daniel Vacek
2026-02-06 18:23 ` [PATCH v6 37/43] btrfs: decrypt file names for send Daniel Vacek
2026-02-06 18:23 ` [PATCH v6 38/43] btrfs: load the inode context before sending writes Daniel Vacek
2026-02-06 18:23 ` [PATCH v6 39/43] btrfs: set the appropriate free space settings in reconfigure Daniel Vacek
2026-02-06 18:23 ` [PATCH v6 40/43] btrfs: support encryption with log replay Daniel Vacek
2026-02-06 18:23 ` [PATCH v6 41/43] btrfs: disable auto defrag on encrypted files Daniel Vacek
2026-02-06 18:23 ` [PATCH v6 42/43] btrfs: disable encryption on RAID5/6 Daniel Vacek
2026-02-08 13:14   ` Chris Mason
2026-02-06 18:23 ` [PATCH v6 43/43] btrfs: disable send if we have encryption enabled Daniel Vacek
2026-02-06 18:42 ` [PATCH v6 00/43] btrfs: add fscrypt support Daniel Vacek
2026-02-21 20:56 ` Eric Biggers
2026-02-27 15:50   ` Daniel Vacek
2026-02-27 22:26     ` Neal Gompa
2026-02-28  7:57       ` Daniel Vacek

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=20260206182336.1397715-34-neelx@suse.com \
    --to=neelx@suse.com \
    --cc=axboe@kernel.dk \
    --cc=clm@fb.com \
    --cc=dsterba@suse.com \
    --cc=ebiggers@kernel.org \
    --cc=jaegeuk@kernel.org \
    --cc=josef@toxicpanda.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-fscrypt@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tytso@mit.edu \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox