* [PATCH v2 0/3] btrfs: add basic alignment checks for extent_state
@ 2026-07-28 9:11 Qu Wenruo
2026-07-28 9:11 ` [PATCH v2 1/3] btrfs: use aligned range for locking in extent_fiemap() Qu Wenruo
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Qu Wenruo @ 2026-07-28 9:11 UTC (permalink / raw)
To: linux-btrfs
[CHANGELOG]
v2:
- Fix a crash caught by generic/136
Since btrfs allow reflinking the last block until EOF, the range
passed for btrfs_clone() can be unaligned.
Fix it by passing aligned range end into btrfs_lock_extent().
This won't affect the btrfs_clone() call as it doesn't use inclusive
end.
- Slightly rewords the first patch
To explain it more clearly that the fix is for btrfs_lock_extent().
Commit 3f255ece2f1e ("btrfs: introduce extra sanity checks for extent
maps") introduced sanity checks for extent maps, but for extent state we
do not have any extra sanity checks.
This series will introduce the check in the 2nd patch, meanwhile the
first one is to fix an exposed unaligned access in extent_fiemap().
Qu Wenruo (3):
btrfs: use aligned range for locking in extent_fiemap()
btrfs: use aligned range for locking in reflink
btrfs: add validation for extent states
fs/btrfs/extent-io-tree.c | 21 +++++++++++++++++++++
fs/btrfs/fiemap.c | 8 ++++----
fs/btrfs/reflink.c | 4 ++--
3 files changed, 27 insertions(+), 6 deletions(-)
--
2.54.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/3] btrfs: use aligned range for locking in extent_fiemap()
2026-07-28 9:11 [PATCH v2 0/3] btrfs: add basic alignment checks for extent_state Qu Wenruo
@ 2026-07-28 9:11 ` Qu Wenruo
2026-07-28 9:38 ` Filipe Manana
2026-07-28 9:11 ` [PATCH v2 2/3] btrfs: use aligned range for locking in reflink Qu Wenruo
2026-07-28 9:11 ` [PATCH v2 3/3] btrfs: add validation for extent states Qu Wenruo
2 siblings, 1 reply; 9+ messages in thread
From: Qu Wenruo @ 2026-07-28 9:11 UTC (permalink / raw)
To: linux-btrfs
The @end parameter for all extent io tree helpers is inclusive, but
the call site in extent_fiemap() is passing an exclusive end into
btrfs_lock_extent(), which will step into the next block unexpectedly.
Fix @range_end to be inclusive, so that btrfs_lock_extent() and
btrfs_unlock_extent() will lock/unlock the correct aligned range.
Fixes: ac3c0d36a2a2 ("btrfs: make fiemap more efficient and accurate reporting extent sharedness")
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/fiemap.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/fs/btrfs/fiemap.c b/fs/btrfs/fiemap.c
index ba6a360074c0..2f2ae0b76799 100644
--- a/fs/btrfs/fiemap.c
+++ b/fs/btrfs/fiemap.c
@@ -657,7 +657,7 @@ static int extent_fiemap(struct btrfs_inode *inode,
restart:
range_start = round_down(start, sectorsize);
- range_end = round_up(start + len, sectorsize);
+ range_end = round_up(start + len, sectorsize) - 1;
prev_extent_end = range_start;
btrfs_lock_extent(&inode->io_tree, range_start, range_end, &cached_state);
@@ -710,7 +710,7 @@ static int extent_fiemap(struct btrfs_inode *inode,
/* We have in implicit hole (NO_HOLES feature enabled). */
if (prev_extent_end < key.offset) {
- const u64 hole_end = min(key.offset, range_end) - 1;
+ const u64 hole_end = min(key.offset - 1, range_end);
ret = fiemap_process_hole(inode, fieinfo, &cache,
&delalloc_cached_state,
@@ -812,10 +812,10 @@ static int extent_fiemap(struct btrfs_inode *inode,
if (!stopped && prev_extent_end < range_end) {
ret = fiemap_process_hole(inode, fieinfo, &cache,
&delalloc_cached_state, backref_ctx,
- 0, 0, 0, prev_extent_end, range_end - 1);
+ 0, 0, 0, prev_extent_end, range_end);
if (ret < 0)
goto out_unlock;
- prev_extent_end = range_end;
+ prev_extent_end = range_end + 1;
}
if (cache.cached && cache.offset + cache.len >= last_extent_end) {
--
2.54.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/3] btrfs: use aligned range for locking in reflink
2026-07-28 9:11 [PATCH v2 0/3] btrfs: add basic alignment checks for extent_state Qu Wenruo
2026-07-28 9:11 ` [PATCH v2 1/3] btrfs: use aligned range for locking in extent_fiemap() Qu Wenruo
@ 2026-07-28 9:11 ` Qu Wenruo
2026-07-28 9:46 ` Filipe Manana
2026-07-28 9:11 ` [PATCH v2 3/3] btrfs: add validation for extent states Qu Wenruo
2 siblings, 1 reply; 9+ messages in thread
From: Qu Wenruo @ 2026-07-28 9:11 UTC (permalink / raw)
To: linux-btrfs
In btrfs_extent_same_range() and btrfs_clone_files(), the range passed
into btrfs_lock_extent() is not aligned at its end, because we can
reflink until the EOF, which may not be block aligned.
Although this is not a big deal, for the sake of consistency, and to
prepare for the upcoming stricter alignment check, pass an aligned range
end to btrfs_lock_extent() and btrfs_unlock_extent().
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/reflink.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/reflink.c b/fs/btrfs/reflink.c
index ec6a760519d9..d2a4101912bd 100644
--- a/fs/btrfs/reflink.c
+++ b/fs/btrfs/reflink.c
@@ -687,10 +687,10 @@ static void btrfs_double_mmap_unlock(struct btrfs_inode *inode1, struct btrfs_in
static int btrfs_extent_same_range(struct btrfs_inode *src, u64 loff, u64 len,
struct btrfs_inode *dst, u64 dst_loff)
{
- const u64 end = dst_loff + len - 1;
struct extent_state *cached_state = NULL;
struct btrfs_fs_info *fs_info = src->root->fs_info;
const u32 bs = fs_info->sectorsize;
+ const u64 end = round_up(dst_loff + len, bs) - 1;
int ret;
/*
@@ -799,7 +799,7 @@ static noinline int btrfs_clone_files(struct file *file, struct file *file_src,
* because we have already locked the inode's i_mmap_lock in exclusive
* mode.
*/
- end = destoff + len - 1;
+ end = round_up(destoff + len, bs) - 1;
btrfs_lock_extent(&inode->io_tree, destoff, end, &cached_state);
ret = btrfs_clone(src, inode, off, olen, len, destoff, false);
btrfs_unlock_extent(&inode->io_tree, destoff, end, &cached_state);
--
2.54.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 3/3] btrfs: add validation for extent states
2026-07-28 9:11 [PATCH v2 0/3] btrfs: add basic alignment checks for extent_state Qu Wenruo
2026-07-28 9:11 ` [PATCH v2 1/3] btrfs: use aligned range for locking in extent_fiemap() Qu Wenruo
2026-07-28 9:11 ` [PATCH v2 2/3] btrfs: use aligned range for locking in reflink Qu Wenruo
@ 2026-07-28 9:11 ` Qu Wenruo
2026-07-28 9:43 ` Filipe Manana
2 siblings, 1 reply; 9+ messages in thread
From: Qu Wenruo @ 2026-07-28 9:11 UTC (permalink / raw)
To: linux-btrfs
Extent maps have the extra validation since commit 3f255ece2f1e ("btrfs:
introduce extra sanity checks for extent maps"), but extent states do
not have a similar check.
Introduce a basic alignment check for the following call sites, so that
we can cover all extent states inserted into the tree:
- insert_state_fast()
- insert_state()
- split_state()
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/extent-io-tree.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
index c18ea5ef2974..c6e639d0fcc1 100644
--- a/fs/btrfs/extent-io-tree.c
+++ b/fs/btrfs/extent-io-tree.c
@@ -334,6 +334,21 @@ static inline struct extent_state *tree_search(struct extent_io_tree *tree, u64
return tree_search_for_insert(tree, offset, NULL, NULL);
}
+static void validate_extent_state(const struct extent_io_tree *tree,
+ struct extent_state *state)
+{
+ u32 blocksize;
+
+ if (tree->owner != IO_TREE_INODE_IO)
+ return;
+
+ blocksize = btrfs_extent_io_tree_to_fs_info(tree)->sectorsize;
+ ASSERT(IS_ALIGNED(state->start, blocksize) &&
+ IS_ALIGNED(state->end + 1, blocksize),
+ "unaligned extent state, blocksize=%u start=%llu end=%llu state=0x%x",
+ blocksize, state->start, state->end, state->state);
+}
+
#define extent_io_tree_panic(tree, state, opname, err) \
btrfs_panic(btrfs_extent_io_tree_to_fs_info((tree)), (err), \
"extent io tree error on %s state start %llu end %llu", \
@@ -429,6 +444,8 @@ static struct extent_state *insert_state(struct extent_io_tree *tree,
const u64 end = state->end + 1;
const bool try_merge = !(bits & (EXTENT_LOCK_BITS | EXTENT_BOUNDARY));
+ validate_extent_state(tree, state);
+
set_state_bits(tree, state, bits, changeset);
node = &tree->state.rb_node;
@@ -481,6 +498,8 @@ static void insert_state_fast(struct extent_io_tree *tree,
struct rb_node *parent, unsigned bits,
struct extent_changeset *changeset)
{
+ validate_extent_state(tree, state);
+
set_state_bits(tree, state, bits, changeset);
rb_link_node(&state->rb_node, parent, node);
rb_insert_color(&state->rb_node, &tree->state);
@@ -533,6 +552,8 @@ static int split_state(struct extent_io_tree *tree, struct extent_state *orig,
}
}
+ validate_extent_state(tree, orig);
+ validate_extent_state(tree, prealloc);
rb_link_node(&prealloc->rb_node, parent, node);
rb_insert_color(&prealloc->rb_node, &tree->state);
--
2.54.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/3] btrfs: use aligned range for locking in extent_fiemap()
2026-07-28 9:11 ` [PATCH v2 1/3] btrfs: use aligned range for locking in extent_fiemap() Qu Wenruo
@ 2026-07-28 9:38 ` Filipe Manana
2026-07-28 9:41 ` Qu Wenruo
0 siblings, 1 reply; 9+ messages in thread
From: Filipe Manana @ 2026-07-28 9:38 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Tue, Jul 28, 2026 at 10:23 AM Qu Wenruo <wqu@suse.com> wrote:
>
> The @end parameter for all extent io tree helpers is inclusive, but
> the call site in extent_fiemap() is passing an exclusive end into
> btrfs_lock_extent(), which will step into the next block unexpectedly.
>
> Fix @range_end to be inclusive, so that btrfs_lock_extent() and
> btrfs_unlock_extent() will lock/unlock the correct aligned range.
>
> Fixes: ac3c0d36a2a2 ("btrfs: make fiemap more efficient and accurate reporting extent sharedness")
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> fs/btrfs/fiemap.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/btrfs/fiemap.c b/fs/btrfs/fiemap.c
> index ba6a360074c0..2f2ae0b76799 100644
> --- a/fs/btrfs/fiemap.c
> +++ b/fs/btrfs/fiemap.c
> @@ -657,7 +657,7 @@ static int extent_fiemap(struct btrfs_inode *inode,
>
> restart:
> range_start = round_down(start, sectorsize);
> - range_end = round_up(start + len, sectorsize);
> + range_end = round_up(start + len, sectorsize) - 1;
> prev_extent_end = range_start;
>
> btrfs_lock_extent(&inode->io_tree, range_start, range_end, &cached_state);
> @@ -710,7 +710,7 @@ static int extent_fiemap(struct btrfs_inode *inode,
>
> /* We have in implicit hole (NO_HOLES feature enabled). */
> if (prev_extent_end < key.offset) {
> - const u64 hole_end = min(key.offset, range_end) - 1;
> + const u64 hole_end = min(key.offset - 1, range_end);
>
> ret = fiemap_process_hole(inode, fieinfo, &cache,
> &delalloc_cached_state,
> @@ -812,10 +812,10 @@ static int extent_fiemap(struct btrfs_inode *inode,
> if (!stopped && prev_extent_end < range_end) {
> ret = fiemap_process_hole(inode, fieinfo, &cache,
> &delalloc_cached_state, backref_ctx,
> - 0, 0, 0, prev_extent_end, range_end - 1);
> + 0, 0, 0, prev_extent_end, range_end);
> if (ret < 0)
> goto out_unlock;
> - prev_extent_end = range_end;
> + prev_extent_end = range_end + 1;
If the problem is passing an exclusive end to btrfs_lock_extent(),
then rather than doing all these changes to this variables, which is
error prone since they are used in several places and generally we use
exclusive ends everywhere, I'd prefer to simply change the call to
btrfs_lock_extent() and btrfs_unlock_extent() to receive 'range_end -
1'. That also makes the fix much clearer.
Thanks.
> }
>
> if (cache.cached && cache.offset + cache.len >= last_extent_end) {
> --
> 2.54.0
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/3] btrfs: use aligned range for locking in extent_fiemap()
2026-07-28 9:38 ` Filipe Manana
@ 2026-07-28 9:41 ` Qu Wenruo
2026-07-28 9:50 ` Filipe Manana
0 siblings, 1 reply; 9+ messages in thread
From: Qu Wenruo @ 2026-07-28 9:41 UTC (permalink / raw)
To: Filipe Manana; +Cc: linux-btrfs
在 2026/7/28 19:08, Filipe Manana 写道:
> On Tue, Jul 28, 2026 at 10:23 AM Qu Wenruo <wqu@suse.com> wrote:
>>
>> The @end parameter for all extent io tree helpers is inclusive, but
>> the call site in extent_fiemap() is passing an exclusive end into
>> btrfs_lock_extent(), which will step into the next block unexpectedly.
>>
>> Fix @range_end to be inclusive, so that btrfs_lock_extent() and
>> btrfs_unlock_extent() will lock/unlock the correct aligned range.
>>
>> Fixes: ac3c0d36a2a2 ("btrfs: make fiemap more efficient and accurate reporting extent sharedness")
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>> fs/btrfs/fiemap.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/btrfs/fiemap.c b/fs/btrfs/fiemap.c
>> index ba6a360074c0..2f2ae0b76799 100644
>> --- a/fs/btrfs/fiemap.c
>> +++ b/fs/btrfs/fiemap.c
>> @@ -657,7 +657,7 @@ static int extent_fiemap(struct btrfs_inode *inode,
>>
>> restart:
>> range_start = round_down(start, sectorsize);
>> - range_end = round_up(start + len, sectorsize);
>> + range_end = round_up(start + len, sectorsize) - 1;
>> prev_extent_end = range_start;
>>
>> btrfs_lock_extent(&inode->io_tree, range_start, range_end, &cached_state);
>> @@ -710,7 +710,7 @@ static int extent_fiemap(struct btrfs_inode *inode,
>>
>> /* We have in implicit hole (NO_HOLES feature enabled). */
>> if (prev_extent_end < key.offset) {
>> - const u64 hole_end = min(key.offset, range_end) - 1;
>> + const u64 hole_end = min(key.offset - 1, range_end);
>>
>> ret = fiemap_process_hole(inode, fieinfo, &cache,
>> &delalloc_cached_state,
>> @@ -812,10 +812,10 @@ static int extent_fiemap(struct btrfs_inode *inode,
>> if (!stopped && prev_extent_end < range_end) {
>> ret = fiemap_process_hole(inode, fieinfo, &cache,
>> &delalloc_cached_state, backref_ctx,
>> - 0, 0, 0, prev_extent_end, range_end - 1);
>> + 0, 0, 0, prev_extent_end, range_end);
>> if (ret < 0)
>> goto out_unlock;
>> - prev_extent_end = range_end;
>> + prev_extent_end = range_end + 1;
>
> If the problem is passing an exclusive end to btrfs_lock_extent(),
> then rather than doing all these changes to this variables, which is
> error prone since they are used in several places and generally we use
> exclusive ends everywhere, I'd prefer to simply change the call to
> btrfs_lock_extent() and btrfs_unlock_extent() to receive 'range_end -
> 1'. That also makes the fix much clearer.
Thanks for the advice, indeed that is much safer.
Thanks,
Qu>
> Thanks.
>
>> }
>>
>> if (cache.cached && cache.offset + cache.len >= last_extent_end) {
>> --
>> 2.54.0
>>
>>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/3] btrfs: add validation for extent states
2026-07-28 9:11 ` [PATCH v2 3/3] btrfs: add validation for extent states Qu Wenruo
@ 2026-07-28 9:43 ` Filipe Manana
0 siblings, 0 replies; 9+ messages in thread
From: Filipe Manana @ 2026-07-28 9:43 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Tue, Jul 28, 2026 at 10:36 AM Qu Wenruo <wqu@suse.com> wrote:
>
> Extent maps have the extra validation since commit 3f255ece2f1e ("btrfs:
> introduce extra sanity checks for extent maps"), but extent states do
> not have a similar check.
>
> Introduce a basic alignment check for the following call sites, so that
> we can cover all extent states inserted into the tree:
>
> - insert_state_fast()
> - insert_state()
> - split_state()
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> fs/btrfs/extent-io-tree.c | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
> diff --git a/fs/btrfs/extent-io-tree.c b/fs/btrfs/extent-io-tree.c
> index c18ea5ef2974..c6e639d0fcc1 100644
> --- a/fs/btrfs/extent-io-tree.c
> +++ b/fs/btrfs/extent-io-tree.c
> @@ -334,6 +334,21 @@ static inline struct extent_state *tree_search(struct extent_io_tree *tree, u64
> return tree_search_for_insert(tree, offset, NULL, NULL);
> }
>
> +static void validate_extent_state(const struct extent_io_tree *tree,
> + struct extent_state *state)
Why isn't state marked as const as well?
With that change:
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Thanks.
> +{
> + u32 blocksize;
> +
> + if (tree->owner != IO_TREE_INODE_IO)
> + return;
> +
> + blocksize = btrfs_extent_io_tree_to_fs_info(tree)->sectorsize;
> + ASSERT(IS_ALIGNED(state->start, blocksize) &&
> + IS_ALIGNED(state->end + 1, blocksize),
> + "unaligned extent state, blocksize=%u start=%llu end=%llu state=0x%x",
> + blocksize, state->start, state->end, state->state);
> +}
> +
> #define extent_io_tree_panic(tree, state, opname, err) \
> btrfs_panic(btrfs_extent_io_tree_to_fs_info((tree)), (err), \
> "extent io tree error on %s state start %llu end %llu", \
> @@ -429,6 +444,8 @@ static struct extent_state *insert_state(struct extent_io_tree *tree,
> const u64 end = state->end + 1;
> const bool try_merge = !(bits & (EXTENT_LOCK_BITS | EXTENT_BOUNDARY));
>
> + validate_extent_state(tree, state);
> +
> set_state_bits(tree, state, bits, changeset);
>
> node = &tree->state.rb_node;
> @@ -481,6 +498,8 @@ static void insert_state_fast(struct extent_io_tree *tree,
> struct rb_node *parent, unsigned bits,
> struct extent_changeset *changeset)
> {
> + validate_extent_state(tree, state);
> +
> set_state_bits(tree, state, bits, changeset);
> rb_link_node(&state->rb_node, parent, node);
> rb_insert_color(&state->rb_node, &tree->state);
> @@ -533,6 +552,8 @@ static int split_state(struct extent_io_tree *tree, struct extent_state *orig,
> }
> }
>
> + validate_extent_state(tree, orig);
> + validate_extent_state(tree, prealloc);
> rb_link_node(&prealloc->rb_node, parent, node);
> rb_insert_color(&prealloc->rb_node, &tree->state);
>
> --
> 2.54.0
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] btrfs: use aligned range for locking in reflink
2026-07-28 9:11 ` [PATCH v2 2/3] btrfs: use aligned range for locking in reflink Qu Wenruo
@ 2026-07-28 9:46 ` Filipe Manana
0 siblings, 0 replies; 9+ messages in thread
From: Filipe Manana @ 2026-07-28 9:46 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Tue, Jul 28, 2026 at 10:44 AM Qu Wenruo <wqu@suse.com> wrote:
>
> In btrfs_extent_same_range() and btrfs_clone_files(), the range passed
> into btrfs_lock_extent() is not aligned at its end, because we can
> reflink until the EOF, which may not be block aligned.
>
> Although this is not a big deal, for the sake of consistency, and to
> prepare for the upcoming stricter alignment check, pass an aligned range
> end to btrfs_lock_extent() and btrfs_unlock_extent().
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Thanks.
> ---
> fs/btrfs/reflink.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/reflink.c b/fs/btrfs/reflink.c
> index ec6a760519d9..d2a4101912bd 100644
> --- a/fs/btrfs/reflink.c
> +++ b/fs/btrfs/reflink.c
> @@ -687,10 +687,10 @@ static void btrfs_double_mmap_unlock(struct btrfs_inode *inode1, struct btrfs_in
> static int btrfs_extent_same_range(struct btrfs_inode *src, u64 loff, u64 len,
> struct btrfs_inode *dst, u64 dst_loff)
> {
> - const u64 end = dst_loff + len - 1;
> struct extent_state *cached_state = NULL;
> struct btrfs_fs_info *fs_info = src->root->fs_info;
> const u32 bs = fs_info->sectorsize;
> + const u64 end = round_up(dst_loff + len, bs) - 1;
> int ret;
>
> /*
> @@ -799,7 +799,7 @@ static noinline int btrfs_clone_files(struct file *file, struct file *file_src,
> * because we have already locked the inode's i_mmap_lock in exclusive
> * mode.
> */
> - end = destoff + len - 1;
> + end = round_up(destoff + len, bs) - 1;
> btrfs_lock_extent(&inode->io_tree, destoff, end, &cached_state);
> ret = btrfs_clone(src, inode, off, olen, len, destoff, false);
> btrfs_unlock_extent(&inode->io_tree, destoff, end, &cached_state);
> --
> 2.54.0
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/3] btrfs: use aligned range for locking in extent_fiemap()
2026-07-28 9:41 ` Qu Wenruo
@ 2026-07-28 9:50 ` Filipe Manana
0 siblings, 0 replies; 9+ messages in thread
From: Filipe Manana @ 2026-07-28 9:50 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Tue, Jul 28, 2026 at 10:41 AM Qu Wenruo <wqu@suse.com> wrote:
>
>
>
> 在 2026/7/28 19:08, Filipe Manana 写道:
> > On Tue, Jul 28, 2026 at 10:23 AM Qu Wenruo <wqu@suse.com> wrote:
> >>
> >> The @end parameter for all extent io tree helpers is inclusive, but
> >> the call site in extent_fiemap() is passing an exclusive end into
> >> btrfs_lock_extent(), which will step into the next block unexpectedly.
> >>
> >> Fix @range_end to be inclusive, so that btrfs_lock_extent() and
> >> btrfs_unlock_extent() will lock/unlock the correct aligned range.
> >>
> >> Fixes: ac3c0d36a2a2 ("btrfs: make fiemap more efficient and accurate reporting extent sharedness")
> >> Signed-off-by: Qu Wenruo <wqu@suse.com>
> >> ---
> >> fs/btrfs/fiemap.c | 8 ++++----
> >> 1 file changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/fs/btrfs/fiemap.c b/fs/btrfs/fiemap.c
> >> index ba6a360074c0..2f2ae0b76799 100644
> >> --- a/fs/btrfs/fiemap.c
> >> +++ b/fs/btrfs/fiemap.c
> >> @@ -657,7 +657,7 @@ static int extent_fiemap(struct btrfs_inode *inode,
> >>
> >> restart:
> >> range_start = round_down(start, sectorsize);
> >> - range_end = round_up(start + len, sectorsize);
> >> + range_end = round_up(start + len, sectorsize) - 1;
> >> prev_extent_end = range_start;
> >>
> >> btrfs_lock_extent(&inode->io_tree, range_start, range_end, &cached_state);
> >> @@ -710,7 +710,7 @@ static int extent_fiemap(struct btrfs_inode *inode,
> >>
> >> /* We have in implicit hole (NO_HOLES feature enabled). */
> >> if (prev_extent_end < key.offset) {
> >> - const u64 hole_end = min(key.offset, range_end) - 1;
> >> + const u64 hole_end = min(key.offset - 1, range_end);
> >>
> >> ret = fiemap_process_hole(inode, fieinfo, &cache,
> >> &delalloc_cached_state,
> >> @@ -812,10 +812,10 @@ static int extent_fiemap(struct btrfs_inode *inode,
> >> if (!stopped && prev_extent_end < range_end) {
> >> ret = fiemap_process_hole(inode, fieinfo, &cache,
> >> &delalloc_cached_state, backref_ctx,
> >> - 0, 0, 0, prev_extent_end, range_end - 1);
> >> + 0, 0, 0, prev_extent_end, range_end);
> >> if (ret < 0)
> >> goto out_unlock;
> >> - prev_extent_end = range_end;
> >> + prev_extent_end = range_end + 1;
> >
> > If the problem is passing an exclusive end to btrfs_lock_extent(),
> > then rather than doing all these changes to this variables, which is
> > error prone since they are used in several places and generally we use
> > exclusive ends everywhere, I'd prefer to simply change the call to
> > btrfs_lock_extent() and btrfs_unlock_extent() to receive 'range_end -
> > 1'. That also makes the fix much clearer.
>
> Thanks for the advice, indeed that is much safer.
After that you can add:
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Thanks.
>
> Thanks,
> Qu>
> > Thanks.
> >
> >> }
> >>
> >> if (cache.cached && cache.offset + cache.len >= last_extent_end) {
> >> --
> >> 2.54.0
> >>
> >>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-07-28 9:51 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-28 9:11 [PATCH v2 0/3] btrfs: add basic alignment checks for extent_state Qu Wenruo
2026-07-28 9:11 ` [PATCH v2 1/3] btrfs: use aligned range for locking in extent_fiemap() Qu Wenruo
2026-07-28 9:38 ` Filipe Manana
2026-07-28 9:41 ` Qu Wenruo
2026-07-28 9:50 ` Filipe Manana
2026-07-28 9:11 ` [PATCH v2 2/3] btrfs: use aligned range for locking in reflink Qu Wenruo
2026-07-28 9:46 ` Filipe Manana
2026-07-28 9:11 ` [PATCH v2 3/3] btrfs: add validation for extent states Qu Wenruo
2026-07-28 9:43 ` Filipe Manana
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox