linux-ext4 - Re: BUG in ext4_ind_remove

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 25 Feb 2022 11:19:19 -0800
From:   Tadeusz Struk <tadeusz.struk@...aro.org>
To:     Ritesh Harjani <riteshh@...ux.ibm.com>
Cc:     Theodore Ts'o <tytso@....edu>,
        Andreas Dilger <adilger.kernel@...ger.ca>,
        linux-ext4@...r.kernel.org
Subject: Re: BUG in ext4_ind_remove_space

On 2/25/22 09:10, Ritesh Harjani wrote:
> On 22/02/24 05:12PM, Tadeusz Struk wrote:
>>
>> Hi,
>> Syzbot found an issue [1] in fallocate() that looks to me like a loss of precision.
>> The C reproducer [2] calls fallocate() and passes the size 0xffeffeff000ul, and
>> offset 0x1000000ul, which is then used to calculate the first_block and
>> stop_block using ext4_lblk_t type (u32). I think this gets the MSB of the size
>> truncated and leads to invalid calculations, and eventually his BUG() in
>> https://elixir.bootlin.com/linux/v5.16.11/source/fs/ext4/indirect.c#L1244
>> The issue can be reproduced on 5.17.0-rc5, but I don't think it's a new
>> regression. I spent some time debugging it, but could spot anything obvious.
>> Can someone have a look please.
> 
> I did look into it a little. Below are some of my observations.
> If nobody gets to it before me, I can spend sometime next week to verify it's
> correctness.
> 
> So I think based on the warning log before kernel BUG_ON() [1], it looks like it
> has the problem with ext4_block_to_path() calculation with end offset.
> It seems it is not fitting into triple block ptrs calculation.
> 
> <log>
> ======
> EXT4-fs warning (device sda1): ext4_block_to_path:107: block 1074791436 > max in inode 1137
> 
> But ideally it should fit in (right?) since we do make sure if end >= max_block;
> then we set it to end = max_block.
> 
> Then looking at how we calculate max_block is below
> 	max_block = (EXT4_SB(inode->i_sb)->s_bitmap_maxbytes + blocksize-1) >> EXT4_BLOCK_SIZE_BITS(inode->i_sb);
> 
> So looking closely I think there _could be_ a off by 1 calculation error in
> above. So I think above calculation is for max_len and not max_end_block.
> 
> I think max_block should be max_end_block which should be this -
> 	max_end_block = ((EXT4_SB(inode->i_sb)->s_bitmap_maxbytes + blocksize-1) >> EXT4_BLOCK_SIZE_BITS(inode->i_sb))-1;
> 
> 
> But I haven't yet verified it completely. This is just my initial thought.
> Maybe others can confirm too. Or maybe there is more then one problem.
> But somehow above looks more likely to me.

I tried the above and it does help.

> 
> I can verify this sometime next week when I get back to it.
> But thanks for reporting the issue :)

Next week is perfectly fine. Thanks for looking into it.

-- 
Thanks,
Tadeusz