On Mon, Dec 04, 2023 at 05:58:00PM -0800, Eric Biggers wrote:
> On Fri, Dec 01, 2023 at 05:10:58PM -0500, Josef Bacik wrote:
> > btrfs has a variety of asynchronous things we do with inodes that can
> > potentially last until ->put_super, when we shut everything down and
> > clean up all of our async work. Due to this we need to move
> > fscrypt_destroy_keyring() to after ->put_super, otherwise we get
> > warnings about still having active references on the master key.
> >
> > Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> > ---
> > fs/super.c | 12 ++++++------
> > 1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/super.c b/fs/super.c
> > index 076392396e72..faf7d248145d 100644
> > --- a/fs/super.c
> > +++ b/fs/super.c
> > @@ -681,12 +681,6 @@ void generic_shutdown_super(struct super_block *sb)
> > fsnotify_sb_delete(sb);
> > security_sb_delete(sb);
> >
> > - /*
> > - * Now that all potentially-encrypted inodes have been evicted,
> > - * the fscrypt keyring can be destroyed.
> > - */
> > - fscrypt_destroy_keyring(sb);
> > -
> > if (sb->s_dio_done_wq) {
> > destroy_workqueue(sb->s_dio_done_wq);
> > sb->s_dio_done_wq = NULL;
> > @@ -695,6 +689,12 @@ void generic_shutdown_super(struct super_block *sb)
> > if (sop->put_super)
> > sop->put_super(sb);
> >
> > + /*
> > + * Now that all potentially-encrypted inodes have been evicted,
> > + * the fscrypt keyring can be destroyed.
> > + */
> > + fscrypt_destroy_keyring(sb);
> > +
>
> This patch will cause a NULL dereference on f2fs, since f2fs_put_super() frees
> ->s_fs_info, and then fscrypt_destroy_keyring() can call f2fs_get_devices() (via
> fscrypt_operations::get_devices) which dereferences it. (ext4 also frees
> ->s_fs_info in its ->put_super, but ext4 doesn't implement ->get_devices.)
>
> I think we need to move the fscrypt keyring destruction into ->put_super for
> each filesystem.
I can do this, I'll send a separate series for this since this should be
straightforward and we can get that part done. Thanks,
Josef
