linux-ext4 - Re: [PATCH] ext4: Fix data corruption with EXT4_GET_BLOCKS

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] [day] [month] [year] [list]

Message-ID: <20170526214456.4dihz6evugpfrlff@thunk.org>
Date:   Fri, 26 May 2017 17:44:56 -0400
From:   Theodore Ts'o <tytso@....edu>
To:     Jan Kara <jack@...e.cz>
Cc:     linux-ext4@...r.kernel.org,
        Ross Zwisler <ross.zwisler@...ux.intel.com>,
        stable@...r.kernel.org
Subject: Re: [PATCH] ext4: Fix data corruption with EXT4_GET_BLOCKS_ZERO

On Thu, May 25, 2017 at 10:43:13AM +0200, Jan Kara wrote:
> When ext4_map_blocks() is called with EXT4_GET_BLOCKS_ZERO to zero-out
> allocated blocks and these blocks are actually converted from unwritten
> extent the following race can happen:
> 
> CPU0					CPU1
> 
> page fault				page fault
> ...					...
> ext4_map_blocks()
>   ext4_ext_map_blocks()
>     ext4_ext_handle_unwritten_extents()
>       ext4_ext_convert_to_initialized()
> 	- zero out converted extent
> 	ext4_zeroout_es()
> 	  - inserts extent as initialized in status tree
> 
> 					ext4_map_blocks()
> 					  ext4_es_lookup_extent()
> 					    - finds initialized extent
> 					write data
>   ext4_issue_zeroout()
>     - zeroes out new extent overwriting data
> 
> This problem can be reproduced by generic/340 for the fallocated case
> for the last block in the file.
> 
> Fix the problem by avoiding zeroing out the area we are mapping with
> ext4_map_blocks() in ext4_ext_convert_to_initialized(). It is pointless
> to zero out this area in the first place as the caller asked us to
> convert the area to initialized because he is just going to write data
> there before the transaction finishes. To achieve this we delete the
> special case of zeroing out full extent as that will be handled by the
> cases below zeroing only the part of the extent that needs it. We also
> instruct ext4_split_extent() that the middle of extent being split
> contains data so that ext4_split_extent_at() cannot zero out full extent
> in case of ENOSPC.
> 
> CC: stable@...r.kernel.org
> Fixes: 12735f881952c32b31bc4e433768f18489f79ec9
> Signed-off-by: Jan Kara <jack@...e.cz>

Thanks, applied.

						- Ted