On Wed, Aug 14, 2024 at 10:12:00AM +0800, Edward Adam Davis wrote:
> Before determining that the goal length is a multiple of the stripe size,
> check CR_GOAL_LEN_FAST and CR_BEST_AVAIL_LEN first.
>
> Fixes: 1f6bc02f1848 ("ext4: fallback to complex scan if aligned scan doesn't work")
> Reported-and-tested-by: syzbot+1ad8bac5af24d01e2cbd@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=1ad8bac5af24d01e2cbd
> Signed-off-by: Edward Adam Davis <eadavis@xxxxxx>
> ---
> fs/ext4/mballoc.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/fs/ext4/mballoc.c b/fs/ext4/mballoc.c
> index 9dda9cd68ab2..451f92cde461 100644
> --- a/fs/ext4/mballoc.c
> +++ b/fs/ext4/mballoc.c
> @@ -2928,13 +2928,12 @@ ext4_mb_regular_allocator(struct ext4_allocation_context *ac)
> if (cr == CR_POWER2_ALIGNED)
> ext4_mb_simple_scan_group(ac, &e4b);
> else {
> - bool is_stripe_aligned = sbi->s_stripe &&
> + bool is_stripe_aligned = (cr == CR_GOAL_LEN_FAST ||
> + cr == CR_BEST_AVAIL_LEN) && sbi->s_stripe &&
> !(ac->ac_g_ex.fe_len %
> EXT4_B2C(sbi, sbi->s_stripe));
Hi Edward,
Thanks for the patch. So I didn't get a chance to look at syszcaller
report but assuming that EXT4_B2C(sbi, sbi->s_stripe) is becoming 0,
I'm not understanding how is this patch fixing the bug?
It just seems to short circuit the actual bug but we might still hit it
right?
As for EXT4_B2C(stripe) becoming zero, I have 2 observations:
1. We should definitely be using EXT4_NUM_B2C() here to make sure we
don't get 0 if stripe is less than cluster size.
2. That being saidIm not sure if it's even possible for this to become zero
because we do check that stripe size is a multiple of cluster size in
ext4_fill_super, else we disable it:
/*
* It's hard to get stripe aligned blocks if stripe is not aligned with
* cluster, just disable stripe and alert user to simpfy code and avoid
* stripe aligned allocation which will rarely successes.
*/
if (sbi->s_stripe > 0 && sbi->s_cluster_ratio > 1 &&
sbi->s_stripe % sbi->s_cluster_ratio != 0) {
ext4_msg(sb, KERN_WARNING,
"stripe (%lu) is not aligned with cluster size (%u), "
"stripe is disabled",
sbi->s_stripe, sbi->s_cluster_ratio);
sbi->s_stripe = 0;
}
I'll try to look into this a bit more and get back. Do let me know
if I missed anything.
Regards,
Ojaswin
>
> - if ((cr == CR_GOAL_LEN_FAST ||
> - cr == CR_BEST_AVAIL_LEN) &&
> - is_stripe_aligned)
> + if (is_stripe_aligned)
> ext4_mb_scan_aligned(ac, &e4b);
>
> if (ac->ac_status == AC_STATUS_CONTINUE)
> --
> 2.43.0
>
