linux-kernel - Re: [PATCH AUTOSEL 6.14 099/642] btrfs: prevent inline data extents read from touching blocks beyond its range

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]

Message-ID: <aCyObd3OCtX9K-Jd@lappy>
Date: Tue, 20 May 2025 10:15:09 -0400
From: Sasha Levin <sashal@...nel.org>
To: David Sterba <dsterba@...e.cz>
Cc: linux-kernel@...r.kernel.org, stable@...r.kernel.org,
	Qu Wenruo <wqu@...e.com>, Filipe Manana <fdmanana@...e.com>,
	David Sterba <dsterba@...e.com>, clm@...com, josef@...icpanda.com,
	linux-btrfs@...r.kernel.org
Subject: Re: [PATCH AUTOSEL 6.14 099/642] btrfs: prevent inline data extents
 read from touching blocks beyond its range

On Tue, May 06, 2025 at 03:19:13PM +0200, David Sterba wrote:
>On Mon, May 05, 2025 at 06:05:15PM -0400, Sasha Levin wrote:
>> From: Qu Wenruo <wqu@...e.com>
>>
>> [ Upstream commit 1a5b5668d711d3d1ef447446beab920826decec3 ]
>>
>> Currently reading an inline data extent will zero out the remaining
>> range in the page.
>>
>> This is not yet causing problems even for block size < page size
>> (subpage) cases because:
>>
>> 1) An inline data extent always starts at file offset 0
>>    Meaning at page read, we always read the inline extent first, before
>>    any other blocks in the page. Then later blocks are properly read out
>>    and re-fill the zeroed out ranges.
>>
>> 2) Currently btrfs will read out the whole page if a buffered write is
>>    not page aligned
>>    So a page is either fully uptodate at buffered write time (covers the
>>    whole page), or we will read out the whole page first.
>>    Meaning there is nothing to lose for such an inline extent read.
>>
>> But it's still not ideal:
>>
>> - We're zeroing out the page twice
>>   Once done by read_inline_extent()/uncompress_inline(), once done by
>>   btrfs_do_readpage() for ranges beyond i_size.
>>
>> - We're touching blocks that don't belong to the inline extent
>>   In the incoming patches, we can have a partial uptodate folio, of
>>   which some dirty blocks can exist while the page is not fully uptodate:
>>
>>   The page size is 16K and block size is 4K:
>>
>>   0         4K        8K        12K        16K
>>   |         |         |/////////|          |
>>
>>   And range [8K, 12K) is dirtied by a buffered write, the remaining
>>   blocks are not uptodate.
>>
>>   If range [0, 4K) contains an inline data extent, and we try to read
>>   the whole page, the current behavior will overwrite range [8K, 12K)
>>   with zero and cause data loss.
>>
>> So to make the behavior more consistent and in preparation for future
>> changes, limit the inline data extents read to only zero out the range
>> inside the first block, not the whole page.
>>
>> Reviewed-by: Filipe Manana <fdmanana@...e.com>
>> Signed-off-by: Qu Wenruo <wqu@...e.com>
>> Signed-off-by: David Sterba <dsterba@...e.com>
>> Signed-off-by: Sasha Levin <sashal@...nel.org>
>
>This is not a stable dependency and the patch is not fixing anything
>but a preparation so this does not make much sense for stable backports,
>please drop it. Thanks.

Will do, thanks!

-- 
Thanks,
Sasha