From: Josef Bacik <josef@toxicpanda.com>

For extent encryption we have to use a logical block nr as input for the
IV.  For btrfs we're using the offset into the extent we're operating
on.  For most ordered extents this is the same as the file_offset,
however for prealloc and NOCOW we have to use the original offset.

Add this as an argument and plumb it through everywhere, this will be
used when setting up the bio.

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

v5: https://lore.kernel.org/linux-btrfs/fe06053fe2973c424dd539fecfee8cc171bdd22d.1706116485.git.josef@toxicpanda.com/
 * Splitted the dio-related hunks from inode.c to direct-io.c as upstream
   refactored in the meantime.
 * Open-code orig_start to start - offset.
---
 fs/btrfs/direct-io.c    |  1 +
 fs/btrfs/inode.c        |  5 +++++
 fs/btrfs/ordered-data.c | 21 ++++++++++++++++++---
 fs/btrfs/ordered-data.h |  7 +++++++
 4 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/direct-io.c b/fs/btrfs/direct-io.c
index d3789109ca85..2d89ac05b1b3 100644
--- a/fs/btrfs/direct-io.c
+++ b/fs/btrfs/direct-io.c
@@ -155,6 +155,7 @@ static struct extent_map *btrfs_create_dio_extent(struct btrfs_inode *inode,
 		file_extent->fscrypt_info = em->fscrypt_info;
 	}
 
+	file_extent->orig_offset = start - file_extent->offset;
 	ordered = btrfs_alloc_ordered_extent(inode, start, file_extent,
 					     (1U << type) |
 					     (1U << BTRFS_ORDERED_DIRECT));
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index de1989edffc1..b28e1b7497b8 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -1192,6 +1192,7 @@ static void submit_one_async_extent(struct async_chunk *async_chunk,
 	}
 
 	file_extent.fscrypt_info = em->fscrypt_info;
+	file_extent.orig_offset = start;
 	ordered = btrfs_alloc_ordered_extent(inode, start, &file_extent,
 					     1U << BTRFS_ORDERED_COMPRESSED);
 	btrfs_free_extent_map(em);
@@ -1336,6 +1337,7 @@ static int cow_one_range(struct btrfs_inode *inode, struct folio *locked_folio,
 	}
 
 	file_extent.fscrypt_info = em->fscrypt_info;
+	file_extent.orig_offset = file_offset;
 	ordered = btrfs_alloc_ordered_extent(inode, file_offset, &file_extent,
 					     1U << BTRFS_ORDERED_REGULAR);
 	btrfs_free_extent_map(em);
@@ -2281,6 +2283,8 @@ static noinline int run_delalloc_nocow(struct btrfs_inode *inode,
 			cow_start = (u64)-1;
 		}
 
+		nocow_args.file_extent.orig_offset =
+			found_key.offset - nocow_args.file_extent.offset;
 		ret = nocow_one_range(inode, locked_folio, &cached_state,
 				      &nocow_args, cur_offset,
 				      extent_type == BTRFS_FILE_EXTENT_PREALLOC);
@@ -10218,6 +10222,7 @@ ssize_t btrfs_do_encoded_write(struct kiocb *iocb, struct iov_iter *from,
 	}
 
 	file_extent.fscrypt_info = em->fscrypt_info;
+	file_extent.orig_offset = start - encoded->unencoded_offset;
 	ordered = btrfs_alloc_ordered_extent(inode, start, &file_extent,
 				       (1U << BTRFS_ORDERED_ENCODED) |
 				       (1U << BTRFS_ORDERED_COMPRESSED));
diff --git a/fs/btrfs/ordered-data.c b/fs/btrfs/ordered-data.c
index 1b4d20a2f983..bb5477ce58db 100644
--- a/fs/btrfs/ordered-data.c
+++ b/fs/btrfs/ordered-data.c
@@ -146,7 +146,8 @@ static inline struct rb_node *ordered_tree_search(struct btrfs_inode *inode,
 }
 
 static struct btrfs_ordered_extent *alloc_ordered_extent(
-			struct btrfs_inode *inode, u64 file_offset, u64 num_bytes,
+			struct btrfs_inode *inode,
+			u64 file_offset, u64 orig_offset, u64 num_bytes,
 			u64 ram_bytes, u64 disk_bytenr, u64 disk_num_bytes,
 			u64 offset, unsigned long flags, int compress_type,
 			struct fscrypt_extent_info *fscrypt_info)
@@ -180,6 +181,7 @@ static struct btrfs_ordered_extent *alloc_ordered_extent(
 	}
 
 	entry->file_offset = file_offset;
+	entry->orig_offset = orig_offset;
 	entry->num_bytes = num_bytes;
 	entry->ram_bytes = ram_bytes;
 	entry->disk_bytenr = disk_bytenr;
@@ -268,6 +270,7 @@ static void insert_ordered_extent(struct btrfs_ordered_extent *entry)
  *
  * @inode:           Inode that this extent is for.
  * @file_offset:     Logical offset in file where the extent starts.
+ * @orig_offset:     Logical offset of the original extent (PREALLOC or NOCOW)
  * @num_bytes:       Logical length of extent in file.
  * @ram_bytes:       Full length of unencoded data.
  * @disk_bytenr:     Offset of extent on disk.
@@ -305,6 +308,7 @@ struct btrfs_ordered_extent *btrfs_alloc_ordered_extent(
 	 */
 	if (flags & ((1U << BTRFS_ORDERED_NOCOW) | (1U << BTRFS_ORDERED_PREALLOC)))
 		entry = alloc_ordered_extent(inode, file_offset,
+					     file_extent->orig_offset,
 					     file_extent->num_bytes,
 					     file_extent->num_bytes,
 					     file_extent->disk_bytenr + file_extent->offset,
@@ -313,6 +317,7 @@ struct btrfs_ordered_extent *btrfs_alloc_ordered_extent(
 					     file_extent->fscrypt_info);
 	else
 		entry = alloc_ordered_extent(inode, file_offset,
+					     file_extent->orig_offset,
 					     file_extent->num_bytes,
 					     file_extent->ram_bytes,
 					     file_extent->disk_bytenr,
@@ -1277,8 +1282,8 @@ struct btrfs_ordered_extent *btrfs_split_ordered_extent(
 	if (WARN_ON_ONCE(ordered->disk_num_bytes != ordered->num_bytes))
 		return ERR_PTR(-EINVAL);
 
-	new = alloc_ordered_extent(inode, file_offset, len, len, disk_bytenr, len, 0,
-				   flags, ordered->compress_type, ordered->fscrypt_info);
+	new = alloc_ordered_extent(inode, file_offset, ordered->orig_offset, len, len, disk_bytenr,
+				   len, 0, flags, ordered->compress_type, ordered->fscrypt_info);
 	if (IS_ERR(new))
 		return new;
 
@@ -1315,6 +1320,16 @@ struct btrfs_ordered_extent *btrfs_split_ordered_extent(
 	ordered->disk_num_bytes -= len;
 	ordered->ram_bytes -= len;
 
+	/*
+	 * ->orig_offset is the original offset of the original extent, which
+	 * for PREALLOC or NOCOW stays the same, but if we're a regular extent
+	 * that means this is a new extent and thus ->orig_offset must equal
+	 * ->file_offset.  This is only important for encryption as we only use
+	 * it for setting the offset for the bio encryption context.
+	 */
+	if (test_bit(BTRFS_ORDERED_REGULAR, &ordered->flags))
+		ordered->orig_offset = ordered->file_offset;
+
 	if (test_bit(BTRFS_ORDERED_IO_DONE, &ordered->flags)) {
 		ASSERT(ordered->bytes_left == 0);
 		new->bytes_left = 0;
diff --git a/fs/btrfs/ordered-data.h b/fs/btrfs/ordered-data.h
index 51c795865fe6..58cc1713eb4d 100644
--- a/fs/btrfs/ordered-data.h
+++ b/fs/btrfs/ordered-data.h
@@ -99,6 +99,12 @@ struct btrfs_ordered_extent {
 	/* logical offset in the file */
 	u64 file_offset;
 
+	/*
+	 * The original logical offset of the extent, this is for NOCOW and
+	 * PREALLOC extents, otherwise it'll be the same as file_offset.
+	 */
+	u64 orig_offset;
+
 	/*
 	 * These fields directly correspond to the same fields in
 	 * btrfs_file_extent_item.
@@ -188,6 +194,7 @@ struct btrfs_file_extent {
 	u64 num_bytes;
 	u64 ram_bytes;
 	u64 offset;
+	u64 orig_offset;
 	struct fscrypt_extent_info *fscrypt_info;
 	u8 compression;
 };
-- 
2.51.0

Daniel Vacek <neelx@suse.com> wrote:
> From: Josef Bacik <josef@toxicpanda.com>
> 
> For extent encryption we have to use a logical block nr as input for the
> IV.  For btrfs we're using the offset into the extent we're operating
> on.  For most ordered extents this is the same as the file_offset,
> however for prealloc and NOCOW we have to use the original offset.
> 
> Add this as an argument and plumb it through everywhere, this will be
> used when setting up the bio.
> 

Hi everyone,

I'm running the btrfs for-next branch, along with the encryption patch set
through my AI review prompts:

https://github.com/masoncl/review-prompts

I'm working on some btrfs specific instructions, but hopefully these reviews
are already useful.  I've kept the headers to make it easier to double check
that I'm actually replying to the right email, but please let me know if any
parts of this are incorrect:

commit 0e7f6a617a2642bbdf97bcd639fb887fa88189ad
Author: Josef Bacik <josef@toxicpanda.com>

btrfs: populate ordered_extent with the orig offset

This commit adds orig_offset tracking to btrfs_ordered_extent and
btrfs_file_extent structures for extent encryption support, where the
offset into the extent is needed for IV generation.

> diff --git a/fs/btrfs/direct-io.c b/fs/btrfs/direct-io.c
> index d3789109ca851..2d89ac05b1b38 100644
> --- a/fs/btrfs/direct-io.c
> +++ b/fs/btrfs/direct-io.c
> @@ -155,6 +155,7 @@ static struct extent_map *btrfs_create_dio_extent(struct btrfs_inode *inode,
>  		file_extent->fscrypt_info = em->fscrypt_info;
>  	}
>
> +	file_extent->orig_offset = start - file_extent->offset;
>  	ordered = btrfs_alloc_ordered_extent(inode, start, file_extent,
>  					     (1U << type) |
>  					     (1U << BTRFS_ORDERED_DIRECT));

[ ... ]

> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index fbb968d4010b7..8078049f690e1 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c

[ ... ]

> @@ -2296,6 +2298,8 @@ static noinline int run_delalloc_nocow(struct btrfs_inode *inode,
>  			cow_start = (u64)-1;
>  		}
>
> +		nocow_args.file_extent.orig_offset =
> +			found_key.offset - nocow_args.file_extent.offset;
>  		ret = nocow_one_range(inode, locked_folio, &cached_state,
>  				      &nocow_args, cur_offset,
>  				      extent_type == BTRFS_FILE_EXTENT_PREALLOC);

Should this calculation use cur_offset instead of found_key.offset?

Looking at the DIO path in btrfs_create_dio_extent() (direct-io.c line
158), the orig_offset is calculated as:

    file_extent->orig_offset = start - file_extent->offset;

where 'start' is the current write position. In run_delalloc_nocow(),
found_key.offset represents where the extent item is stored, while
cur_offset represents the current write position (analogous to 'start'
in the DIO path).

When writing to a position within an existing extent (not at its start),
using found_key.offset - file_extent.offset could produce incorrect
values. For consistency with the DIO path, would the formula be:

    nocow_args.file_extent.orig_offset =
        cur_offset - nocow_args.file_extent.offset;

This would ensure that NOCOW writes calculate orig_offset the same way
as DIO writes, which matters for encryption IV generation where an
incorrect offset would cause data corruption on reads.