[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251211221943.GN94594@frogsfrogsfrogs>
Date: Thu, 11 Dec 2025 14:19:43 -0800
From: "Darrick J. Wong" <djwong@...nel.org>
To: Eric Biggers <ebiggers@...nel.org>
Cc: syzbot <syzbot+7add5c56bc2a14145d20@...kaller.appspotmail.com>,
.davem@...emloft.net, herbert@...dor.apana.org.au,
jaegeuk@...nel.org, linux-crypto@...r.kernel.org,
linux-ext4@...r.kernel.org, linux-fscrypt@...r.kernel.org,
linux-kernel@...r.kernel.org, syzkaller-bugs@...glegroups.com,
tytso@....edu, Neal Gompa <neal@...pa.dev>
Subject: Re: [syzbot] [ext4] [fscrypt] KMSAN: uninit-value in
fscrypt_crypt_data_unit
On Thu, Dec 11, 2025 at 07:45:02PM +0000, Eric Biggers wrote:
> On Thu, Dec 11, 2025 at 10:52:15AM -0800, Darrick J. Wong wrote:
> > On Tue, Dec 09, 2025 at 06:22:02PM -0800, Eric Biggers wrote:
> > > On Tue, Dec 09, 2025 at 03:08:17AM -0800, syzbot wrote:
> > > > syzbot has found a reproducer for the following issue on:
> > > >
> > > > HEAD commit: a110f942672c Merge tag 'pinctrl-v6.19-1' of git://git.kern..
> > > > git tree: upstream
> > > > console output: https://syzkaller.appspot.com/x/log.txt?x=17495992580000
> > > > kernel config: https://syzkaller.appspot.com/x/.config?x=10d58c94af5f9772
> > > > dashboard link: https://syzkaller.appspot.com/bug?extid=7add5c56bc2a14145d20
> > > > compiler: Debian clang version 20.1.8 (++20250708063551+0c9f909b7976-1~exp1~20250708183702.136), Debian LLD 20.1.8
> > > > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1122aec2580000
> > > > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=14012a1a580000
> > >
> > > Simplified reproducer:
> > >
> > > rm -f image
> > > mkdir -p mnt
> > > mkfs.ext4 -O encrypt -b 1024 image 1M
> > > mount image mnt -o test_dummy_encryption
> > > dd if=/dev/urandom of=mnt/file bs=1 seek=1024 count=1
> > > sync
> > >
> > > It causes ext4 to encrypt uninitialized memory:
> > >
> > > BUG: KMSAN: uninit-value in crypto_aes_encrypt+0x511b/0x5260
> > > [...]
> > > fscrypt_encrypt_pagecache_blocks+0x309/0x6c0
> > > ext4_bio_write_folio+0xd2f/0x2210
> > > [...]
> > >
> > > ext4_bio_write_folio() has:
> > >
> > > /*
> > > * If any blocks are being written to an encrypted file, encrypt them
> > > * into a bounce page. For simplicity, just encrypt until the last
> > > * 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.
> > > */
> > > if (fscrypt_inode_uses_fs_layer_crypto(inode)) {
> > > gfp_t gfp_flags = GFP_NOFS;
> > > unsigned int enc_bytes = round_up(len, i_blocksize(inode));
> > >
> > > So I think that if a non-first block in a page is being written to disk
> > > and all preceding blocks in the page are holes, the (uninitialized)
> > > sections of the page corresponding to the holes are being encrypted too.
> > >
> > > This is probably "benign", as ext4 doesn't do anything with the
> > > encrypted uninitialized data. (Also note that this issue can occur only
> > > when block_size < PAGE_SIZE.)
> > >
> > > I'm not yet sure how to proceed here. We could make ext4 be more
> > > selective about encrypting the exact set of blocks in the page that are
> > > being written. That would require support in fs/crypto/ for that. We
> > > could use kmsan_unpoison_memory() to just suppress the warning.
> > >
> > > Or, we could go forward with removing support for the "fs-layer crypto"
> > > from ext4 and only support blk-crypto (relying on blk-crypto-fallback
> > > for the software fallback). The blk-crypto code path doesn't have this
> > > problem since it more closely ties the encryption to the actual write.
> > > It also works better with folios.
> >
> > Hey waitaminute, are you planning to withdraw fscrypt from ext4?
> >
> > (I might just not know enough about what blk-crypto is)
> >
>
> ext4 (and also f2fs) has two different implementations of file contents
> en/decryption: one where the filesystem directly calls the crypto
> functions to en/decrypt file contents, and one where the filesystem
> instead adds a bio_crypt_ctx to the bios it submits, causing the block
> layer to handle the en/decryption (via either inline crypto hardware or
> blk-crypto-fallback). See the "Inline encryption support" section of
> Documentation/filesystems/fscrypt.rst.
>
> These correspond to fscrypt_inode_uses_fs_layer_crypto() and
> fscrypt_inode_uses_inline_crypto() in the code. The "fs-layer"
> implementation is used by default, while the inline crypto
> implementation is used when the 'inlinecrypt' mount option is used.
>
> It's just an implementation detail and doesn't affect the end result.
>
> Note that "fscrypt" is the name for the overall ext4/f2fs/etc encryption
> feature, which both these implementations are part of.
>
> I'm talking about possibly removing the first of these file contents
> encryption implementations, which again are just implementation details,
> so that we standardize on just the blk-crypto one.
Ooh, I bet that will integrate better with iomap, whenever someone gets
around to attempting the first port. :)
--D
> Again, this KMSAN warning is specific to the first implementation. I.e.
> it doesn't appear when the inlinecrypt mount option is used.
>
> - Eric
>
Powered by blists - more mailing lists