Re: [PATCH v4 01/46] fs: move fscrypt keyring destruction to after ->put_super

[Josef Bacik <josef@toxicpanda.com>]

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@toxicpanda.com>
> > ---
> >  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