* [PATCH v2 0/3] ext4: fix some ext4_lblk_t overflow issues
@ 2023-07-24 12:10 Baokun Li
2023-07-24 12:10 ` [PATCH v2 1/3] ext4: add two helper functions extent_logical_end() and pa_logical_end() Baokun Li
` (3 more replies)
0 siblings, 4 replies; 12+ messages in thread
From: Baokun Li @ 2023-07-24 12:10 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, adilger.kernel, jack, ritesh.list, ojaswin, linux-kernel,
yi.zhang, yangerkun, yukuai3, libaokun1
Changes since v1:
* Rename fex_end() and pa_end() to extent_logical_end() and pa_logical_end()
to make the code more readable.
* Refactor the logic for adjusting the best extent in ext4_mb_new_inode_pa()
to simplify the code and remove redundant parameter for helper function.
* Merged patch 4 to patch 1 as mainline commit 9d3de7ee192a fixed the issue.
Baokun Li (3):
ext4: add two helper functions extent_logical_end() and pa_logical_end()
ext4: fix BUG in ext4_mb_new_inode_pa() due to overflow
ext4: avoid overlapping preallocations due to overflow
fs/ext4/mballoc.c | 61 +++++++++++++++++++++--------------------------
fs/ext4/mballoc.h | 14 +++++++++++
2 files changed, 41 insertions(+), 34 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 1/3] ext4: add two helper functions extent_logical_end() and pa_logical_end()
2023-07-24 12:10 [PATCH v2 0/3] ext4: fix some ext4_lblk_t overflow issues Baokun Li
@ 2023-07-24 12:10 ` Baokun Li
2023-07-25 17:29 ` Ritesh Harjani
2023-08-03 13:58 ` Jan Kara
2023-07-24 12:10 ` [PATCH v2 2/3] ext4: fix BUG in ext4_mb_new_inode_pa() due to overflow Baokun Li
` (2 subsequent siblings)
3 siblings, 2 replies; 12+ messages in thread
From: Baokun Li @ 2023-07-24 12:10 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, adilger.kernel, jack, ritesh.list, ojaswin, linux-kernel,
yi.zhang, yangerkun, yukuai3, libaokun1
When we use lstart + len to calculate the end of free extent or prealloc
space, it may exceed the maximum value of 4294967295(0xffffffff) supported
by ext4_lblk_t and cause overflow, which may lead to various problems.
Therefore, we add two helper functions, extent_logical_end() and
pa_logical_end(), to limit the type of end to loff_t, and also convert
lstart to loff_t for calculation to avoid overflow.
Signed-off-by: Baokun Li <libaokun1@huawei.com>
---
fs/ext4/mballoc.c | 9 +++------
fs/ext4/mballoc.h | 14 ++++++++++++++
2 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 21b903fe546e..4cb13b3e41b3 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -4432,7 +4432,7 @@ ext4_mb_normalize_request(struct ext4_allocation_context *ac,
/* first, let's learn actual file size
* given current request is allocated */
- size = ac->ac_o_ex.fe_logical + EXT4_C2B(sbi, ac->ac_o_ex.fe_len);
+ size = extent_logical_end(sbi, &ac->ac_o_ex);
size = size << bsbits;
if (size < i_size_read(ac->ac_inode))
size = i_size_read(ac->ac_inode);
@@ -4766,7 +4766,6 @@ ext4_mb_use_preallocated(struct ext4_allocation_context *ac)
struct ext4_inode_info *ei = EXT4_I(ac->ac_inode);
struct ext4_locality_group *lg;
struct ext4_prealloc_space *tmp_pa = NULL, *cpa = NULL;
- loff_t tmp_pa_end;
struct rb_node *iter;
ext4_fsblk_t goal_block;
@@ -4862,9 +4861,7 @@ ext4_mb_use_preallocated(struct ext4_allocation_context *ac)
* pa can possibly satisfy the request hence check if it overlaps
* original logical start and stop searching if it doesn't.
*/
- tmp_pa_end = (loff_t)tmp_pa->pa_lstart + EXT4_C2B(sbi, tmp_pa->pa_len);
-
- if (ac->ac_o_ex.fe_logical >= tmp_pa_end) {
+ if (ac->ac_o_ex.fe_logical >= pa_logical_end(sbi, tmp_pa)) {
spin_unlock(&tmp_pa->pa_lock);
goto try_group_pa;
}
@@ -5769,7 +5766,7 @@ static void ext4_mb_group_or_file(struct ext4_allocation_context *ac)
group_pa_eligible = sbi->s_mb_group_prealloc > 0;
inode_pa_eligible = true;
- size = ac->ac_o_ex.fe_logical + EXT4_C2B(sbi, ac->ac_o_ex.fe_len);
+ size = extent_logical_end(sbi, &ac->ac_o_ex);
isize = (i_size_read(ac->ac_inode) + ac->ac_sb->s_blocksize - 1)
>> bsbits;
diff --git a/fs/ext4/mballoc.h b/fs/ext4/mballoc.h
index df6b5e7c2274..d7aeb5da7d86 100644
--- a/fs/ext4/mballoc.h
+++ b/fs/ext4/mballoc.h
@@ -233,6 +233,20 @@ static inline ext4_fsblk_t ext4_grp_offs_to_block(struct super_block *sb,
(fex->fe_start << EXT4_SB(sb)->s_cluster_bits);
}
+static inline loff_t extent_logical_end(struct ext4_sb_info *sbi,
+ struct ext4_free_extent *fex)
+{
+ /* Use loff_t to avoid end exceeding ext4_lblk_t max. */
+ return (loff_t)fex->fe_logical + EXT4_C2B(sbi, fex->fe_len);
+}
+
+static inline loff_t pa_logical_end(struct ext4_sb_info *sbi,
+ struct ext4_prealloc_space *pa)
+{
+ /* Use loff_t to avoid end exceeding ext4_lblk_t max. */
+ return (loff_t)pa->pa_lstart + EXT4_C2B(sbi, pa->pa_len);
+}
+
typedef int (*ext4_mballoc_query_range_fn)(
struct super_block *sb,
ext4_group_t agno,
--
2.31.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 2/3] ext4: fix BUG in ext4_mb_new_inode_pa() due to overflow
2023-07-24 12:10 [PATCH v2 0/3] ext4: fix some ext4_lblk_t overflow issues Baokun Li
2023-07-24 12:10 ` [PATCH v2 1/3] ext4: add two helper functions extent_logical_end() and pa_logical_end() Baokun Li
@ 2023-07-24 12:10 ` Baokun Li
2023-07-25 17:29 ` Ritesh Harjani
2023-08-03 13:58 ` Jan Kara
2023-07-24 12:10 ` [PATCH v2 3/3] ext4: avoid overlapping preallocations " Baokun Li
2023-08-03 14:37 ` [PATCH v2 0/3] ext4: fix some ext4_lblk_t overflow issues Theodore Ts'o
3 siblings, 2 replies; 12+ messages in thread
From: Baokun Li @ 2023-07-24 12:10 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, adilger.kernel, jack, ritesh.list, ojaswin, linux-kernel,
yi.zhang, yangerkun, yukuai3, libaokun1, stable
When we calculate the end position of ext4_free_extent, this position may
be exactly where ext4_lblk_t (i.e. uint) overflows. For example, if
ac_g_ex.fe_logical is 4294965248 and ac_orig_goal_len is 2048, then the
computed end is 0x100000000, which is 0. If ac->ac_o_ex.fe_logical is not
the first case of adjusting the best extent, that is, new_bex_end > 0, the
following BUG_ON will be triggered:
=========================================================
kernel BUG at fs/ext4/mballoc.c:5116!
invalid opcode: 0000 [#1] PREEMPT SMP PTI
CPU: 3 PID: 673 Comm: xfs_io Tainted: G E 6.5.0-rc1+ #279
RIP: 0010:ext4_mb_new_inode_pa+0xc5/0x430
Call Trace:
<TASK>
ext4_mb_use_best_found+0x203/0x2f0
ext4_mb_try_best_found+0x163/0x240
ext4_mb_regular_allocator+0x158/0x1550
ext4_mb_new_blocks+0x86a/0xe10
ext4_ext_map_blocks+0xb0c/0x13a0
ext4_map_blocks+0x2cd/0x8f0
ext4_iomap_begin+0x27b/0x400
iomap_iter+0x222/0x3d0
__iomap_dio_rw+0x243/0xcb0
iomap_dio_rw+0x16/0x80
=========================================================
A simple reproducer demonstrating the problem:
mkfs.ext4 -F /dev/sda -b 4096 100M
mount /dev/sda /tmp/test
fallocate -l1M /tmp/test/tmp
fallocate -l10M /tmp/test/file
fallocate -i -o 1M -l16777203M /tmp/test/file
fsstress -d /tmp/test -l 0 -n 100000 -p 8 &
sleep 10 && killall -9 fsstress
rm -f /tmp/test/tmp
xfs_io -c "open -ad /tmp/test/file" -c "pwrite -S 0xff 0 8192"
We simply refactor the logic for adjusting the best extent by adding
a temporary ext4_free_extent ex and use extent_logical_end() to avoid
overflow, which also simplifies the code.
Cc: stable@kernel.org # 6.4
Fixes: 93cdf49f6eca ("ext4: Fix best extent lstart adjustment logic in ext4_mb_new_inode_pa()")
Signed-off-by: Baokun Li <libaokun1@huawei.com>
---
fs/ext4/mballoc.c | 31 ++++++++++++++-----------------
1 file changed, 14 insertions(+), 17 deletions(-)
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 4cb13b3e41b3..86bce870dc5a 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -5177,8 +5177,11 @@ ext4_mb_new_inode_pa(struct ext4_allocation_context *ac)
pa = ac->ac_pa;
if (ac->ac_b_ex.fe_len < ac->ac_orig_goal_len) {
- int new_bex_start;
- int new_bex_end;
+ struct ext4_free_extent ex = {
+ .fe_logical = ac->ac_g_ex.fe_logical,
+ .fe_len = ac->ac_orig_goal_len,
+ };
+ loff_t orig_goal_end = extent_logical_end(sbi, &ex);
/* we can't allocate as much as normalizer wants.
* so, found space must get proper lstart
@@ -5197,29 +5200,23 @@ ext4_mb_new_inode_pa(struct ext4_allocation_context *ac)
* still cover original start
* 3. Else, keep the best ex at start of original request.
*/
- new_bex_end = ac->ac_g_ex.fe_logical +
- EXT4_C2B(sbi, ac->ac_orig_goal_len);
- new_bex_start = new_bex_end - EXT4_C2B(sbi, ac->ac_b_ex.fe_len);
- if (ac->ac_o_ex.fe_logical >= new_bex_start)
- goto adjust_bex;
+ ex.fe_len = ac->ac_b_ex.fe_len;
- new_bex_start = ac->ac_g_ex.fe_logical;
- new_bex_end =
- new_bex_start + EXT4_C2B(sbi, ac->ac_b_ex.fe_len);
- if (ac->ac_o_ex.fe_logical < new_bex_end)
+ ex.fe_logical = orig_goal_end - EXT4_C2B(sbi, ex.fe_len);
+ if (ac->ac_o_ex.fe_logical >= ex.fe_logical)
goto adjust_bex;
- new_bex_start = ac->ac_o_ex.fe_logical;
- new_bex_end =
- new_bex_start + EXT4_C2B(sbi, ac->ac_b_ex.fe_len);
+ ex.fe_logical = ac->ac_g_ex.fe_logical;
+ if (ac->ac_o_ex.fe_logical < extent_logical_end(sbi, &ex))
+ goto adjust_bex;
+ ex.fe_logical = ac->ac_o_ex.fe_logical;
adjust_bex:
- ac->ac_b_ex.fe_logical = new_bex_start;
+ ac->ac_b_ex.fe_logical = ex.fe_logical;
BUG_ON(ac->ac_o_ex.fe_logical < ac->ac_b_ex.fe_logical);
BUG_ON(ac->ac_o_ex.fe_len > ac->ac_b_ex.fe_len);
- BUG_ON(new_bex_end > (ac->ac_g_ex.fe_logical +
- EXT4_C2B(sbi, ac->ac_orig_goal_len)));
+ BUG_ON(extent_logical_end(sbi, &ex) > orig_goal_end);
}
pa->pa_lstart = ac->ac_b_ex.fe_logical;
--
2.31.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 3/3] ext4: avoid overlapping preallocations due to overflow
2023-07-24 12:10 [PATCH v2 0/3] ext4: fix some ext4_lblk_t overflow issues Baokun Li
2023-07-24 12:10 ` [PATCH v2 1/3] ext4: add two helper functions extent_logical_end() and pa_logical_end() Baokun Li
2023-07-24 12:10 ` [PATCH v2 2/3] ext4: fix BUG in ext4_mb_new_inode_pa() due to overflow Baokun Li
@ 2023-07-24 12:10 ` Baokun Li
2023-07-25 17:30 ` Ritesh Harjani
2023-08-03 13:58 ` Jan Kara
2023-08-03 14:37 ` [PATCH v2 0/3] ext4: fix some ext4_lblk_t overflow issues Theodore Ts'o
3 siblings, 2 replies; 12+ messages in thread
From: Baokun Li @ 2023-07-24 12:10 UTC (permalink / raw)
To: linux-ext4
Cc: tytso, adilger.kernel, jack, ritesh.list, ojaswin, linux-kernel,
yi.zhang, yangerkun, yukuai3, libaokun1
Let's say we want to allocate 2 blocks starting from 4294966386, after
predicting the file size, start is aligned to 4294965248, len is changed
to 2048, then end = start + size = 0x100000000. Since end is of
type ext4_lblk_t, i.e. uint, end is truncated to 0.
This causes (pa->pa_lstart >= end) to always hold when checking if the
current extent to be allocated crosses already preallocated blocks, so the
resulting ac_g_ex may cross already preallocated blocks. Hence we convert
the end type to loff_t and use pa_logical_end() to avoid overflow.
Signed-off-by: Baokun Li <libaokun1@huawei.com>
---
fs/ext4/mballoc.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
index 86bce870dc5a..78a4a24e2f57 100644
--- a/fs/ext4/mballoc.c
+++ b/fs/ext4/mballoc.c
@@ -4222,12 +4222,13 @@ ext4_mb_pa_rb_next_iter(ext4_lblk_t new_start, ext4_lblk_t cur_start, struct rb_
static inline void
ext4_mb_pa_assert_overlap(struct ext4_allocation_context *ac,
- ext4_lblk_t start, ext4_lblk_t end)
+ ext4_lblk_t start, loff_t end)
{
struct ext4_sb_info *sbi = EXT4_SB(ac->ac_sb);
struct ext4_inode_info *ei = EXT4_I(ac->ac_inode);
struct ext4_prealloc_space *tmp_pa;
- ext4_lblk_t tmp_pa_start, tmp_pa_end;
+ ext4_lblk_t tmp_pa_start;
+ loff_t tmp_pa_end;
struct rb_node *iter;
read_lock(&ei->i_prealloc_lock);
@@ -4236,7 +4237,7 @@ ext4_mb_pa_assert_overlap(struct ext4_allocation_context *ac,
tmp_pa = rb_entry(iter, struct ext4_prealloc_space,
pa_node.inode_node);
tmp_pa_start = tmp_pa->pa_lstart;
- tmp_pa_end = tmp_pa->pa_lstart + EXT4_C2B(sbi, tmp_pa->pa_len);
+ tmp_pa_end = pa_logical_end(sbi, tmp_pa);
spin_lock(&tmp_pa->pa_lock);
if (tmp_pa->pa_deleted == 0)
@@ -4258,14 +4259,14 @@ ext4_mb_pa_assert_overlap(struct ext4_allocation_context *ac,
*/
static inline void
ext4_mb_pa_adjust_overlap(struct ext4_allocation_context *ac,
- ext4_lblk_t *start, ext4_lblk_t *end)
+ ext4_lblk_t *start, loff_t *end)
{
struct ext4_inode_info *ei = EXT4_I(ac->ac_inode);
struct ext4_sb_info *sbi = EXT4_SB(ac->ac_sb);
struct ext4_prealloc_space *tmp_pa = NULL, *left_pa = NULL, *right_pa = NULL;
struct rb_node *iter;
- ext4_lblk_t new_start, new_end;
- ext4_lblk_t tmp_pa_start, tmp_pa_end, left_pa_end = -1, right_pa_start = -1;
+ ext4_lblk_t new_start, tmp_pa_start, right_pa_start = -1;
+ loff_t new_end, tmp_pa_end, left_pa_end = -1;
new_start = *start;
new_end = *end;
@@ -4284,7 +4285,7 @@ ext4_mb_pa_adjust_overlap(struct ext4_allocation_context *ac,
tmp_pa = rb_entry(iter, struct ext4_prealloc_space,
pa_node.inode_node);
tmp_pa_start = tmp_pa->pa_lstart;
- tmp_pa_end = tmp_pa->pa_lstart + EXT4_C2B(sbi, tmp_pa->pa_len);
+ tmp_pa_end = pa_logical_end(sbi, tmp_pa);
/* PA must not overlap original request */
spin_lock(&tmp_pa->pa_lock);
@@ -4364,8 +4365,7 @@ ext4_mb_pa_adjust_overlap(struct ext4_allocation_context *ac,
}
if (left_pa) {
- left_pa_end =
- left_pa->pa_lstart + EXT4_C2B(sbi, left_pa->pa_len);
+ left_pa_end = pa_logical_end(sbi, left_pa);
BUG_ON(left_pa_end > ac->ac_o_ex.fe_logical);
}
@@ -4404,8 +4404,7 @@ ext4_mb_normalize_request(struct ext4_allocation_context *ac,
struct ext4_sb_info *sbi = EXT4_SB(ac->ac_sb);
struct ext4_super_block *es = sbi->s_es;
int bsbits, max;
- ext4_lblk_t end;
- loff_t size, start_off;
+ loff_t size, start_off, end;
loff_t orig_size __maybe_unused;
ext4_lblk_t start;
--
2.31.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/3] ext4: add two helper functions extent_logical_end() and pa_logical_end()
2023-07-24 12:10 ` [PATCH v2 1/3] ext4: add two helper functions extent_logical_end() and pa_logical_end() Baokun Li
@ 2023-07-25 17:29 ` Ritesh Harjani
2023-07-26 1:17 ` Baokun Li
2023-08-03 13:58 ` Jan Kara
1 sibling, 1 reply; 12+ messages in thread
From: Ritesh Harjani @ 2023-07-25 17:29 UTC (permalink / raw)
To: Baokun Li, linux-ext4
Cc: tytso, adilger.kernel, jack, ojaswin, linux-kernel, yi.zhang,
yangerkun, yukuai3, libaokun1
Baokun Li <libaokun1@huawei.com> writes:
> When we use lstart + len to calculate the end of free extent or prealloc
> space, it may exceed the maximum value of 4294967295(0xffffffff) supported
> by ext4_lblk_t and cause overflow, which may lead to various problems.
>
> Therefore, we add two helper functions, extent_logical_end() and
> pa_logical_end(), to limit the type of end to loff_t, and also convert
> lstart to loff_t for calculation to avoid overflow.
Sure. extent_logical_end() is not as bad after dropping the third param.
Thanks for addressing review comments and identifying overflow issues :)
Looks good to me. Feel free to add:
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
>
> Signed-off-by: Baokun Li <libaokun1@huawei.com>
> ---
> fs/ext4/mballoc.c | 9 +++------
> fs/ext4/mballoc.h | 14 ++++++++++++++
> 2 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 21b903fe546e..4cb13b3e41b3 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -4432,7 +4432,7 @@ ext4_mb_normalize_request(struct ext4_allocation_context *ac,
>
> /* first, let's learn actual file size
> * given current request is allocated */
> - size = ac->ac_o_ex.fe_logical + EXT4_C2B(sbi, ac->ac_o_ex.fe_len);
> + size = extent_logical_end(sbi, &ac->ac_o_ex);
> size = size << bsbits;
> if (size < i_size_read(ac->ac_inode))
> size = i_size_read(ac->ac_inode);
> @@ -4766,7 +4766,6 @@ ext4_mb_use_preallocated(struct ext4_allocation_context *ac)
> struct ext4_inode_info *ei = EXT4_I(ac->ac_inode);
> struct ext4_locality_group *lg;
> struct ext4_prealloc_space *tmp_pa = NULL, *cpa = NULL;
> - loff_t tmp_pa_end;
> struct rb_node *iter;
> ext4_fsblk_t goal_block;
>
> @@ -4862,9 +4861,7 @@ ext4_mb_use_preallocated(struct ext4_allocation_context *ac)
> * pa can possibly satisfy the request hence check if it overlaps
> * original logical start and stop searching if it doesn't.
> */
> - tmp_pa_end = (loff_t)tmp_pa->pa_lstart + EXT4_C2B(sbi, tmp_pa->pa_len);
> -
> - if (ac->ac_o_ex.fe_logical >= tmp_pa_end) {
> + if (ac->ac_o_ex.fe_logical >= pa_logical_end(sbi, tmp_pa)) {
> spin_unlock(&tmp_pa->pa_lock);
> goto try_group_pa;
> }
> @@ -5769,7 +5766,7 @@ static void ext4_mb_group_or_file(struct ext4_allocation_context *ac)
>
> group_pa_eligible = sbi->s_mb_group_prealloc > 0;
> inode_pa_eligible = true;
> - size = ac->ac_o_ex.fe_logical + EXT4_C2B(sbi, ac->ac_o_ex.fe_len);
> + size = extent_logical_end(sbi, &ac->ac_o_ex);
> isize = (i_size_read(ac->ac_inode) + ac->ac_sb->s_blocksize - 1)
> >> bsbits;
>
> diff --git a/fs/ext4/mballoc.h b/fs/ext4/mballoc.h
> index df6b5e7c2274..d7aeb5da7d86 100644
> --- a/fs/ext4/mballoc.h
> +++ b/fs/ext4/mballoc.h
> @@ -233,6 +233,20 @@ static inline ext4_fsblk_t ext4_grp_offs_to_block(struct super_block *sb,
> (fex->fe_start << EXT4_SB(sb)->s_cluster_bits);
> }
>
> +static inline loff_t extent_logical_end(struct ext4_sb_info *sbi,
> + struct ext4_free_extent *fex)
> +{
> + /* Use loff_t to avoid end exceeding ext4_lblk_t max. */
> + return (loff_t)fex->fe_logical + EXT4_C2B(sbi, fex->fe_len);
> +}
> +
> +static inline loff_t pa_logical_end(struct ext4_sb_info *sbi,
> + struct ext4_prealloc_space *pa)
> +{
> + /* Use loff_t to avoid end exceeding ext4_lblk_t max. */
> + return (loff_t)pa->pa_lstart + EXT4_C2B(sbi, pa->pa_len);
> +}
> +
> typedef int (*ext4_mballoc_query_range_fn)(
> struct super_block *sb,
> ext4_group_t agno,
> --
> 2.31.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/3] ext4: fix BUG in ext4_mb_new_inode_pa() due to overflow
2023-07-24 12:10 ` [PATCH v2 2/3] ext4: fix BUG in ext4_mb_new_inode_pa() due to overflow Baokun Li
@ 2023-07-25 17:29 ` Ritesh Harjani
2023-08-03 13:58 ` Jan Kara
1 sibling, 0 replies; 12+ messages in thread
From: Ritesh Harjani @ 2023-07-25 17:29 UTC (permalink / raw)
To: Baokun Li, linux-ext4
Cc: tytso, adilger.kernel, jack, ojaswin, linux-kernel, yi.zhang,
yangerkun, yukuai3, libaokun1, stable
Baokun Li <libaokun1@huawei.com> writes:
> When we calculate the end position of ext4_free_extent, this position may
> be exactly where ext4_lblk_t (i.e. uint) overflows. For example, if
> ac_g_ex.fe_logical is 4294965248 and ac_orig_goal_len is 2048, then the
> computed end is 0x100000000, which is 0. If ac->ac_o_ex.fe_logical is not
> the first case of adjusting the best extent, that is, new_bex_end > 0, the
> following BUG_ON will be triggered:
>
> =========================================================
> kernel BUG at fs/ext4/mballoc.c:5116!
> invalid opcode: 0000 [#1] PREEMPT SMP PTI
> CPU: 3 PID: 673 Comm: xfs_io Tainted: G E 6.5.0-rc1+ #279
> RIP: 0010:ext4_mb_new_inode_pa+0xc5/0x430
> Call Trace:
> <TASK>
> ext4_mb_use_best_found+0x203/0x2f0
> ext4_mb_try_best_found+0x163/0x240
> ext4_mb_regular_allocator+0x158/0x1550
> ext4_mb_new_blocks+0x86a/0xe10
> ext4_ext_map_blocks+0xb0c/0x13a0
> ext4_map_blocks+0x2cd/0x8f0
> ext4_iomap_begin+0x27b/0x400
> iomap_iter+0x222/0x3d0
> __iomap_dio_rw+0x243/0xcb0
> iomap_dio_rw+0x16/0x80
> =========================================================
>
> A simple reproducer demonstrating the problem:
>
> mkfs.ext4 -F /dev/sda -b 4096 100M
> mount /dev/sda /tmp/test
> fallocate -l1M /tmp/test/tmp
> fallocate -l10M /tmp/test/file
> fallocate -i -o 1M -l16777203M /tmp/test/file
> fsstress -d /tmp/test -l 0 -n 100000 -p 8 &
> sleep 10 && killall -9 fsstress
> rm -f /tmp/test/tmp
> xfs_io -c "open -ad /tmp/test/file" -c "pwrite -S 0xff 0 8192"
>
> We simply refactor the logic for adjusting the best extent by adding
> a temporary ext4_free_extent ex and use extent_logical_end() to avoid
> overflow, which also simplifies the code.
>
> Cc: stable@kernel.org # 6.4
> Fixes: 93cdf49f6eca ("ext4: Fix best extent lstart adjustment logic in ext4_mb_new_inode_pa()")
> Signed-off-by: Baokun Li <libaokun1@huawei.com>
> ---
> fs/ext4/mballoc.c | 31 ++++++++++++++-----------------
> 1 file changed, 14 insertions(+), 17 deletions(-)
Looks good to me. Feel free to add:
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 3/3] ext4: avoid overlapping preallocations due to overflow
2023-07-24 12:10 ` [PATCH v2 3/3] ext4: avoid overlapping preallocations " Baokun Li
@ 2023-07-25 17:30 ` Ritesh Harjani
2023-08-03 13:58 ` Jan Kara
1 sibling, 0 replies; 12+ messages in thread
From: Ritesh Harjani @ 2023-07-25 17:30 UTC (permalink / raw)
To: Baokun Li, linux-ext4
Cc: tytso, adilger.kernel, jack, ojaswin, linux-kernel, yi.zhang,
yangerkun, yukuai3, libaokun1
Baokun Li <libaokun1@huawei.com> writes:
> Let's say we want to allocate 2 blocks starting from 4294966386, after
> predicting the file size, start is aligned to 4294965248, len is changed
> to 2048, then end = start + size = 0x100000000. Since end is of
> type ext4_lblk_t, i.e. uint, end is truncated to 0.
>
> This causes (pa->pa_lstart >= end) to always hold when checking if the
> current extent to be allocated crosses already preallocated blocks, so the
> resulting ac_g_ex may cross already preallocated blocks. Hence we convert
> the end type to loff_t and use pa_logical_end() to avoid overflow.
>
> Signed-off-by: Baokun Li <libaokun1@huawei.com>
> ---
> fs/ext4/mballoc.c | 21 ++++++++++-----------
> 1 file changed, 10 insertions(+), 11 deletions(-)
Looks good to me. Feel free to add:
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
-ritesh
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/3] ext4: add two helper functions extent_logical_end() and pa_logical_end()
2023-07-25 17:29 ` Ritesh Harjani
@ 2023-07-26 1:17 ` Baokun Li
0 siblings, 0 replies; 12+ messages in thread
From: Baokun Li @ 2023-07-26 1:17 UTC (permalink / raw)
To: Ritesh Harjani (IBM), linux-ext4
Cc: tytso, adilger.kernel, jack, ojaswin, linux-kernel, yi.zhang,
yangerkun, yukuai3, Baokun Li
On 2023/7/26 1:29, Ritesh Harjani (IBM) wrote:
> Baokun Li <libaokun1@huawei.com> writes:
>
>> When we use lstart + len to calculate the end of free extent or prealloc
>> space, it may exceed the maximum value of 4294967295(0xffffffff) supported
>> by ext4_lblk_t and cause overflow, which may lead to various problems.
>>
>> Therefore, we add two helper functions, extent_logical_end() and
>> pa_logical_end(), to limit the type of end to loff_t, and also convert
>> lstart to loff_t for calculation to avoid overflow.
> Sure. extent_logical_end() is not as bad after dropping the third param.
> Thanks for addressing review comments and identifying overflow issues :)
>
> Looks good to me. Feel free to add:
>
> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
>
Thank you very much for your patient review! 😊
>> Signed-off-by: Baokun Li <libaokun1@huawei.com>
>> ---
>> fs/ext4/mballoc.c | 9 +++------
>> fs/ext4/mballoc.h | 14 ++++++++++++++
>> 2 files changed, 17 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
>> index 21b903fe546e..4cb13b3e41b3 100644
>> --- a/fs/ext4/mballoc.c
>> +++ b/fs/ext4/mballoc.c
>> @@ -4432,7 +4432,7 @@ ext4_mb_normalize_request(struct ext4_allocation_context *ac,
>>
>> /* first, let's learn actual file size
>> * given current request is allocated */
>> - size = ac->ac_o_ex.fe_logical + EXT4_C2B(sbi, ac->ac_o_ex.fe_len);
>> + size = extent_logical_end(sbi, &ac->ac_o_ex);
>> size = size << bsbits;
>> if (size < i_size_read(ac->ac_inode))
>> size = i_size_read(ac->ac_inode);
>> @@ -4766,7 +4766,6 @@ ext4_mb_use_preallocated(struct ext4_allocation_context *ac)
>> struct ext4_inode_info *ei = EXT4_I(ac->ac_inode);
>> struct ext4_locality_group *lg;
>> struct ext4_prealloc_space *tmp_pa = NULL, *cpa = NULL;
>> - loff_t tmp_pa_end;
>> struct rb_node *iter;
>> ext4_fsblk_t goal_block;
>>
>> @@ -4862,9 +4861,7 @@ ext4_mb_use_preallocated(struct ext4_allocation_context *ac)
>> * pa can possibly satisfy the request hence check if it overlaps
>> * original logical start and stop searching if it doesn't.
>> */
>> - tmp_pa_end = (loff_t)tmp_pa->pa_lstart + EXT4_C2B(sbi, tmp_pa->pa_len);
>> -
>> - if (ac->ac_o_ex.fe_logical >= tmp_pa_end) {
>> + if (ac->ac_o_ex.fe_logical >= pa_logical_end(sbi, tmp_pa)) {
>> spin_unlock(&tmp_pa->pa_lock);
>> goto try_group_pa;
>> }
>> @@ -5769,7 +5766,7 @@ static void ext4_mb_group_or_file(struct ext4_allocation_context *ac)
>>
>> group_pa_eligible = sbi->s_mb_group_prealloc > 0;
>> inode_pa_eligible = true;
>> - size = ac->ac_o_ex.fe_logical + EXT4_C2B(sbi, ac->ac_o_ex.fe_len);
>> + size = extent_logical_end(sbi, &ac->ac_o_ex);
>> isize = (i_size_read(ac->ac_inode) + ac->ac_sb->s_blocksize - 1)
>> >> bsbits;
>>
>> diff --git a/fs/ext4/mballoc.h b/fs/ext4/mballoc.h
>> index df6b5e7c2274..d7aeb5da7d86 100644
>> --- a/fs/ext4/mballoc.h
>> +++ b/fs/ext4/mballoc.h
>> @@ -233,6 +233,20 @@ static inline ext4_fsblk_t ext4_grp_offs_to_block(struct super_block *sb,
>> (fex->fe_start << EXT4_SB(sb)->s_cluster_bits);
>> }
>>
>> +static inline loff_t extent_logical_end(struct ext4_sb_info *sbi,
>> + struct ext4_free_extent *fex)
>> +{
>> + /* Use loff_t to avoid end exceeding ext4_lblk_t max. */
>> + return (loff_t)fex->fe_logical + EXT4_C2B(sbi, fex->fe_len);
>> +}
>> +
>> +static inline loff_t pa_logical_end(struct ext4_sb_info *sbi,
>> + struct ext4_prealloc_space *pa)
>> +{
>> + /* Use loff_t to avoid end exceeding ext4_lblk_t max. */
>> + return (loff_t)pa->pa_lstart + EXT4_C2B(sbi, pa->pa_len);
>> +}
>> +
>> typedef int (*ext4_mballoc_query_range_fn)(
>> struct super_block *sb,
>> ext4_group_t agno,
>> --
>> 2.31.1
Cheers!
--
With Best Regards,
Baokun Li
.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/3] ext4: add two helper functions extent_logical_end() and pa_logical_end()
2023-07-24 12:10 ` [PATCH v2 1/3] ext4: add two helper functions extent_logical_end() and pa_logical_end() Baokun Li
2023-07-25 17:29 ` Ritesh Harjani
@ 2023-08-03 13:58 ` Jan Kara
1 sibling, 0 replies; 12+ messages in thread
From: Jan Kara @ 2023-08-03 13:58 UTC (permalink / raw)
To: Baokun Li
Cc: linux-ext4, tytso, adilger.kernel, jack, ritesh.list, ojaswin,
linux-kernel, yi.zhang, yangerkun, yukuai3
On Mon 24-07-23 20:10:57, Baokun Li wrote:
> When we use lstart + len to calculate the end of free extent or prealloc
> space, it may exceed the maximum value of 4294967295(0xffffffff) supported
> by ext4_lblk_t and cause overflow, which may lead to various problems.
>
> Therefore, we add two helper functions, extent_logical_end() and
> pa_logical_end(), to limit the type of end to loff_t, and also convert
> lstart to loff_t for calculation to avoid overflow.
>
> Signed-off-by: Baokun Li <libaokun1@huawei.com>
Looks good to me. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/mballoc.c | 9 +++------
> fs/ext4/mballoc.h | 14 ++++++++++++++
> 2 files changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 21b903fe546e..4cb13b3e41b3 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -4432,7 +4432,7 @@ ext4_mb_normalize_request(struct ext4_allocation_context *ac,
>
> /* first, let's learn actual file size
> * given current request is allocated */
> - size = ac->ac_o_ex.fe_logical + EXT4_C2B(sbi, ac->ac_o_ex.fe_len);
> + size = extent_logical_end(sbi, &ac->ac_o_ex);
> size = size << bsbits;
> if (size < i_size_read(ac->ac_inode))
> size = i_size_read(ac->ac_inode);
> @@ -4766,7 +4766,6 @@ ext4_mb_use_preallocated(struct ext4_allocation_context *ac)
> struct ext4_inode_info *ei = EXT4_I(ac->ac_inode);
> struct ext4_locality_group *lg;
> struct ext4_prealloc_space *tmp_pa = NULL, *cpa = NULL;
> - loff_t tmp_pa_end;
> struct rb_node *iter;
> ext4_fsblk_t goal_block;
>
> @@ -4862,9 +4861,7 @@ ext4_mb_use_preallocated(struct ext4_allocation_context *ac)
> * pa can possibly satisfy the request hence check if it overlaps
> * original logical start and stop searching if it doesn't.
> */
> - tmp_pa_end = (loff_t)tmp_pa->pa_lstart + EXT4_C2B(sbi, tmp_pa->pa_len);
> -
> - if (ac->ac_o_ex.fe_logical >= tmp_pa_end) {
> + if (ac->ac_o_ex.fe_logical >= pa_logical_end(sbi, tmp_pa)) {
> spin_unlock(&tmp_pa->pa_lock);
> goto try_group_pa;
> }
> @@ -5769,7 +5766,7 @@ static void ext4_mb_group_or_file(struct ext4_allocation_context *ac)
>
> group_pa_eligible = sbi->s_mb_group_prealloc > 0;
> inode_pa_eligible = true;
> - size = ac->ac_o_ex.fe_logical + EXT4_C2B(sbi, ac->ac_o_ex.fe_len);
> + size = extent_logical_end(sbi, &ac->ac_o_ex);
> isize = (i_size_read(ac->ac_inode) + ac->ac_sb->s_blocksize - 1)
> >> bsbits;
>
> diff --git a/fs/ext4/mballoc.h b/fs/ext4/mballoc.h
> index df6b5e7c2274..d7aeb5da7d86 100644
> --- a/fs/ext4/mballoc.h
> +++ b/fs/ext4/mballoc.h
> @@ -233,6 +233,20 @@ static inline ext4_fsblk_t ext4_grp_offs_to_block(struct super_block *sb,
> (fex->fe_start << EXT4_SB(sb)->s_cluster_bits);
> }
>
> +static inline loff_t extent_logical_end(struct ext4_sb_info *sbi,
> + struct ext4_free_extent *fex)
> +{
> + /* Use loff_t to avoid end exceeding ext4_lblk_t max. */
> + return (loff_t)fex->fe_logical + EXT4_C2B(sbi, fex->fe_len);
> +}
> +
> +static inline loff_t pa_logical_end(struct ext4_sb_info *sbi,
> + struct ext4_prealloc_space *pa)
> +{
> + /* Use loff_t to avoid end exceeding ext4_lblk_t max. */
> + return (loff_t)pa->pa_lstart + EXT4_C2B(sbi, pa->pa_len);
> +}
> +
> typedef int (*ext4_mballoc_query_range_fn)(
> struct super_block *sb,
> ext4_group_t agno,
> --
> 2.31.1
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/3] ext4: fix BUG in ext4_mb_new_inode_pa() due to overflow
2023-07-24 12:10 ` [PATCH v2 2/3] ext4: fix BUG in ext4_mb_new_inode_pa() due to overflow Baokun Li
2023-07-25 17:29 ` Ritesh Harjani
@ 2023-08-03 13:58 ` Jan Kara
1 sibling, 0 replies; 12+ messages in thread
From: Jan Kara @ 2023-08-03 13:58 UTC (permalink / raw)
To: Baokun Li
Cc: linux-ext4, tytso, adilger.kernel, jack, ritesh.list, ojaswin,
linux-kernel, yi.zhang, yangerkun, yukuai3, stable
On Mon 24-07-23 20:10:58, Baokun Li wrote:
> When we calculate the end position of ext4_free_extent, this position may
> be exactly where ext4_lblk_t (i.e. uint) overflows. For example, if
> ac_g_ex.fe_logical is 4294965248 and ac_orig_goal_len is 2048, then the
> computed end is 0x100000000, which is 0. If ac->ac_o_ex.fe_logical is not
> the first case of adjusting the best extent, that is, new_bex_end > 0, the
> following BUG_ON will be triggered:
>
> =========================================================
> kernel BUG at fs/ext4/mballoc.c:5116!
> invalid opcode: 0000 [#1] PREEMPT SMP PTI
> CPU: 3 PID: 673 Comm: xfs_io Tainted: G E 6.5.0-rc1+ #279
> RIP: 0010:ext4_mb_new_inode_pa+0xc5/0x430
> Call Trace:
> <TASK>
> ext4_mb_use_best_found+0x203/0x2f0
> ext4_mb_try_best_found+0x163/0x240
> ext4_mb_regular_allocator+0x158/0x1550
> ext4_mb_new_blocks+0x86a/0xe10
> ext4_ext_map_blocks+0xb0c/0x13a0
> ext4_map_blocks+0x2cd/0x8f0
> ext4_iomap_begin+0x27b/0x400
> iomap_iter+0x222/0x3d0
> __iomap_dio_rw+0x243/0xcb0
> iomap_dio_rw+0x16/0x80
> =========================================================
>
> A simple reproducer demonstrating the problem:
>
> mkfs.ext4 -F /dev/sda -b 4096 100M
> mount /dev/sda /tmp/test
> fallocate -l1M /tmp/test/tmp
> fallocate -l10M /tmp/test/file
> fallocate -i -o 1M -l16777203M /tmp/test/file
> fsstress -d /tmp/test -l 0 -n 100000 -p 8 &
> sleep 10 && killall -9 fsstress
> rm -f /tmp/test/tmp
> xfs_io -c "open -ad /tmp/test/file" -c "pwrite -S 0xff 0 8192"
>
> We simply refactor the logic for adjusting the best extent by adding
> a temporary ext4_free_extent ex and use extent_logical_end() to avoid
> overflow, which also simplifies the code.
>
> Cc: stable@kernel.org # 6.4
> Fixes: 93cdf49f6eca ("ext4: Fix best extent lstart adjustment logic in ext4_mb_new_inode_pa()")
> Signed-off-by: Baokun Li <libaokun1@huawei.com>
Looks good to me. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/mballoc.c | 31 ++++++++++++++-----------------
> 1 file changed, 14 insertions(+), 17 deletions(-)
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 4cb13b3e41b3..86bce870dc5a 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -5177,8 +5177,11 @@ ext4_mb_new_inode_pa(struct ext4_allocation_context *ac)
> pa = ac->ac_pa;
>
> if (ac->ac_b_ex.fe_len < ac->ac_orig_goal_len) {
> - int new_bex_start;
> - int new_bex_end;
> + struct ext4_free_extent ex = {
> + .fe_logical = ac->ac_g_ex.fe_logical,
> + .fe_len = ac->ac_orig_goal_len,
> + };
> + loff_t orig_goal_end = extent_logical_end(sbi, &ex);
>
> /* we can't allocate as much as normalizer wants.
> * so, found space must get proper lstart
> @@ -5197,29 +5200,23 @@ ext4_mb_new_inode_pa(struct ext4_allocation_context *ac)
> * still cover original start
> * 3. Else, keep the best ex at start of original request.
> */
> - new_bex_end = ac->ac_g_ex.fe_logical +
> - EXT4_C2B(sbi, ac->ac_orig_goal_len);
> - new_bex_start = new_bex_end - EXT4_C2B(sbi, ac->ac_b_ex.fe_len);
> - if (ac->ac_o_ex.fe_logical >= new_bex_start)
> - goto adjust_bex;
> + ex.fe_len = ac->ac_b_ex.fe_len;
>
> - new_bex_start = ac->ac_g_ex.fe_logical;
> - new_bex_end =
> - new_bex_start + EXT4_C2B(sbi, ac->ac_b_ex.fe_len);
> - if (ac->ac_o_ex.fe_logical < new_bex_end)
> + ex.fe_logical = orig_goal_end - EXT4_C2B(sbi, ex.fe_len);
> + if (ac->ac_o_ex.fe_logical >= ex.fe_logical)
> goto adjust_bex;
>
> - new_bex_start = ac->ac_o_ex.fe_logical;
> - new_bex_end =
> - new_bex_start + EXT4_C2B(sbi, ac->ac_b_ex.fe_len);
> + ex.fe_logical = ac->ac_g_ex.fe_logical;
> + if (ac->ac_o_ex.fe_logical < extent_logical_end(sbi, &ex))
> + goto adjust_bex;
>
> + ex.fe_logical = ac->ac_o_ex.fe_logical;
> adjust_bex:
> - ac->ac_b_ex.fe_logical = new_bex_start;
> + ac->ac_b_ex.fe_logical = ex.fe_logical;
>
> BUG_ON(ac->ac_o_ex.fe_logical < ac->ac_b_ex.fe_logical);
> BUG_ON(ac->ac_o_ex.fe_len > ac->ac_b_ex.fe_len);
> - BUG_ON(new_bex_end > (ac->ac_g_ex.fe_logical +
> - EXT4_C2B(sbi, ac->ac_orig_goal_len)));
> + BUG_ON(extent_logical_end(sbi, &ex) > orig_goal_end);
> }
>
> pa->pa_lstart = ac->ac_b_ex.fe_logical;
> --
> 2.31.1
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 3/3] ext4: avoid overlapping preallocations due to overflow
2023-07-24 12:10 ` [PATCH v2 3/3] ext4: avoid overlapping preallocations " Baokun Li
2023-07-25 17:30 ` Ritesh Harjani
@ 2023-08-03 13:58 ` Jan Kara
1 sibling, 0 replies; 12+ messages in thread
From: Jan Kara @ 2023-08-03 13:58 UTC (permalink / raw)
To: Baokun Li
Cc: linux-ext4, tytso, adilger.kernel, jack, ritesh.list, ojaswin,
linux-kernel, yi.zhang, yangerkun, yukuai3
On Mon 24-07-23 20:10:59, Baokun Li wrote:
> Let's say we want to allocate 2 blocks starting from 4294966386, after
> predicting the file size, start is aligned to 4294965248, len is changed
> to 2048, then end = start + size = 0x100000000. Since end is of
> type ext4_lblk_t, i.e. uint, end is truncated to 0.
>
> This causes (pa->pa_lstart >= end) to always hold when checking if the
> current extent to be allocated crosses already preallocated blocks, so the
> resulting ac_g_ex may cross already preallocated blocks. Hence we convert
> the end type to loff_t and use pa_logical_end() to avoid overflow.
>
> Signed-off-by: Baokun Li <libaokun1@huawei.com>
Looks good to me. Feel free to add:
Reviewed-by: Jan Kara <jack@suse.cz>
Honza
> ---
> fs/ext4/mballoc.c | 21 ++++++++++-----------
> 1 file changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 86bce870dc5a..78a4a24e2f57 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -4222,12 +4222,13 @@ ext4_mb_pa_rb_next_iter(ext4_lblk_t new_start, ext4_lblk_t cur_start, struct rb_
>
> static inline void
> ext4_mb_pa_assert_overlap(struct ext4_allocation_context *ac,
> - ext4_lblk_t start, ext4_lblk_t end)
> + ext4_lblk_t start, loff_t end)
> {
> struct ext4_sb_info *sbi = EXT4_SB(ac->ac_sb);
> struct ext4_inode_info *ei = EXT4_I(ac->ac_inode);
> struct ext4_prealloc_space *tmp_pa;
> - ext4_lblk_t tmp_pa_start, tmp_pa_end;
> + ext4_lblk_t tmp_pa_start;
> + loff_t tmp_pa_end;
> struct rb_node *iter;
>
> read_lock(&ei->i_prealloc_lock);
> @@ -4236,7 +4237,7 @@ ext4_mb_pa_assert_overlap(struct ext4_allocation_context *ac,
> tmp_pa = rb_entry(iter, struct ext4_prealloc_space,
> pa_node.inode_node);
> tmp_pa_start = tmp_pa->pa_lstart;
> - tmp_pa_end = tmp_pa->pa_lstart + EXT4_C2B(sbi, tmp_pa->pa_len);
> + tmp_pa_end = pa_logical_end(sbi, tmp_pa);
>
> spin_lock(&tmp_pa->pa_lock);
> if (tmp_pa->pa_deleted == 0)
> @@ -4258,14 +4259,14 @@ ext4_mb_pa_assert_overlap(struct ext4_allocation_context *ac,
> */
> static inline void
> ext4_mb_pa_adjust_overlap(struct ext4_allocation_context *ac,
> - ext4_lblk_t *start, ext4_lblk_t *end)
> + ext4_lblk_t *start, loff_t *end)
> {
> struct ext4_inode_info *ei = EXT4_I(ac->ac_inode);
> struct ext4_sb_info *sbi = EXT4_SB(ac->ac_sb);
> struct ext4_prealloc_space *tmp_pa = NULL, *left_pa = NULL, *right_pa = NULL;
> struct rb_node *iter;
> - ext4_lblk_t new_start, new_end;
> - ext4_lblk_t tmp_pa_start, tmp_pa_end, left_pa_end = -1, right_pa_start = -1;
> + ext4_lblk_t new_start, tmp_pa_start, right_pa_start = -1;
> + loff_t new_end, tmp_pa_end, left_pa_end = -1;
>
> new_start = *start;
> new_end = *end;
> @@ -4284,7 +4285,7 @@ ext4_mb_pa_adjust_overlap(struct ext4_allocation_context *ac,
> tmp_pa = rb_entry(iter, struct ext4_prealloc_space,
> pa_node.inode_node);
> tmp_pa_start = tmp_pa->pa_lstart;
> - tmp_pa_end = tmp_pa->pa_lstart + EXT4_C2B(sbi, tmp_pa->pa_len);
> + tmp_pa_end = pa_logical_end(sbi, tmp_pa);
>
> /* PA must not overlap original request */
> spin_lock(&tmp_pa->pa_lock);
> @@ -4364,8 +4365,7 @@ ext4_mb_pa_adjust_overlap(struct ext4_allocation_context *ac,
> }
>
> if (left_pa) {
> - left_pa_end =
> - left_pa->pa_lstart + EXT4_C2B(sbi, left_pa->pa_len);
> + left_pa_end = pa_logical_end(sbi, left_pa);
> BUG_ON(left_pa_end > ac->ac_o_ex.fe_logical);
> }
>
> @@ -4404,8 +4404,7 @@ ext4_mb_normalize_request(struct ext4_allocation_context *ac,
> struct ext4_sb_info *sbi = EXT4_SB(ac->ac_sb);
> struct ext4_super_block *es = sbi->s_es;
> int bsbits, max;
> - ext4_lblk_t end;
> - loff_t size, start_off;
> + loff_t size, start_off, end;
> loff_t orig_size __maybe_unused;
> ext4_lblk_t start;
>
> --
> 2.31.1
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 0/3] ext4: fix some ext4_lblk_t overflow issues
2023-07-24 12:10 [PATCH v2 0/3] ext4: fix some ext4_lblk_t overflow issues Baokun Li
` (2 preceding siblings ...)
2023-07-24 12:10 ` [PATCH v2 3/3] ext4: avoid overlapping preallocations " Baokun Li
@ 2023-08-03 14:37 ` Theodore Ts'o
3 siblings, 0 replies; 12+ messages in thread
From: Theodore Ts'o @ 2023-08-03 14:37 UTC (permalink / raw)
To: linux-ext4, Baokun Li
Cc: Theodore Ts'o, adilger.kernel, jack, ritesh.list, ojaswin,
linux-kernel, yi.zhang, yangerkun, yukuai3
On Mon, 24 Jul 2023 20:10:56 +0800, Baokun Li wrote:
> Changes since v1:
> * Rename fex_end() and pa_end() to extent_logical_end() and pa_logical_end()
> to make the code more readable.
> * Refactor the logic for adjusting the best extent in ext4_mb_new_inode_pa()
> to simplify the code and remove redundant parameter for helper function.
> * Merged patch 4 to patch 1 as mainline commit 9d3de7ee192a fixed the issue.
>
> [...]
Applied, thanks!
[1/3] ext4: add two helper functions extent_logical_end() and pa_logical_end()
commit: 43bbddc067883d94de7a43d5756a295439fbe37d
[2/3] ext4: fix BUG in ext4_mb_new_inode_pa() due to overflow
commit: bc056e7163ac7db945366de219745cf94f32a3e6
[3/3] ext4: avoid overlapping preallocations due to overflow
commit: bedc5d34632c21b5adb8ca7143d4c1f794507e4c
Best regards,
--
Theodore Ts'o <tytso@mit.edu>
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2023-08-03 14:39 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-24 12:10 [PATCH v2 0/3] ext4: fix some ext4_lblk_t overflow issues Baokun Li
2023-07-24 12:10 ` [PATCH v2 1/3] ext4: add two helper functions extent_logical_end() and pa_logical_end() Baokun Li
2023-07-25 17:29 ` Ritesh Harjani
2023-07-26 1:17 ` Baokun Li
2023-08-03 13:58 ` Jan Kara
2023-07-24 12:10 ` [PATCH v2 2/3] ext4: fix BUG in ext4_mb_new_inode_pa() due to overflow Baokun Li
2023-07-25 17:29 ` Ritesh Harjani
2023-08-03 13:58 ` Jan Kara
2023-07-24 12:10 ` [PATCH v2 3/3] ext4: avoid overlapping preallocations " Baokun Li
2023-07-25 17:30 ` Ritesh Harjani
2023-08-03 13:58 ` Jan Kara
2023-08-03 14:37 ` [PATCH v2 0/3] ext4: fix some ext4_lblk_t overflow issues Theodore Ts'o
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).