Re: [PATCH] ext4: zero non-uptodate buffers before encryption in writeback

[Jan Kara]

On Wed 24-06-26 14:59:48, Yun Zhou wrote:
> ext4_bio_write_folio() encrypts the folio content from offset 0 up to
> round_up(len, blocksize) before submitting IO. When blocksize < PAGE_SIZE,
> this range may include blocks that are not being written out (holes,
> delay, or unwritten blocks). If the folio was freshly allocated by the
> page cache (via write_begin) for a partial-page write, these non-target
> blocks may remain uninitialized from the buddy allocator.
> 
> The encryption engine (AES-XTS) then reads these uninitialized bytes as
> operands, triggering a KMSAN uninit-value report in aes_encrypt().
> 
> Fix this by zeroing any non-uptodate buffer that is not being written
> out, before calling fscrypt_encrypt_pagecache_blocks(). This ensures the
> crypto engine never operates on uninitialized data regardless of which
> blocks are actually being submitted for IO.
> 
> The common case of blocksize == PAGE_SIZE is unaffected since there can
> be no non-overlapping blocks within a single-block folio.
> 
> Reported-by: syzbot+7add5c56bc2a14145d20@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=7add5c56bc2a14145d20
> Signed-off-by: Yun Zhou <yun.zhou@xxxxxxxxxxxxx>

Eric has just sent patch set that deletes this code [1]. So I think this
patch isn't needed anymore.

[1] lore.kernel.org/20260624050334.124606-1-ebiggers@xxxxxxxxxx

								Honza

> ---
>  fs/ext4/page-io.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
> index bc674aa4a656..2d380b5a1501 100644
> --- a/fs/ext4/page-io.c
> +++ b/fs/ext4/page-io.c
> @@ -555,12 +555,22 @@ int ext4_bio_write_folio(struct ext4_io_submit *io, struct folio *folio,
>  	 * block which might be needed.  This may cause some unneeded blocks
>  	 * (e.g. holes) to be unnecessarily encrypted, but this is rare and
>  	 * can't happen in the common case of blocksize == PAGE_SIZE.
> +	 *
> +	 * Zero out any non-uptodate buffers that are not being written out,
> +	 * to prevent uninitialized memory from being fed into the crypto
> +	 * engine.
>  	 */
>  	if (fscrypt_inode_uses_fs_layer_crypto(inode)) {
>  		gfp_t gfp_flags = GFP_NOFS;
>  		unsigned int enc_bytes = round_up(len, i_blocksize(inode));
>  		struct page *bounce_page;
>  
> +		do {
> +			if (!buffer_async_write(bh) && !buffer_uptodate(bh))
> +				folio_zero_range(folio, bh_offset(bh),
> +						 bh->b_size);
> +		} while ((bh = bh->b_this_page) != head);
> +
>  		/*
>  		 * Since bounce page allocation uses a mempool, we can only use
>  		 * a waiting mask (i.e. request guaranteed allocation) on the
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR