diff mbox series

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

Message ID daee5e8b14d706fe4dd96bd910fd46038512861b.1709203710.git.fdmanana@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs: fix off-by-one chunk length calculation at contains_pending_extent() | expand

Commit Message

Filipe Manana Feb. 29, 2024, 10:50 a.m. UTC
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(-)

Comments

Josef Bacik March 1, 2024, 9:11 p.m. UTC | #1
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
Qu Wenruo March 2, 2024, 12:35 a.m. UTC | #2
在 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;
>   		}
diff mbox series

Patch

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;
 		}