[PATCH] btrfs: fix off-by-one chunk length calculation at contains_pending_extent()

[PATCH] btrfs: fix off-by-one chunk length calculation at contains_pending_extent()

[fdmanana]

From: Filipe Manana <fdmanana@suse.com>

At contains_pending_extent() the value of the end offset of a chunk we
found in the device's allocation state io tree is inclusive, so when
we calculate the length we pass to the in_range() macro, we must sum
1 to the expression "physical_end - physical_offset".

In practice the wrong calculation should be harmless as chunks sizes
are never 1 byte and we should never have 1 byte ranges of unallocated
space. Nevertheless fix the wrong calculation.

Fixes: 1c11b63eff2a ("btrfs: replace pending/pinned chunks lists with io tree")
Reported-by: Alex Lyakas <alex.lyakas@zadara.com>
Link: https://lore.kernel.org/linux-btrfs/CAOcd+r30e-f4R-5x-S7sV22RJPe7+pgwherA6xqN2_qe7o4XTg@mail.gmail.com/
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/volumes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 3cc947a42116..473fe92274d9 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -1401,7 +1401,7 @@ static bool contains_pending_extent(struct btrfs_device *device, u64 *start,
 
 		if (in_range(physical_start, *start, len) ||
 		    in_range(*start, physical_start,
-			     physical_end - physical_start)) {
+			     physical_end + 1 - physical_start)) {
 			*start = physical_end + 1;
 			return true;
 		}
-- 
2.40.1

Re: [PATCH] btrfs: fix off-by-one chunk length calculation at contains_pending_extent()

[Josef Bacik]

On Thu, Feb 29, 2024 at 10:50:03AM +0000, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> At contains_pending_extent() the value of the end offset of a chunk we
> found in the device's allocation state io tree is inclusive, so when
> we calculate the length we pass to the in_range() macro, we must sum
> 1 to the expression "physical_end - physical_offset".
> 
> In practice the wrong calculation should be harmless as chunks sizes
> are never 1 byte and we should never have 1 byte ranges of unallocated
> space. Nevertheless fix the wrong calculation.
> 
> Fixes: 1c11b63eff2a ("btrfs: replace pending/pinned chunks lists with io tree")
> Reported-by: Alex Lyakas <alex.lyakas@zadara.com>
> Link: https://lore.kernel.org/linux-btrfs/CAOcd+r30e-f4R-5x-S7sV22RJPe7+pgwherA6xqN2_qe7o4XTg@mail.gmail.com/
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef

Re: [PATCH] btrfs: fix off-by-one chunk length calculation at contains_pending_extent()

[Qu Wenruo]

在 2024/2/29 21:20, fdmanana@kernel.org 写道:
> From: Filipe Manana <fdmanana@suse.com>
>
> At contains_pending_extent() the value of the end offset of a chunk we
> found in the device's allocation state io tree is inclusive, so when
> we calculate the length we pass to the in_range() macro, we must sum
> 1 to the expression "physical_end - physical_offset".
>
> In practice the wrong calculation should be harmless as chunks sizes
> are never 1 byte and we should never have 1 byte ranges of unallocated
> space. Nevertheless fix the wrong calculation.
>
> Fixes: 1c11b63eff2a ("btrfs: replace pending/pinned chunks lists with io tree")
> Reported-by: Alex Lyakas <alex.lyakas@zadara.com>
> Link: https://lore.kernel.org/linux-btrfs/CAOcd+r30e-f4R-5x-S7sV22RJPe7+pgwherA6xqN2_qe7o4XTg@mail.gmail.com/
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thankfully it's mostly harmless.

Thanks,
Qu
> ---
>   fs/btrfs/volumes.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 3cc947a42116..473fe92274d9 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -1401,7 +1401,7 @@ static bool contains_pending_extent(struct btrfs_device *device, u64 *start,
>
>   		if (in_range(physical_start, *start, len) ||
>   		    in_range(*start, physical_start,
> -			     physical_end - physical_start)) {
> +			     physical_end + 1 - physical_start)) {
>   			*start = physical_end + 1;
>   			return true;
>   		}