On Thu, Jun 13, 2024 at 03:48:25PM +0200, Norbert Kamiński wrote: > syzbot is reporting an uninitialized value in aes_encrypt(). The block > cipher expects the bytes to encrypt or decrypt to be a multiple of the > cipher’s block size. However, when ext4_write_begin() is called and new > folios are allocated, they might not be aligned to the required block > size. While the length of file content blocks does need to be a multiple of FSCRYPT_CONTENTS_ALIGNMENT bytes, this has nothing to do with the syzbot report that this patch is trying to fix, and this is always the case in ext4 anyway. > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index 4bae9ccf5fe0..965f790a9d36 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -1156,6 +1156,9 @@ static int ext4_write_begin(struct file *file, struct address_space *mapping, > * the folio (if needed) without using GFP_NOFS. > */ > retry_grab: > + if (IS_ENABLED(CONFIG_FS_ENCRYPTION)) > + mapping_set_gfp_mask(mapping, > + mapping_gfp_mask(mapping) | __GFP_ZERO); > folio = __filemap_get_folio(mapping, index, FGP_WRITEBEGIN, > mapping_gfp_mask(mapping)); > if (IS_ERR(folio)) > @@ -2882,6 +2885,9 @@ static int ext4_da_write_begin(struct file *file, struct address_space *mapping, > } > > retry: > + if (IS_ENABLED(CONFIG_FS_ENCRYPTION)) > + mapping_set_gfp_mask(mapping, > + mapping_gfp_mask(mapping) | __GFP_ZERO); No, it's not acceptable to force all pagecache pages to be zeroized in ext4 without opting into init_on_alloc. This is also the wrong place to set the mapping's gfp_mask, as the mapping has already been activated. What actually needs to be done is root-cause this bug and fix the underlying cause. It looks like somehow data got marked as valid in the pagecache without being initialized, which is never supposed to happen. - Eric
