On Tue, Dec 05, 2023 at 05:48:48PM -0500, Josef Bacik wrote:
> 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,
>
I actually started a patch for this earlier today, just haven't had a chance to
send it out yet. I'll do so in a minute. Thanks,
- Eric
