As a rule of thumb everything allocated to the fs_context and moved into
the super_block should be freed by ->kill_sb so that the teardown
handling doesn't need to be duplicated between the fill_super error
path and put_super. Implement an ntfs3-specific kill_sb method to do
that.
Signed-off-by: Christoph Hellwig <[email protected]>
Reviewed-by: Christian Brauner <[email protected]>
---
fs/ntfs3/super.c | 25 ++++++++++++-------------
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c
index 727138933a9324..5fffddea554f18 100644
--- a/fs/ntfs3/super.c
+++ b/fs/ntfs3/super.c
@@ -625,10 +625,6 @@ static void ntfs_put_super(struct super_block *sb)
/* Mark rw ntfs as clear, if possible. */
ntfs_set_state(sbi, NTFS_DIRTY_CLEAR);
-
- put_mount_options(sbi->options);
- ntfs3_free_sbi(sbi);
- sb->s_fs_info = NULL;
}
static int ntfs_statfs(struct dentry *dentry, struct kstatfs *buf)
@@ -1562,15 +1558,7 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc)
put_inode_out:
iput(inode);
out:
- /*
- * Free resources here.
- * ntfs_fs_free will be called with fc->s_fs_info = NULL
- */
- put_mount_options(sbi->options);
- ntfs3_free_sbi(sbi);
- sb->s_fs_info = NULL;
kfree(boot2);
-
return err;
}
@@ -1726,13 +1714,24 @@ static int ntfs_init_fs_context(struct fs_context *fc)
return -ENOMEM;
}
+static void ntfs3_kill_sb(struct super_block *sb)
+{
+ struct ntfs_sb_info *sbi = sb->s_fs_info;
+
+ kill_block_super(sb);
+
+ if (sbi->options)
+ put_mount_options(sbi->options);
+ ntfs3_free_sbi(sbi);
+}
+
// clang-format off
static struct file_system_type ntfs_fs_type = {
.owner = THIS_MODULE,
.name = "ntfs3",
.init_fs_context = ntfs_init_fs_context,
.parameters = ntfs_fs_parameters,
- .kill_sb = kill_block_super,
+ .kill_sb = ntfs3_kill_sb,
.fs_flags = FS_REQUIRES_DEV | FS_ALLOW_IDMAP,
};
// clang-format on
--
2.39.2
On Thu, Sep 07, 2023 at 08:23:09AM -0700, Guenter Roeck wrote:
> On 9/7/23 06:54, Christian Brauner wrote:
> > On Thu, Sep 07, 2023 at 06:05:40AM -0700, Guenter Roeck wrote:
> > > On Wed, Aug 09, 2023 at 03:05:45PM -0700, Christoph Hellwig wrote:
> > > > As a rule of thumb everything allocated to the fs_context and moved into
> > > > the super_block should be freed by ->kill_sb so that the teardown
> > > > handling doesn't need to be duplicated between the fill_super error
> > > > path and put_super. Implement an ntfs3-specific kill_sb method to do
> > > > that.
> > > >
> > > > Signed-off-by: Christoph Hellwig <[email protected]>
> > > > Reviewed-by: Christian Brauner <[email protected]>
> > >
> > > This patch results in:
> >
> > The appended patch should fix this. Are you able to test it?
> > I will as well.
>
> Yes, this patch restores the previous behavior (no more backtrace or crash).
Great, I'll get this fixed in upstream.
>
> Tested-by: Guenter Roeck <[email protected]>
>
> I say "restore previous behavior" because my simple "recursive copy; partially
> remove copied files" test still fails. That problem apparently existed since
> ntfs3 has been introduced (I see it as far back as v5.15).
I don't think anyone finds that surprising.
On Thu, Sep 07, 2023 at 06:05:40AM -0700, Guenter Roeck wrote:
> On Wed, Aug 09, 2023 at 03:05:45PM -0700, Christoph Hellwig wrote:
> > As a rule of thumb everything allocated to the fs_context and moved into
> > the super_block should be freed by ->kill_sb so that the teardown
> > handling doesn't need to be duplicated between the fill_super error
> > path and put_super. Implement an ntfs3-specific kill_sb method to do
> > that.
> >
> > Signed-off-by: Christoph Hellwig <[email protected]>
> > Reviewed-by: Christian Brauner <[email protected]>
>
> This patch results in:
The appended patch should fix this. Are you able to test it?
I will as well.