* [PATCH] fscrypt: fix lockdep warning
@ 2022-10-05 0:13 Yu Zhao
2022-10-05 1:39 ` Eric Biggers
0 siblings, 1 reply; 3+ messages in thread
From: Yu Zhao @ 2022-10-05 0:13 UTC (permalink / raw)
To: Theodore Y . Ts'o, Jaegeuk Kim, Eric Biggers
Cc: linux-fscrypt, linux-kernel, Yu Zhao
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@google.com>
---
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;
--
2.38.0.rc1.362.ged0d419d3c-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] fscrypt: fix lockdep warning
2022-10-05 0:13 [PATCH] fscrypt: fix lockdep warning Yu Zhao
@ 2022-10-05 1:39 ` Eric Biggers
2022-10-05 1:57 ` Yu Zhao
0 siblings, 1 reply; 3+ messages in thread
From: Eric Biggers @ 2022-10-05 1:39 UTC (permalink / raw)
To: Yu Zhao; +Cc: Theodore Y . Ts'o, Jaegeuk Kim, linux-fscrypt, linux-kernel
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@google.com>
> ---
> 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@syzkaller.appspotmail.com",
as there's a syzbot report open already
(https://lore.kernel.org/all/00000000000070395e05dd1fb4d7@google.com/T/#u).
- Eric
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] fscrypt: fix lockdep warning
2022-10-05 1:39 ` Eric Biggers
@ 2022-10-05 1:57 ` Yu Zhao
0 siblings, 0 replies; 3+ messages in thread
From: Yu Zhao @ 2022-10-05 1:57 UTC (permalink / raw)
To: Eric Biggers
Cc: Theodore Y . Ts'o, Jaegeuk Kim, linux-fscrypt, linux-kernel
On Tue, Oct 4, 2022 at 7:39 PM Eric Biggers <ebiggers@kernel.org> wrote:
>
> 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@google.com>
> > ---
> > 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.
Thanks. So the jbd2_handle -> fscrypt_init_mutex dependency is bad.
> 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?
I have no idea how to move fscrypt_get_encryption_info() out of jbd2
transactions. I'll just leave it to you.
> If you do, please make sure to include
> "Reported-by: syzbot+1a748d0007eeac3ab079@syzkaller.appspotmail.com",
> as there's a syzbot report open already
> (https://lore.kernel.org/all/00000000000070395e05dd1fb4d7@google.com/T/#u).
>
> - Eric
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-10-05 1:58 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-05 0:13 [PATCH] fscrypt: fix lockdep warning Yu Zhao
2022-10-05 1:39 ` Eric Biggers
2022-10-05 1:57 ` Yu Zhao
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.