[PATCH] ntfs3: drop inode references in ntfs_put_super()

[PATCH] ntfs3: drop inode references in ntfs_put_super()

[Christian Brauner]

Recently we moved most cleanup from ntfs_put_super() into
ntfs3_kill_sb() as part of a bigger cleanup. This accidently also moved
dropping inode references stashed in ntfs3's sb->s_fs_info from
@sb->put_super() to @sb->kill_sb(). But generic_shutdown_super()
verifies that there are no busy inodes past sb->put_super(). Fix this
and disentangle dropping inode references from freeing @sb->s_fs_info.

Fixes: a4f64a300a29 ("ntfs3: free the sbi in ->kill_sb") # mainline only
Reported-by: Guenter Roeck <linux@roeck-us.net>
Tested-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Christian Brauner <brauner@kernel.org>
---
Hey Linus,

Would you mind applying this patch directly? It's a simple fix for an
ntfs3 bug that was introduced by some refactoring we did in mainline
only. Guenter reported it and both he and I verified that this patch
fixes the issue. I can also resend it as a PR tomorrow if you prefer.

Thanks!
Christian
---
 fs/ntfs3/super.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index 5fffddea554f..cfec5e0c7f66 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -571,12 +571,8 @@ static void init_once(void *foo)
 /*
  * Noinline to reduce binary size.
  */
-static noinline void ntfs3_free_sbi(struct ntfs_sb_info *sbi)
+static noinline void ntfs3_put_sbi(struct ntfs_sb_info *sbi)
 {
-	kfree(sbi->new_rec);
-	kvfree(ntfs_put_shared(sbi->upcase));
-	kfree(sbi->def_table);
-
 	wnd_close(&sbi->mft.bitmap);
 	wnd_close(&sbi->used.bitmap);
 
@@ -601,6 +597,13 @@ static noinline void ntfs3_free_sbi(struct ntfs_sb_info *sbi)
 	indx_clear(&sbi->security.index_sdh);
 	indx_clear(&sbi->reparse.index_r);
 	indx_clear(&sbi->objid.index_o);
+}
+
+static void ntfs3_free_sbi(struct ntfs_sb_info *sbi)
+{
+	kfree(sbi->new_rec);
+	kvfree(ntfs_put_shared(sbi->upcase));
+	kfree(sbi->def_table);
 	kfree(sbi->compress.lznt);
 #ifdef CONFIG_NTFS3_LZX_XPRESS
 	xpress_free_decompressor(sbi->compress.xpress);
@@ -625,6 +628,7 @@ static void ntfs_put_super(struct super_block *sb)
 
 	/* Mark rw ntfs as clear, if possible. */
 	ntfs_set_state(sbi, NTFS_DIRTY_CLEAR);
+	ntfs3_put_sbi(sbi);
 }
 
 static int ntfs_statfs(struct dentry *dentry, struct kstatfs *buf)
@@ -1644,8 +1648,10 @@ static void ntfs_fs_free(struct fs_context *fc)
 	struct ntfs_mount_options *opts = fc->fs_private;
 	struct ntfs_sb_info *sbi = fc->s_fs_info;
 
-	if (sbi)
+	if (sbi) {
+		ntfs3_put_sbi(sbi);
 		ntfs3_free_sbi(sbi);
+	}
 
 	if (opts)
 		put_mount_options(opts);

---
base-commit: 7ba2090ca64ea1aa435744884124387db1fac70f
change-id: 20230907-vfs-ntfs3-kill_sb-52bb6f5230d4

Re: [PATCH] ntfs3: drop inode references in ntfs_put_super()

[Linus Torvalds]

On Thu, 7 Sept 2023 at 09:04, Christian Brauner <brauner@kernel.org> wrote:
>
> Hey Linus,
>
> Would you mind applying this patch directly? It's a simple fix for an
> ntfs3 bug that was introduced by some refactoring we did in mainline
> only.

Done. Thanks,

                 Linus

Re: [PATCH] ntfs3: drop inode references in ntfs_put_super()

[Christoph Hellwig]

On Thu, Sep 07, 2023 at 06:03:40PM +0200, Christian Brauner wrote:
> Recently we moved most cleanup from ntfs_put_super() into
> ntfs3_kill_sb() as part of a bigger cleanup. This accidently also moved
> dropping inode references stashed in ntfs3's sb->s_fs_info from
> @sb->put_super() to @sb->kill_sb(). But generic_shutdown_super()
> verifies that there are no busy inodes past sb->put_super(). Fix this
> and disentangle dropping inode references from freeing @sb->s_fs_info.

Sorry for the delay, I've been travelling.  Wouldn't it make more
sense to just free it in ->kill_sb before calling kill_block_super?

Either way the fix looks good, and as Linus has already applied it
it's probably not worth arguing..

Re: [PATCH] ntfs3: drop inode references in ntfs_put_super()

[Christian Brauner]

On Fri, Sep 08, 2023 at 10:12:13AM +0200, Christoph Hellwig wrote:
> On Thu, Sep 07, 2023 at 06:03:40PM +0200, Christian Brauner wrote:
> > Recently we moved most cleanup from ntfs_put_super() into
> > ntfs3_kill_sb() as part of a bigger cleanup. This accidently also moved
> > dropping inode references stashed in ntfs3's sb->s_fs_info from
> > @sb->put_super() to @sb->kill_sb(). But generic_shutdown_super()
> > verifies that there are no busy inodes past sb->put_super(). Fix this
> > and disentangle dropping inode references from freeing @sb->s_fs_info.
> 
> Sorry for the delay, I've been travelling.  Wouldn't it make more
> sense to just free it in ->kill_sb before calling kill_block_super?

ntfs3 has ntfs_evict_inodes() which might depend on the info in
sbi->s_fs_info to be valid. So calling it before kill_block_super()
risks putting resources that ntfs_evict_inodes() might depend on. Doing
it in ntfs_put_super() will prevent this from becoming an issue without
having to wade through all callchains. And since we can't get rid of
ntfs_put_super() anyway why bother risking that.