On Tue, Oct 04, 2022 at 06:13:26PM -0600, Yu Zhao wrote:
> fscrypt_initialize() shouldn't allocate memory without GFP_NOFS.
>
> The problem seems to go back to 2015
> commit 57e5055b0a5e ("f2fs crypto: add f2fs encryption facilities")
> but I have never heard of any complaints, hence not CC'ing stable.
>
> ======================================================
> WARNING: possible circular locking dependency detected
> 6.0.0-lockdep #1 Not tainted
> ------------------------------------------------------
> kswapd0/77 is trying to acquire lock:
> 71ffff808b254a18 (jbd2_handle){++++}-{0:0}, at: start_this_handle+0x76c/0x8dc
>
> but task is already holding lock:
> ffffffea26533310 (fs_reclaim){+.+.}-{0:0}, at: 0x1
>
> which lock already depends on the new lock.
>
> <snipped>
>
> other info that might help us debug this:
>
> Chain exists of:
> jbd2_handle --> fscrypt_init_mutex --> fs_reclaim
>
> Possible unsafe locking scenario:
>
> CPU0 CPU1
> ---- ----
> lock(fs_reclaim);
> lock(fscrypt_init_mutex);
> lock(fs_reclaim);
> lock(jbd2_handle);
>
> *** DEADLOCK ***
>
> 3 locks held by kswapd0/77:
> #0: ffffffea26533310 (fs_reclaim){+.+.}-{0:0}, at: 0x1
> #1: ffffffea26529220 (shrinker_rwsem){++++}-{3:3}, at: shrink_slab+0x54/0x464
> #2: 6dffff808abe90e8 (&type->s_umount_key#47){++++}-{3:3}, at: trylock_super+0x2c/0x8c
>
> <snipped>
>
> Signed-off-by: Yu Zhao <yuzhao@xxxxxxxxxx>
> ---
> fs/crypto/crypto.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
> index e78be66bbf01..e10fc30142a6 100644
> --- a/fs/crypto/crypto.c
> +++ b/fs/crypto/crypto.c
> @@ -316,6 +316,7 @@ EXPORT_SYMBOL(fscrypt_decrypt_block_inplace);
> int fscrypt_initialize(unsigned int cop_flags)
> {
> int err = 0;
> + unsigned int flags;
>
> /* No need to allocate a bounce page pool if this FS won't use it. */
> if (cop_flags & FS_CFLG_OWN_PAGES)
> @@ -326,8 +327,10 @@ int fscrypt_initialize(unsigned int cop_flags)
> goto out_unlock;
>
> err = -ENOMEM;
> + flags = memalloc_nofs_save();
> fscrypt_bounce_page_pool =
> mempool_create_page_pool(num_prealloc_crypto_pages, 0);
> + memalloc_nofs_restore(flags);
> if (!fscrypt_bounce_page_pool)
> goto out_unlock;
Thanks, but this isn't the correct fix. The real problem is that ext4 is
calling fscrypt_get_encryption_info() from within a jbd2 transaction, which is
fundamentally unsafe. It's a known regression from commit a80f7fcf1867
("ext4: fixup ext4_fc_track_* functions' signature"), which extended the scope
of the transaction in ext4_unlink() too far.
Sorry for not getting around to fixing this earlier. Are you interested in
sending a patch for it? If you do, please make sure to include
"Reported-by: syzbot+1a748d0007eeac3ab079@xxxxxxxxxxxxxxxxxxxxxxxxx",
as there's a syzbot report open already
(https://lore.kernel.org/all/00000000000070395e05dd1fb4d7@xxxxxxxxxx/T/#u).
- Eric
|