[PATCH v3 0/2] Fix memory leaks in hfs and hfsplus

[PATCH v3 0/2] Fix memory leaks in hfs and hfsplus

[Mehdi Ben Hadj Khelifa]

The Following series fixes a bug reported by syzbot which is specific to
HFS but this issue is also persistent in other filesystems.For now this
series fixes the bug for HFS and HFS+ Filesystems. Other filesystems
need to be checked for the same issue fixed here.

ChangeLog:

Changes from v2:

-Include hfsplus fix

-Align changes to christian's recommendation.

Link:https://lore.kernel.org/all/20251119073845.18578-1-mehdi.benhadjkhelifa@gmail.com/

Changes from v1:

-Changed the patch direction to focus on hfs changes specifically as 
suggested by al viro

Link:https://lore.kernel.org/all/20251114165255.101361-1-mehdi.benhadjkhelifa@gmail.com/

Mehdi Ben Hadj Khelifa (2):
  hfs: ensure sb->s_fs_info is always cleaned up
  hfsplus: ensure sb->s_fs_info is always cleaned up

 fs/hfs/mdb.c       | 35 ++++++++++++++---------------------
 fs/hfs/super.c     | 10 +++++++++-
 fs/hfsplus/super.c | 13 +++++++++----
 3 files changed, 32 insertions(+), 26 deletions(-)

-- 
2.52.0

[PATCH v3 1/2] hfs: ensure sb->s_fs_info is always cleaned up

[Mehdi Ben Hadj Khelifa]

When hfs was converted to the new mount api a bug was introduced by
changing the allocation pattern of sb->s_fs_info. If setup_bdev_super()
fails after a new superblock has been allocated by sget_fc(), but before
hfs_fill_super() takes ownership of the filesystem-specific s_fs_info
data it was leaked.

Fix this by freeing sb->s_fs_info in hfs_kill_super().

Cc: stable@vger.kernel.org
Fixes: ffcd06b6d13b ("hfs: convert hfs to use the new mount api")
Reported-by: syzbot+ad45f827c88778ff7df6@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=ad45f827c88778ff7df6
Tested-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
Signed-off-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
---
 fs/hfs/mdb.c   | 35 ++++++++++++++---------------------
 fs/hfs/super.c | 10 +++++++++-
 2 files changed, 23 insertions(+), 22 deletions(-)

diff --git a/fs/hfs/mdb.c b/fs/hfs/mdb.c
index 53f3fae60217..f28cd24dee84 100644
--- a/fs/hfs/mdb.c
+++ b/fs/hfs/mdb.c
@@ -92,7 +92,7 @@ int hfs_mdb_get(struct super_block *sb)
 		/* See if this is an HFS filesystem */
 		bh = sb_bread512(sb, part_start + HFS_MDB_BLK, mdb);
 		if (!bh)
-			goto out;
+			return -EIO;
 
 		if (mdb->drSigWord == cpu_to_be16(HFS_SUPER_MAGIC))
 			break;
@@ -102,13 +102,14 @@ int hfs_mdb_get(struct super_block *sb)
 		 * (should do this only for cdrom/loop though)
 		 */
 		if (hfs_part_find(sb, &part_start, &part_size))
-			goto out;
+			return -EIO;
 	}
 
 	HFS_SB(sb)->alloc_blksz = size = be32_to_cpu(mdb->drAlBlkSiz);
 	if (!size || (size & (HFS_SECTOR_SIZE - 1))) {
 		pr_err("bad allocation block size %d\n", size);
-		goto out_bh;
+		brelse(bh);
+		return -EIO;
 	}
 
 	size = min(HFS_SB(sb)->alloc_blksz, (u32)PAGE_SIZE);
@@ -125,14 +126,16 @@ int hfs_mdb_get(struct super_block *sb)
 	brelse(bh);
 	if (!sb_set_blocksize(sb, size)) {
 		pr_err("unable to set blocksize to %u\n", size);
-		goto out;
+		return -EIO;
 	}
 
 	bh = sb_bread512(sb, part_start + HFS_MDB_BLK, mdb);
 	if (!bh)
-		goto out;
-	if (mdb->drSigWord != cpu_to_be16(HFS_SUPER_MAGIC))
-		goto out_bh;
+		return -EIO;
+	if (mdb->drSigWord != cpu_to_be16(HFS_SUPER_MAGIC)) {
+		brelse(bh);
+		return -EIO;
+	}
 
 	HFS_SB(sb)->mdb_bh = bh;
 	HFS_SB(sb)->mdb = mdb;
@@ -174,7 +177,7 @@ int hfs_mdb_get(struct super_block *sb)
 
 	HFS_SB(sb)->bitmap = kzalloc(8192, GFP_KERNEL);
 	if (!HFS_SB(sb)->bitmap)
-		goto out;
+		return -EIO;
 
 	/* read in the bitmap */
 	block = be16_to_cpu(mdb->drVBMSt) + part_start;
@@ -185,7 +188,7 @@ int hfs_mdb_get(struct super_block *sb)
 		bh = sb_bread(sb, off >> sb->s_blocksize_bits);
 		if (!bh) {
 			pr_err("unable to read volume bitmap\n");
-			goto out;
+			return -EIO;
 		}
 		off2 = off & (sb->s_blocksize - 1);
 		len = min((int)sb->s_blocksize - off2, size);
@@ -199,12 +202,12 @@ int hfs_mdb_get(struct super_block *sb)
 	HFS_SB(sb)->ext_tree = hfs_btree_open(sb, HFS_EXT_CNID, hfs_ext_keycmp);
 	if (!HFS_SB(sb)->ext_tree) {
 		pr_err("unable to open extent tree\n");
-		goto out;
+		return -EIO;
 	}
 	HFS_SB(sb)->cat_tree = hfs_btree_open(sb, HFS_CAT_CNID, hfs_cat_keycmp);
 	if (!HFS_SB(sb)->cat_tree) {
 		pr_err("unable to open catalog tree\n");
-		goto out;
+		return -EIO;
 	}
 
 	attrib = mdb->drAtrb;
@@ -229,12 +232,6 @@ int hfs_mdb_get(struct super_block *sb)
 	}
 
 	return 0;
-
-out_bh:
-	brelse(bh);
-out:
-	hfs_mdb_put(sb);
-	return -EIO;
 }
 
 /*
@@ -359,8 +356,6 @@ void hfs_mdb_close(struct super_block *sb)
  * Release the resources associated with the in-core MDB.  */
 void hfs_mdb_put(struct super_block *sb)
 {
-	if (!HFS_SB(sb))
-		return;
 	/* free the B-trees */
 	hfs_btree_close(HFS_SB(sb)->ext_tree);
 	hfs_btree_close(HFS_SB(sb)->cat_tree);
@@ -373,6 +368,4 @@ void hfs_mdb_put(struct super_block *sb)
 	unload_nls(HFS_SB(sb)->nls_disk);
 
 	kfree(HFS_SB(sb)->bitmap);
-	kfree(HFS_SB(sb));
-	sb->s_fs_info = NULL;
 }
diff --git a/fs/hfs/super.c b/fs/hfs/super.c
index 47f50fa555a4..df289cbdd4e8 100644
--- a/fs/hfs/super.c
+++ b/fs/hfs/super.c
@@ -431,10 +431,18 @@ static int hfs_init_fs_context(struct fs_context *fc)
 	return 0;
 }
 
+static void hfs_kill_super(struct super_block *sb)
+{
+	struct hfs_sb_info *hsb = HFS_SB(sb);
+
+	kill_block_super(sb);
+	kfree(hsb);
+}
+
 static struct file_system_type hfs_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "hfs",
-	.kill_sb	= kill_block_super,
+	.kill_sb	= hfs_kill_super,
 	.fs_flags	= FS_REQUIRES_DEV,
 	.init_fs_context = hfs_init_fs_context,
 };
-- 
2.52.0

Re: [PATCH v3 1/2] hfs: ensure sb->s_fs_info is always cleaned up

[Viacheslav Dubeyko]

On Mon, 2025-12-01 at 23:23 +0100, Mehdi Ben Hadj Khelifa wrote:
> When hfs was converted to the new mount api a bug was introduced by
> changing the allocation pattern of sb->s_fs_info. If setup_bdev_super()
> fails after a new superblock has been allocated by sget_fc(), but before
> hfs_fill_super() takes ownership of the filesystem-specific s_fs_info
> data it was leaked.
> 
> Fix this by freeing sb->s_fs_info in hfs_kill_super().
> 
> Cc: stable@vger.kernel.org
> Fixes: ffcd06b6d13b ("hfs: convert hfs to use the new mount api")
> Reported-by: syzbot+ad45f827c88778ff7df6@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=ad45f827c88778ff7df6  
> Tested-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
> ---
>  fs/hfs/mdb.c   | 35 ++++++++++++++---------------------
>  fs/hfs/super.c | 10 +++++++++-
>  2 files changed, 23 insertions(+), 22 deletions(-)
> 
> diff --git a/fs/hfs/mdb.c b/fs/hfs/mdb.c
> index 53f3fae60217..f28cd24dee84 100644
> --- a/fs/hfs/mdb.c
> +++ b/fs/hfs/mdb.c
> @@ -92,7 +92,7 @@ int hfs_mdb_get(struct super_block *sb)
>  		/* See if this is an HFS filesystem */
>  		bh = sb_bread512(sb, part_start + HFS_MDB_BLK, mdb);
>  		if (!bh)
> -			goto out;
> +			return -EIO;

Frankly speaking, I don't see the point to rework the hfs_mdb_get() method so
intensively. We had pretty good pattern before:

int hfs_mdb_get(struct super_block *sb) {
        if (something_happens)
             goto out;

        if (something_happens_and_we_need_free_buffer)
            goto out_bh;

 	return 0;

out_bh:
	brelse(bh);
out:
	return -EIO;
 }

The point here that we have error management logic in one place. Now you have
spread this logic through the whole function. It makes function more difficult
to manage and we can introduce new bugs. Could you please localize your change
without reworking this pattern of error situation management? Also, it will make
the patch more compact. Could you please rework the patch?

Thanks,
Slava. 

>  
>  		if (mdb->drSigWord == cpu_to_be16(HFS_SUPER_MAGIC))
>  			break;
> @@ -102,13 +102,14 @@ int hfs_mdb_get(struct super_block *sb)
>  		 * (should do this only for cdrom/loop though)
>  		 */
>  		if (hfs_part_find(sb, &part_start, &part_size))
> -			goto out;
> +			return -EIO;
>  	}
>  
>  	HFS_SB(sb)->alloc_blksz = size = be32_to_cpu(mdb->drAlBlkSiz);
>  	if (!size || (size & (HFS_SECTOR_SIZE - 1))) {
>  		pr_err("bad allocation block size %d\n", size);
> -		goto out_bh;
> +		brelse(bh);
> +		return -EIO;
>  	}
>  
>  	size = min(HFS_SB(sb)->alloc_blksz, (u32)PAGE_SIZE);
> @@ -125,14 +126,16 @@ int hfs_mdb_get(struct super_block *sb)
>  	brelse(bh);
>  	if (!sb_set_blocksize(sb, size)) {
>  		pr_err("unable to set blocksize to %u\n", size);
> -		goto out;
> +		return -EIO;
>  	}
>  
>  	bh = sb_bread512(sb, part_start + HFS_MDB_BLK, mdb);
>  	if (!bh)
> -		goto out;
> -	if (mdb->drSigWord != cpu_to_be16(HFS_SUPER_MAGIC))
> -		goto out_bh;
> +		return -EIO;
> +	if (mdb->drSigWord != cpu_to_be16(HFS_SUPER_MAGIC)) {
> +		brelse(bh);
> +		return -EIO;
> +	}
>  
>  	HFS_SB(sb)->mdb_bh = bh;
>  	HFS_SB(sb)->mdb = mdb;
> @@ -174,7 +177,7 @@ int hfs_mdb_get(struct super_block *sb)
>  
>  	HFS_SB(sb)->bitmap = kzalloc(8192, GFP_KERNEL);
>  	if (!HFS_SB(sb)->bitmap)
> -		goto out;
> +		return -EIO;
>  
>  	/* read in the bitmap */
>  	block = be16_to_cpu(mdb->drVBMSt) + part_start;
> @@ -185,7 +188,7 @@ int hfs_mdb_get(struct super_block *sb)
>  		bh = sb_bread(sb, off >> sb->s_blocksize_bits);
>  		if (!bh) {
>  			pr_err("unable to read volume bitmap\n");
> -			goto out;
> +			return -EIO;
>  		}
>  		off2 = off & (sb->s_blocksize - 1);
>  		len = min((int)sb->s_blocksize - off2, size);
> @@ -199,12 +202,12 @@ int hfs_mdb_get(struct super_block *sb)
>  	HFS_SB(sb)->ext_tree = hfs_btree_open(sb, HFS_EXT_CNID, hfs_ext_keycmp);
>  	if (!HFS_SB(sb)->ext_tree) {
>  		pr_err("unable to open extent tree\n");
> -		goto out;
> +		return -EIO;
>  	}
>  	HFS_SB(sb)->cat_tree = hfs_btree_open(sb, HFS_CAT_CNID, hfs_cat_keycmp);
>  	if (!HFS_SB(sb)->cat_tree) {
>  		pr_err("unable to open catalog tree\n");
> -		goto out;
> +		return -EIO;
>  	}
>  
>  	attrib = mdb->drAtrb;
> @@ -229,12 +232,6 @@ int hfs_mdb_get(struct super_block *sb)
>  	}
>  
>  	return 0;
> -
> -out_bh:
> -	brelse(bh);
> -out:
> -	hfs_mdb_put(sb);
> -	return -EIO;
>  }
>  
>  /*
> @@ -359,8 +356,6 @@ void hfs_mdb_close(struct super_block *sb)
>   * Release the resources associated with the in-core MDB.  */
>  void hfs_mdb_put(struct super_block *sb)
>  {
> -	if (!HFS_SB(sb))
> -		return;
>  	/* free the B-trees */
>  	hfs_btree_close(HFS_SB(sb)->ext_tree);
>  	hfs_btree_close(HFS_SB(sb)->cat_tree);
> @@ -373,6 +368,4 @@ void hfs_mdb_put(struct super_block *sb)
>  	unload_nls(HFS_SB(sb)->nls_disk);
>  
>  	kfree(HFS_SB(sb)->bitmap);
> -	kfree(HFS_SB(sb));
> -	sb->s_fs_info = NULL;
>  }
> diff --git a/fs/hfs/super.c b/fs/hfs/super.c
> index 47f50fa555a4..df289cbdd4e8 100644
> --- a/fs/hfs/super.c
> +++ b/fs/hfs/super.c
> @@ -431,10 +431,18 @@ static int hfs_init_fs_context(struct fs_context *fc)
>  	return 0;
>  }
>  
> +static void hfs_kill_super(struct super_block *sb)
> +{
> +	struct hfs_sb_info *hsb = HFS_SB(sb);
> +
> +	kill_block_super(sb);
> +	kfree(hsb);
> +}
> +
>  static struct file_system_type hfs_fs_type = {
>  	.owner		= THIS_MODULE,
>  	.name		= "hfs",
> -	.kill_sb	= kill_block_super,
> +	.kill_sb	= hfs_kill_super,
>  	.fs_flags	= FS_REQUIRES_DEV,
>  	.init_fs_context = hfs_init_fs_context,
>  };

Re: [PATCH v3 1/2] hfs: ensure sb->s_fs_info is always cleaned up

[Mehdi Ben Hadj Khelifa]

On 12/2/25 12:04 AM, Viacheslav Dubeyko wrote:
> On Mon, 2025-12-01 at 23:23 +0100, Mehdi Ben Hadj Khelifa wrote:
>> When hfs was converted to the new mount api a bug was introduced by
>> changing the allocation pattern of sb->s_fs_info. If setup_bdev_super()
>> fails after a new superblock has been allocated by sget_fc(), but before
>> hfs_fill_super() takes ownership of the filesystem-specific s_fs_info
>> data it was leaked.
>>
>> Fix this by freeing sb->s_fs_info in hfs_kill_super().
>>
>> Cc: stable@vger.kernel.org
>> Fixes: ffcd06b6d13b ("hfs: convert hfs to use the new mount api")
>> Reported-by: syzbot+ad45f827c88778ff7df6@syzkaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=ad45f827c88778ff7df6
>> Tested-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
>> Signed-off-by: Christian Brauner <brauner@kernel.org>
>> Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
>> ---
>>   fs/hfs/mdb.c   | 35 ++++++++++++++---------------------
>>   fs/hfs/super.c | 10 +++++++++-
>>   2 files changed, 23 insertions(+), 22 deletions(-)
>>
>> diff --git a/fs/hfs/mdb.c b/fs/hfs/mdb.c
>> index 53f3fae60217..f28cd24dee84 100644
>> --- a/fs/hfs/mdb.c
>> +++ b/fs/hfs/mdb.c
>> @@ -92,7 +92,7 @@ int hfs_mdb_get(struct super_block *sb)
>>   		/* See if this is an HFS filesystem */
>>   		bh = sb_bread512(sb, part_start + HFS_MDB_BLK, mdb);
>>   		if (!bh)
>> -			goto out;
>> +			return -EIO;
> 
> Frankly speaking, I don't see the point to rework the hfs_mdb_get() method so
> intensively. We had pretty good pattern before:
> 
> int hfs_mdb_get(struct super_block *sb) {
>          if (something_happens)
>               goto out;
> 
>          if (something_happens_and_we_need_free_buffer)
>              goto out_bh;
> 
>   	return 0;
> 
> out_bh:
> 	brelse(bh);
> out:
> 	return -EIO;
>   }
> 
> The point here that we have error management logic in one place. Now you have
> spread this logic through the whole function. It makes function more difficult
> to manage and we can introduce new bugs. Could you please localize your change
> without reworking this pattern of error situation management? Also, it will make
> the patch more compact. Could you please rework the patch?
> 
This change in particular is made by christian. As he mentionned in one 
of his emails to my patches[1], his logic was that hfs_mdb_put() should 
only be called in fill_super() which cleans everything up and that the 
cleanup labels don't make sense here which is why he spread the logic of 
cleanup across the function. Maybe he can give us more input on this 
since it wasn't my code.

[1]:https://lore.kernel.org/all/20251119-delfin-bioladen-6bf291941d4f@brauner/
> Thanks,
> Slava.
Best Regards,
Mehdi Ben Hadj Khelifa
> 
>>   
>>   		if (mdb->drSigWord == cpu_to_be16(HFS_SUPER_MAGIC))
>>   			break;
>> @@ -102,13 +102,14 @@ int hfs_mdb_get(struct super_block *sb)
>>   		 * (should do this only for cdrom/loop though)
>>   		 */
>>   		if (hfs_part_find(sb, &part_start, &part_size))
>> -			goto out;
>> +			return -EIO;
>>   	}
>>   
>>   	HFS_SB(sb)->alloc_blksz = size = be32_to_cpu(mdb->drAlBlkSiz);
>>   	if (!size || (size & (HFS_SECTOR_SIZE - 1))) {
>>   		pr_err("bad allocation block size %d\n", size);
>> -		goto out_bh;
>> +		brelse(bh);
>> +		return -EIO;
>>   	}
>>   
>>   	size = min(HFS_SB(sb)->alloc_blksz, (u32)PAGE_SIZE);
>> @@ -125,14 +126,16 @@ int hfs_mdb_get(struct super_block *sb)
>>   	brelse(bh);
>>   	if (!sb_set_blocksize(sb, size)) {
>>   		pr_err("unable to set blocksize to %u\n", size);
>> -		goto out;
>> +		return -EIO;
>>   	}
>>   
>>   	bh = sb_bread512(sb, part_start + HFS_MDB_BLK, mdb);
>>   	if (!bh)
>> -		goto out;
>> -	if (mdb->drSigWord != cpu_to_be16(HFS_SUPER_MAGIC))
>> -		goto out_bh;
>> +		return -EIO;
>> +	if (mdb->drSigWord != cpu_to_be16(HFS_SUPER_MAGIC)) {
>> +		brelse(bh);
>> +		return -EIO;
>> +	}
>>   
>>   	HFS_SB(sb)->mdb_bh = bh;
>>   	HFS_SB(sb)->mdb = mdb;
>> @@ -174,7 +177,7 @@ int hfs_mdb_get(struct super_block *sb)
>>   
>>   	HFS_SB(sb)->bitmap = kzalloc(8192, GFP_KERNEL);
>>   	if (!HFS_SB(sb)->bitmap)
>> -		goto out;
>> +		return -EIO;
>>   
>>   	/* read in the bitmap */
>>   	block = be16_to_cpu(mdb->drVBMSt) + part_start;
>> @@ -185,7 +188,7 @@ int hfs_mdb_get(struct super_block *sb)
>>   		bh = sb_bread(sb, off >> sb->s_blocksize_bits);
>>   		if (!bh) {
>>   			pr_err("unable to read volume bitmap\n");
>> -			goto out;
>> +			return -EIO;
>>   		}
>>   		off2 = off & (sb->s_blocksize - 1);
>>   		len = min((int)sb->s_blocksize - off2, size);
>> @@ -199,12 +202,12 @@ int hfs_mdb_get(struct super_block *sb)
>>   	HFS_SB(sb)->ext_tree = hfs_btree_open(sb, HFS_EXT_CNID, hfs_ext_keycmp);
>>   	if (!HFS_SB(sb)->ext_tree) {
>>   		pr_err("unable to open extent tree\n");
>> -		goto out;
>> +		return -EIO;
>>   	}
>>   	HFS_SB(sb)->cat_tree = hfs_btree_open(sb, HFS_CAT_CNID, hfs_cat_keycmp);
>>   	if (!HFS_SB(sb)->cat_tree) {
>>   		pr_err("unable to open catalog tree\n");
>> -		goto out;
>> +		return -EIO;
>>   	}
>>   
>>   	attrib = mdb->drAtrb;
>> @@ -229,12 +232,6 @@ int hfs_mdb_get(struct super_block *sb)
>>   	}
>>   
>>   	return 0;
>> -
>> -out_bh:
>> -	brelse(bh);
>> -out:
>> -	hfs_mdb_put(sb);
>> -	return -EIO;
>>   }
>>   
>>   /*
>> @@ -359,8 +356,6 @@ void hfs_mdb_close(struct super_block *sb)
>>    * Release the resources associated with the in-core MDB.  */
>>   void hfs_mdb_put(struct super_block *sb)
>>   {
>> -	if (!HFS_SB(sb))
>> -		return;
>>   	/* free the B-trees */
>>   	hfs_btree_close(HFS_SB(sb)->ext_tree);
>>   	hfs_btree_close(HFS_SB(sb)->cat_tree);
>> @@ -373,6 +368,4 @@ void hfs_mdb_put(struct super_block *sb)
>>   	unload_nls(HFS_SB(sb)->nls_disk);
>>   
>>   	kfree(HFS_SB(sb)->bitmap);
>> -	kfree(HFS_SB(sb));
>> -	sb->s_fs_info = NULL;
>>   }
>> diff --git a/fs/hfs/super.c b/fs/hfs/super.c
>> index 47f50fa555a4..df289cbdd4e8 100644
>> --- a/fs/hfs/super.c
>> +++ b/fs/hfs/super.c
>> @@ -431,10 +431,18 @@ static int hfs_init_fs_context(struct fs_context *fc)
>>   	return 0;
>>   }
>>   
>> +static void hfs_kill_super(struct super_block *sb)
>> +{
>> +	struct hfs_sb_info *hsb = HFS_SB(sb);
>> +
>> +	kill_block_super(sb);
>> +	kfree(hsb);
>> +}
>> +
>>   static struct file_system_type hfs_fs_type = {
>>   	.owner		= THIS_MODULE,
>>   	.name		= "hfs",
>> -	.kill_sb	= kill_block_super,
>> +	.kill_sb	= hfs_kill_super,
>>   	.fs_flags	= FS_REQUIRES_DEV,
>>   	.init_fs_context = hfs_init_fs_context,
>>   };

RE: [PATCH v3 1/2] hfs: ensure sb->s_fs_info is always cleaned up

[Viacheslav Dubeyko]

On Tue, 2025-12-02 at 11:16 +0100, Mehdi Ben Hadj Khelifa wrote:
> On 12/2/25 12:04 AM, Viacheslav Dubeyko wrote:
> > On Mon, 2025-12-01 at 23:23 +0100, Mehdi Ben Hadj Khelifa wrote:
> > > When hfs was converted to the new mount api a bug was introduced by
> > > changing the allocation pattern of sb->s_fs_info. If setup_bdev_super()
> > > fails after a new superblock has been allocated by sget_fc(), but before
> > > hfs_fill_super() takes ownership of the filesystem-specific s_fs_info
> > > data it was leaked.
> > > 
> > > Fix this by freeing sb->s_fs_info in hfs_kill_super().
> > > 
> > > Cc: stable@vger.kernel.org
> > > Fixes: ffcd06b6d13b ("hfs: convert hfs to use the new mount api")
> > > Reported-by: syzbot+ad45f827c88778ff7df6@syzkaller.appspotmail.com
> > > Closes: https://syzkaller.appspot.com/bug?extid=ad45f827c88778ff7df6  
> > > Tested-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
> > > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > > Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
> > > ---
> > >   fs/hfs/mdb.c   | 35 ++++++++++++++---------------------
> > >   fs/hfs/super.c | 10 +++++++++-
> > >   2 files changed, 23 insertions(+), 22 deletions(-)
> > > 
> > > diff --git a/fs/hfs/mdb.c b/fs/hfs/mdb.c
> > > index 53f3fae60217..f28cd24dee84 100644
> > > --- a/fs/hfs/mdb.c
> > > +++ b/fs/hfs/mdb.c
> > > @@ -92,7 +92,7 @@ int hfs_mdb_get(struct super_block *sb)
> > >   		/* See if this is an HFS filesystem */
> > >   		bh = sb_bread512(sb, part_start + HFS_MDB_BLK, mdb);
> > >   		if (!bh)
> > > -			goto out;
> > > +			return -EIO;
> > 
> > Frankly speaking, I don't see the point to rework the hfs_mdb_get() method so
> > intensively. We had pretty good pattern before:
> > 
> > int hfs_mdb_get(struct super_block *sb) {
> >          if (something_happens)
> >               goto out;
> > 
> >          if (something_happens_and_we_need_free_buffer)
> >              goto out_bh;
> > 
> >   	return 0;
> > 
> > out_bh:
> > 	brelse(bh);
> > out:
> > 	return -EIO;
> >   }
> > 
> > The point here that we have error management logic in one place. Now you have
> > spread this logic through the whole function. It makes function more difficult
> > to manage and we can introduce new bugs. Could you please localize your change
> > without reworking this pattern of error situation management? Also, it will make
> > the patch more compact. Could you please rework the patch?
> > 
> This change in particular is made by christian. As he mentionned in one 
> of his emails to my patches[1], his logic was that hfs_mdb_put() should 
> only be called in fill_super() which cleans everything up and that the 
> cleanup labels don't make sense here which is why he spread the logic of 
> cleanup across the function. Maybe he can give us more input on this 
> since it wasn't my code.
> 
> [1]:https://lore.kernel.org/all/20251119-delfin-bioladen-6bf291941d4f@brauner/  
> > 

I am not against of not calling the hfs_mdb_put() in hfs_mdb_get(). But if I am
trying to rework some method significantly, guys are not happy at all about it.
:) I am slightly worried about such significant rework of hfs_mdb_get() because
we potentially could introduce some new bugs. And I definitely will have the
conflict with another patch under review. :)

I've spent some more time for reviewing the patch again. And I think I can
accept it as it is. Currently, I don't see any serious issue in hfs_mdb_get().
It is simply my code style preferences. :) But people can see it in different
ways.

> > 
> > >   
> > >   		if (mdb->drSigWord == cpu_to_be16(HFS_SUPER_MAGIC))
> > >   			break;
> > > @@ -102,13 +102,14 @@ int hfs_mdb_get(struct super_block *sb)
> > >   		 * (should do this only for cdrom/loop though)
> > >   		 */
> > >   		if (hfs_part_find(sb, &part_start, &part_size))
> > > -			goto out;
> > > +			return -EIO;
> > >   	}
> > >   
> > >   	HFS_SB(sb)->alloc_blksz = size = be32_to_cpu(mdb->drAlBlkSiz);
> > >   	if (!size || (size & (HFS_SECTOR_SIZE - 1))) {
> > >   		pr_err("bad allocation block size %d\n", size);
> > > -		goto out_bh;
> > > +		brelse(bh);
> > > +		return -EIO;
> > >   	}
> > >   
> > >   	size = min(HFS_SB(sb)->alloc_blksz, (u32)PAGE_SIZE);
> > > @@ -125,14 +126,16 @@ int hfs_mdb_get(struct super_block *sb)
> > >   	brelse(bh);
> > >   	if (!sb_set_blocksize(sb, size)) {
> > >   		pr_err("unable to set blocksize to %u\n", size);
> > > -		goto out;
> > > +		return -EIO;
> > >   	}
> > >   
> > >   	bh = sb_bread512(sb, part_start + HFS_MDB_BLK, mdb);
> > >   	if (!bh)
> > > -		goto out;
> > > -	if (mdb->drSigWord != cpu_to_be16(HFS_SUPER_MAGIC))
> > > -		goto out_bh;
> > > +		return -EIO;
> > > +	if (mdb->drSigWord != cpu_to_be16(HFS_SUPER_MAGIC)) {
> > > +		brelse(bh);
> > > +		return -EIO;
> > > +	}
> > >   
> > >   	HFS_SB(sb)->mdb_bh = bh;
> > >   	HFS_SB(sb)->mdb = mdb;
> > > @@ -174,7 +177,7 @@ int hfs_mdb_get(struct super_block *sb)
> > >   
> > >   	HFS_SB(sb)->bitmap = kzalloc(8192, GFP_KERNEL);
> > >   	if (!HFS_SB(sb)->bitmap)
> > > -		goto out;
> > > +		return -EIO;
> > >   
> > >   	/* read in the bitmap */
> > >   	block = be16_to_cpu(mdb->drVBMSt) + part_start;
> > > @@ -185,7 +188,7 @@ int hfs_mdb_get(struct super_block *sb)
> > >   		bh = sb_bread(sb, off >> sb->s_blocksize_bits);
> > >   		if (!bh) {
> > >   			pr_err("unable to read volume bitmap\n");
> > > -			goto out;
> > > +			return -EIO;
> > >   		}
> > >   		off2 = off & (sb->s_blocksize - 1);
> > >   		len = min((int)sb->s_blocksize - off2, size);
> > > @@ -199,12 +202,12 @@ int hfs_mdb_get(struct super_block *sb)
> > >   	HFS_SB(sb)->ext_tree = hfs_btree_open(sb, HFS_EXT_CNID, hfs_ext_keycmp);
> > >   	if (!HFS_SB(sb)->ext_tree) {
> > >   		pr_err("unable to open extent tree\n");
> > > -		goto out;
> > > +		return -EIO;
> > >   	}
> > >   	HFS_SB(sb)->cat_tree = hfs_btree_open(sb, HFS_CAT_CNID, hfs_cat_keycmp);
> > >   	if (!HFS_SB(sb)->cat_tree) {
> > >   		pr_err("unable to open catalog tree\n");
> > > -		goto out;
> > > +		return -EIO;
> > >   	}
> > >   
> > >   	attrib = mdb->drAtrb;
> > > @@ -229,12 +232,6 @@ int hfs_mdb_get(struct super_block *sb)
> > >   	}
> > >   
> > >   	return 0;
> > > -
> > > -out_bh:
> > > -	brelse(bh);
> > > -out:
> > > -	hfs_mdb_put(sb);
> > > -	return -EIO;
> > >   }
> > >   
> > >   /*
> > > @@ -359,8 +356,6 @@ void hfs_mdb_close(struct super_block *sb)
> > >    * Release the resources associated with the in-core MDB.  */
> > >   void hfs_mdb_put(struct super_block *sb)
> > >   {
> > > -	if (!HFS_SB(sb))
> > > -		return;
> > >   	/* free the B-trees */
> > >   	hfs_btree_close(HFS_SB(sb)->ext_tree);
> > >   	hfs_btree_close(HFS_SB(sb)->cat_tree);
> > > @@ -373,6 +368,4 @@ void hfs_mdb_put(struct super_block *sb)
> > >   	unload_nls(HFS_SB(sb)->nls_disk);
> > >   
> > >   	kfree(HFS_SB(sb)->bitmap);
> > > -	kfree(HFS_SB(sb));
> > > -	sb->s_fs_info = NULL;
> > >   }
> > > diff --git a/fs/hfs/super.c b/fs/hfs/super.c
> > > index 47f50fa555a4..df289cbdd4e8 100644
> > > --- a/fs/hfs/super.c
> > > +++ b/fs/hfs/super.c
> > > @@ -431,10 +431,18 @@ static int hfs_init_fs_context(struct fs_context *fc)
> > >   	return 0;
> > >   }
> > >   
> > > +static void hfs_kill_super(struct super_block *sb)
> > > +{
> > > +	struct hfs_sb_info *hsb = HFS_SB(sb);
> > > +
> > > +	kill_block_super(sb);
> > > +	kfree(hsb);
> > > +}
> > > +
> > >   static struct file_system_type hfs_fs_type = {
> > >   	.owner		= THIS_MODULE,
> > >   	.name		= "hfs",
> > > -	.kill_sb	= kill_block_super,
> > > +	.kill_sb	= hfs_kill_super,
> > >   	.fs_flags	= FS_REQUIRES_DEV,
> > >   	.init_fs_context = hfs_init_fs_context,
> > >   };

Looks good. Thanks a lot for the fix.

Reviewed-by: Viacheslav Dubeyko <slava@dubeyko.com>

Thanks,
Slava.

Re: [PATCH v3 1/2] hfs: ensure sb->s_fs_info is always cleaned up

[Mehdi Ben Hadj Khelifa]

On 12/4/25 12:19 AM, Viacheslav Dubeyko wrote:
> On Tue, 2025-12-02 at 11:16 +0100, Mehdi Ben Hadj Khelifa wrote:
>> On 12/2/25 12:04 AM, Viacheslav Dubeyko wrote:
>>> On Mon, 2025-12-01 at 23:23 +0100, Mehdi Ben Hadj Khelifa wrote:
>>>> When hfs was converted to the new mount api a bug was introduced by
>>>> changing the allocation pattern of sb->s_fs_info. If setup_bdev_super()
>>>> fails after a new superblock has been allocated by sget_fc(), but before
>>>> hfs_fill_super() takes ownership of the filesystem-specific s_fs_info
>>>> data it was leaked.
>>>>
>>>> Fix this by freeing sb->s_fs_info in hfs_kill_super().
>>>>
>>>> Cc: stable@vger.kernel.org
>>>> Fixes: ffcd06b6d13b ("hfs: convert hfs to use the new mount api")
>>>> Reported-by: syzbot+ad45f827c88778ff7df6@syzkaller.appspotmail.com
>>>> Closes: https://syzkaller.appspot.com/bug?extid=ad45f827c88778ff7df6
>>>> Tested-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
>>>> Signed-off-by: Christian Brauner <brauner@kernel.org>
>>>> Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
>>>> ---
>>>>    fs/hfs/mdb.c   | 35 ++++++++++++++---------------------
>>>>    fs/hfs/super.c | 10 +++++++++-
>>>>    2 files changed, 23 insertions(+), 22 deletions(-)
>>>>
>>>> diff --git a/fs/hfs/mdb.c b/fs/hfs/mdb.c
>>>> index 53f3fae60217..f28cd24dee84 100644
>>>> --- a/fs/hfs/mdb.c
>>>> +++ b/fs/hfs/mdb.c
>>>> @@ -92,7 +92,7 @@ int hfs_mdb_get(struct super_block *sb)
>>>>    		/* See if this is an HFS filesystem */
>>>>    		bh = sb_bread512(sb, part_start + HFS_MDB_BLK, mdb);
>>>>    		if (!bh)
>>>> -			goto out;
>>>> +			return -EIO;
>>>
>>> Frankly speaking, I don't see the point to rework the hfs_mdb_get() method so
>>> intensively. We had pretty good pattern before:
>>>
>>> int hfs_mdb_get(struct super_block *sb) {
>>>           if (something_happens)
>>>                goto out;
>>>
>>>           if (something_happens_and_we_need_free_buffer)
>>>               goto out_bh;
>>>
>>>    	return 0;
>>>
>>> out_bh:
>>> 	brelse(bh);
>>> out:
>>> 	return -EIO;
>>>    }
>>>
>>> The point here that we have error management logic in one place. Now you have
>>> spread this logic through the whole function. It makes function more difficult
>>> to manage and we can introduce new bugs. Could you please localize your change
>>> without reworking this pattern of error situation management? Also, it will make
>>> the patch more compact. Could you please rework the patch?
>>>
>> This change in particular is made by christian. As he mentionned in one
>> of his emails to my patches[1], his logic was that hfs_mdb_put() should
>> only be called in fill_super() which cleans everything up and that the
>> cleanup labels don't make sense here which is why he spread the logic of
>> cleanup across the function. Maybe he can give us more input on this
>> since it wasn't my code.
>>
>> [1]:https://lore.kernel.org/all/20251119-delfin-bioladen-6bf291941d4f@brauner/
>>>
> 
> I am not against of not calling the hfs_mdb_put() in hfs_mdb_get(). But if I am
> trying to rework some method significantly, guys are not happy at all about it.
> :) I am slightly worried about such significant rework of hfs_mdb_get() because
> we potentially could introduce some new bugs. And I definitely will have the
> conflict with another patch under review. :)
> 

Totally understandable. If I was to make that change I would probably 
seperate it from the fix (except the part where we delete freeing the 
s_fs_info struct). But I guess christian wanted to do the whole 
refactoring since it was related and it made more sense as he explained it.
> I've spent some more time for reviewing the patch again. And I think I can
> accept it as it is. Currently, I don't see any serious issue in hfs_mdb_get().
> It is simply my code style preferences. :) But people can see it in different
> ways.
> 
Thanks for you time and effort!
>>>
>>>>    
>>>>    		if (mdb->drSigWord == cpu_to_be16(HFS_SUPER_MAGIC))
>>>>    			break;
>>>> @@ -102,13 +102,14 @@ int hfs_mdb_get(struct super_block *sb)
>>>>    		 * (should do this only for cdrom/loop though)
>>>>    		 */
>>>>    		if (hfs_part_find(sb, &part_start, &part_size))
>>>> -			goto out;
>>>> +			return -EIO;
>>>>    	}
>>>>    
>>>>    	HFS_SB(sb)->alloc_blksz = size = be32_to_cpu(mdb->drAlBlkSiz);
>>>>    	if (!size || (size & (HFS_SECTOR_SIZE - 1))) {
>>>>    		pr_err("bad allocation block size %d\n", size);
>>>> -		goto out_bh;
>>>> +		brelse(bh);
>>>> +		return -EIO;
>>>>    	}
>>>>    
>>>>    	size = min(HFS_SB(sb)->alloc_blksz, (u32)PAGE_SIZE);
>>>> @@ -125,14 +126,16 @@ int hfs_mdb_get(struct super_block *sb)
>>>>    	brelse(bh);
>>>>    	if (!sb_set_blocksize(sb, size)) {
>>>>    		pr_err("unable to set blocksize to %u\n", size);
>>>> -		goto out;
>>>> +		return -EIO;
>>>>    	}
>>>>    
>>>>    	bh = sb_bread512(sb, part_start + HFS_MDB_BLK, mdb);
>>>>    	if (!bh)
>>>> -		goto out;
>>>> -	if (mdb->drSigWord != cpu_to_be16(HFS_SUPER_MAGIC))
>>>> -		goto out_bh;
>>>> +		return -EIO;
>>>> +	if (mdb->drSigWord != cpu_to_be16(HFS_SUPER_MAGIC)) {
>>>> +		brelse(bh);
>>>> +		return -EIO;
>>>> +	}
>>>>    
>>>>    	HFS_SB(sb)->mdb_bh = bh;
>>>>    	HFS_SB(sb)->mdb = mdb;
>>>> @@ -174,7 +177,7 @@ int hfs_mdb_get(struct super_block *sb)
>>>>    
>>>>    	HFS_SB(sb)->bitmap = kzalloc(8192, GFP_KERNEL);
>>>>    	if (!HFS_SB(sb)->bitmap)
>>>> -		goto out;
>>>> +		return -EIO;
>>>>    
>>>>    	/* read in the bitmap */
>>>>    	block = be16_to_cpu(mdb->drVBMSt) + part_start;
>>>> @@ -185,7 +188,7 @@ int hfs_mdb_get(struct super_block *sb)
>>>>    		bh = sb_bread(sb, off >> sb->s_blocksize_bits);
>>>>    		if (!bh) {
>>>>    			pr_err("unable to read volume bitmap\n");
>>>> -			goto out;
>>>> +			return -EIO;
>>>>    		}
>>>>    		off2 = off & (sb->s_blocksize - 1);
>>>>    		len = min((int)sb->s_blocksize - off2, size);
>>>> @@ -199,12 +202,12 @@ int hfs_mdb_get(struct super_block *sb)
>>>>    	HFS_SB(sb)->ext_tree = hfs_btree_open(sb, HFS_EXT_CNID, hfs_ext_keycmp);
>>>>    	if (!HFS_SB(sb)->ext_tree) {
>>>>    		pr_err("unable to open extent tree\n");
>>>> -		goto out;
>>>> +		return -EIO;
>>>>    	}
>>>>    	HFS_SB(sb)->cat_tree = hfs_btree_open(sb, HFS_CAT_CNID, hfs_cat_keycmp);
>>>>    	if (!HFS_SB(sb)->cat_tree) {
>>>>    		pr_err("unable to open catalog tree\n");
>>>> -		goto out;
>>>> +		return -EIO;
>>>>    	}
>>>>    
>>>>    	attrib = mdb->drAtrb;
>>>> @@ -229,12 +232,6 @@ int hfs_mdb_get(struct super_block *sb)
>>>>    	}
>>>>    
>>>>    	return 0;
>>>> -
>>>> -out_bh:
>>>> -	brelse(bh);
>>>> -out:
>>>> -	hfs_mdb_put(sb);
>>>> -	return -EIO;
>>>>    }
>>>>    
>>>>    /*
>>>> @@ -359,8 +356,6 @@ void hfs_mdb_close(struct super_block *sb)
>>>>     * Release the resources associated with the in-core MDB.  */
>>>>    void hfs_mdb_put(struct super_block *sb)
>>>>    {
>>>> -	if (!HFS_SB(sb))
>>>> -		return;
>>>>    	/* free the B-trees */
>>>>    	hfs_btree_close(HFS_SB(sb)->ext_tree);
>>>>    	hfs_btree_close(HFS_SB(sb)->cat_tree);
>>>> @@ -373,6 +368,4 @@ void hfs_mdb_put(struct super_block *sb)
>>>>    	unload_nls(HFS_SB(sb)->nls_disk);
>>>>    
>>>>    	kfree(HFS_SB(sb)->bitmap);
>>>> -	kfree(HFS_SB(sb));
>>>> -	sb->s_fs_info = NULL;
>>>>    }
>>>> diff --git a/fs/hfs/super.c b/fs/hfs/super.c
>>>> index 47f50fa555a4..df289cbdd4e8 100644
>>>> --- a/fs/hfs/super.c
>>>> +++ b/fs/hfs/super.c
>>>> @@ -431,10 +431,18 @@ static int hfs_init_fs_context(struct fs_context *fc)
>>>>    	return 0;
>>>>    }
>>>>    
>>>> +static void hfs_kill_super(struct super_block *sb)
>>>> +{
>>>> +	struct hfs_sb_info *hsb = HFS_SB(sb);
>>>> +
>>>> +	kill_block_super(sb);
>>>> +	kfree(hsb);
>>>> +}
>>>> +
>>>>    static struct file_system_type hfs_fs_type = {
>>>>    	.owner		= THIS_MODULE,
>>>>    	.name		= "hfs",
>>>> -	.kill_sb	= kill_block_super,
>>>> +	.kill_sb	= hfs_kill_super,
>>>>    	.fs_flags	= FS_REQUIRES_DEV,
>>>>    	.init_fs_context = hfs_init_fs_context,
>>>>    };
> 
> Looks good. Thanks a lot for the fix.
> 
> Reviewed-by: Viacheslav Dubeyko <slava@dubeyko.com>
> 
> Thanks,
> Slava.

Best Regards,
Mehdi Ben Hadj khelifa

[PATCH v3 2/2] hfsplus: ensure sb->s_fs_info is always cleaned up

[Mehdi Ben Hadj Khelifa]

When hfsplus was converted to the new mount api a bug was introduced by
changing the allocation pattern of sb->s_fs_info. If setup_bdev_super()
fails after a new superblock has been allocated by sget_fc(), but before
hfsplus_fill_super() takes ownership of the filesystem-specific s_fs_info
data it was leaked.

Fix this by freeing sb->s_fs_info in hfsplus_kill_super().

Cc: stable@vger.kernel.org
Fixes: 432f7c78cb00 ("hfsplus: convert hfsplus to use the new mount api")
Reported-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com> 
Tested-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
Signed-off-by: Christian Brauner <brauner@kernel.org>
Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
---
 fs/hfsplus/super.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
index 16bc4abc67e0..8734520f6419 100644
--- a/fs/hfsplus/super.c
+++ b/fs/hfsplus/super.c
@@ -328,8 +328,6 @@ static void hfsplus_put_super(struct super_block *sb)
 	hfs_btree_close(sbi->ext_tree);
 	kfree(sbi->s_vhdr_buf);
 	kfree(sbi->s_backup_vhdr_buf);
-	call_rcu(&sbi->rcu, delayed_free);
-
 	hfs_dbg("finished\n");
 }
 
@@ -629,7 +627,6 @@ static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc)
 out_unload_nls:
 	unload_nls(sbi->nls);
 	unload_nls(nls);
-	kfree(sbi);
 	return err;
 }
 
@@ -688,10 +685,18 @@ static int hfsplus_init_fs_context(struct fs_context *fc)
 	return 0;
 }
 
+static void hfsplus_kill_super(struct super_block *sb)
+{
+	struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
+
+	kill_block_super(sb);
+	call_rcu(&sbi->rcu, delayed_free);
+}
+
 static struct file_system_type hfsplus_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "hfsplus",
-	.kill_sb	= kill_block_super,
+	.kill_sb	= hfsplus_kill_super,
 	.fs_flags	= FS_REQUIRES_DEV,
 	.init_fs_context = hfsplus_init_fs_context,
 };
-- 
2.52.0

Re: [PATCH v3 2/2] hfsplus: ensure sb->s_fs_info is always cleaned up

[Viacheslav Dubeyko]

On Mon, 2025-12-01 at 23:23 +0100, Mehdi Ben Hadj Khelifa wrote:
> When hfsplus was converted to the new mount api a bug was introduced by
> changing the allocation pattern of sb->s_fs_info. If setup_bdev_super()
> fails after a new superblock has been allocated by sget_fc(), but before
> hfsplus_fill_super() takes ownership of the filesystem-specific s_fs_info
> data it was leaked.
> 
> Fix this by freeing sb->s_fs_info in hfsplus_kill_super().
> 
> Cc: stable@vger.kernel.org
> Fixes: 432f7c78cb00 ("hfsplus: convert hfsplus to use the new mount api")
> Reported-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com> 
> Tested-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
> ---
>  fs/hfsplus/super.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/hfsplus/super.c b/fs/hfsplus/super.c
> index 16bc4abc67e0..8734520f6419 100644
> --- a/fs/hfsplus/super.c
> +++ b/fs/hfsplus/super.c
> @@ -328,8 +328,6 @@ static void hfsplus_put_super(struct super_block *sb)
>  	hfs_btree_close(sbi->ext_tree);
>  	kfree(sbi->s_vhdr_buf);
>  	kfree(sbi->s_backup_vhdr_buf);
> -	call_rcu(&sbi->rcu, delayed_free);
> -
>  	hfs_dbg("finished\n");
>  }
>  
> @@ -629,7 +627,6 @@ static int hfsplus_fill_super(struct super_block *sb, struct fs_context *fc)
>  out_unload_nls:
>  	unload_nls(sbi->nls);
>  	unload_nls(nls);
> -	kfree(sbi);
>  	return err;
>  }
>  
> @@ -688,10 +685,18 @@ static int hfsplus_init_fs_context(struct fs_context *fc)
>  	return 0;
>  }
>  
> +static void hfsplus_kill_super(struct super_block *sb)
> +{
> +	struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
> +
> +	kill_block_super(sb);
> +	call_rcu(&sbi->rcu, delayed_free);
> +}
> +
>  static struct file_system_type hfsplus_fs_type = {
>  	.owner		= THIS_MODULE,
>  	.name		= "hfsplus",
> -	.kill_sb	= kill_block_super,
> +	.kill_sb	= hfsplus_kill_super,
>  	.fs_flags	= FS_REQUIRES_DEV,
>  	.init_fs_context = hfsplus_init_fs_context,
>  };

Looks good. Thanks a lot for the fix.

Reviewed-by: Viacheslav Dubeyko <slava@dubeyko.com>

Thanks,
Slava.