lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Zrw4/N7a/XrjOgtS@li-bb2b2a4c-3307-11b2-a85c-8fa5c3a69313.ibm.com>
Date: Wed, 14 Aug 2024 10:26:47 +0530
From: Ojaswin Mujoo <ojaswin@...ux.ibm.com>
To: Edward Adam Davis <eadavis@...com>
Cc: syzbot+1ad8bac5af24d01e2cbd@...kaller.appspotmail.com,
        adilger.kernel@...ger.ca, jack@...e.cz, linux-ext4@...r.kernel.org,
        linux-kernel@...r.kernel.org, syzkaller-bugs@...glegroups.com,
        tytso@....edu
Subject: Re: [PATCH] ext4: fix divide error in ext4_mb_regular_allocator

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@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=1ad8bac5af24d01e2cbd
> Signed-off-by: Edward Adam Davis <eadavis@...com>
> ---
>  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
> 


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ