linux-ext4 - Re: [PATCH] ext4: only update i_reserved_data

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:   Mon, 27 Mar 2023 21:09:42 +0800
From:   Baokun Li <libaokun1@...wei.com>
To:     Jan Kara <jack@...e.cz>
CC:     <linux-ext4@...r.kernel.org>, <tytso@....edu>,
        <adilger.kernel@...ger.ca>, <ritesh.list@...il.com>,
        <linux-kernel@...r.kernel.org>, <yi.zhang@...wei.com>,
        <yangerkun@...wei.com>, <yukuai3@...wei.com>,
        Baokun Li <libaokun1@...wei.com>
Subject: Re: [PATCH] ext4: only update i_reserved_data_blocks on successful
 block allocation

On 2023/3/27 20:47, Jan Kara wrote:
> On Sat 25-03-23 14:34:43, Baokun Li wrote:
>> In our fault injection test, we create an ext4 file, migrate it to
>> non-extent based file, then punch a hole and finally trigger a WARN_ON
>> in the ext4_da_update_reserve_space():
>>
>> EXT4-fs warning (device sda): ext4_da_update_reserve_space:369:
>> ino 14, used 11 with only 10 reserved data blocks
>>
>> When writing back a non-extent based file, if we enable delalloc, the
>> number of reserved blocks will be subtracted from the number of blocks
>> mapped by ext4_ind_map_blocks(), and the extent status tree will be
>> updated. We update the extent status tree by first removing the old
>> extent_status and then inserting the new extent_status. If the block range
>> we remove happens to be in an extent, then we need to allocate another
>> extent_status with ext4_es_alloc_extent().
>>
>>         use old    to remove   to add new
>>      |----------|------------|------------|
>>                old extent_status
>>
>> The problem is that the allocation of a new extent_status failed due to a
>> fault injection, and __es_shrink() did not get free memory, resulting in
>> a return of -ENOMEM. Then do_writepages() retries after receiving -ENOMEM,
>> we map to the same extent again, and the number of reserved blocks is again
>> subtracted from the number of blocks in that extent. Since the blocks in
>> the same extent are subtracted twice, we end up triggering WARN_ON at
>> ext4_da_update_reserve_space() because used > ei->i_reserved_data_blocks.
> Hum, but this second call to ext4_map_blocks() should find already allocated
> blocks in the indirect block and thus should not be subtracting
> ei->i_reserved_data_blocks for the second time. What am I missing?
>
> 								Honza
>
ext4_map_blocks
   1. Lookup extent status tree firstly
        goto found;
   2. get the block without requesting a new file system block.
found:
   3. ceate and map the block

When we call ext4_map_blocks() for the second time, we directly find the
corresponding blocks in the extent status tree, and then go directly to 
step 3,
because our flag is brand new and therefore does not contain EXT4_MAP_MAPPED
but contains EXT4_GET_BLOCKS_CREATE, thus subtracting 
ei->i_reserved_data_blocks
for the second time.

Thanks!
-- 
With Best Regards,
Baokun Li
.