[PATCH] f2fs: fix double free of f2fs_sb_info

[PATCH] f2fs: fix double free of f2fs_sb_info

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

kill_f2fs_super() is called even if f2fs_fill_super() fails.
f2fs_fill_super() frees the struct f2fs_sb_info, so it must set
sb->s_fs_info to NULL to prevent it from being freed again.

Fixes: 275dca4630c1 ("f2fs: move release of block devices to after kill_block_super()")
Reported-by: syzbot+8f477ac014ff5b32d81f@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/r/0000000000006cb174060ec34502@google.com
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/f2fs/super.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index d00d21a8b53ad..d45ab0992ae59 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -4873,20 +4873,21 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
 		kfree(F2FS_OPTION(sbi).s_qf_names[i]);
 #endif
 	fscrypt_free_dummy_policy(&F2FS_OPTION(sbi).dummy_enc_policy);
 	kvfree(options);
 free_sb_buf:
 	kfree(raw_super);
 free_sbi:
 	if (sbi->s_chksum_driver)
 		crypto_free_shash(sbi->s_chksum_driver);
 	kfree(sbi);
+	sb->s_fs_info = NULL;
 
 	/* give only one another chance */
 	if (retry_cnt > 0 && skip_recovery) {
 		retry_cnt--;
 		shrink_dcache_sb(sb);
 		goto try_onemore;
 	}
 	return err;
 }
 

base-commit: 38814330fedd778edffcabe0c8cb462ee365782e
-- 
2.43.0

Re: [PATCH] f2fs: fix double free of f2fs_sb_info

[Eric Biggers]

On Fri, Jan 12, 2024 at 04:57:47PM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> kill_f2fs_super() is called even if f2fs_fill_super() fails.
> f2fs_fill_super() frees the struct f2fs_sb_info, so it must set
> sb->s_fs_info to NULL to prevent it from being freed again.
> 
> Fixes: 275dca4630c1 ("f2fs: move release of block devices to after kill_block_super()")
> Reported-by: syzbot+8f477ac014ff5b32d81f@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/r/0000000000006cb174060ec34502@google.com
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Jaegeuk, I'd be glad to take this through the fscrypt tree since that's where my
broken commit came from.  But let me know if you want to just take this through
the f2fs tree.

- Eric

Re: [f2fs-dev] [PATCH] f2fs: fix double free of f2fs_sb_info

[Chao Yu]

On 2024/1/13 8:57, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> kill_f2fs_super() is called even if f2fs_fill_super() fails.
> f2fs_fill_super() frees the struct f2fs_sb_info, so it must set
> sb->s_fs_info to NULL to prevent it from being freed again.

Oh, I missed that case as well during reviewing, my bad.

> 
> Fixes: 275dca4630c1 ("f2fs: move release of block devices to after kill_block_super()")
> Reported-by: syzbot+8f477ac014ff5b32d81f@syzkaller.appspotmail.com
> Closes: https://lore.kernel.org/r/0000000000006cb174060ec34502@google.com
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,

> ---
>   fs/f2fs/super.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index d00d21a8b53ad..d45ab0992ae59 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -4873,20 +4873,21 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)
>   		kfree(F2FS_OPTION(sbi).s_qf_names[i]);
>   #endif
>   	fscrypt_free_dummy_policy(&F2FS_OPTION(sbi).dummy_enc_policy);
>   	kvfree(options);
>   free_sb_buf:
>   	kfree(raw_super);
>   free_sbi:
>   	if (sbi->s_chksum_driver)
>   		crypto_free_shash(sbi->s_chksum_driver);
>   	kfree(sbi);
> +	sb->s_fs_info = NULL;
>   
>   	/* give only one another chance */
>   	if (retry_cnt > 0 && skip_recovery) {
>   		retry_cnt--;
>   		shrink_dcache_sb(sb);
>   		goto try_onemore;
>   	}
>   	return err;
>   }
>   
> 
> base-commit: 38814330fedd778edffcabe0c8cb462ee365782e

Re: [PATCH] f2fs: fix double free of f2fs_sb_info

[Jaegeuk Kim]

On 01/12, Eric Biggers wrote:
> On Fri, Jan 12, 2024 at 04:57:47PM -0800, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > kill_f2fs_super() is called even if f2fs_fill_super() fails.
> > f2fs_fill_super() frees the struct f2fs_sb_info, so it must set
> > sb->s_fs_info to NULL to prevent it from being freed again.
> > 
> > Fixes: 275dca4630c1 ("f2fs: move release of block devices to after kill_block_super()")
> > Reported-by: syzbot+8f477ac014ff5b32d81f@syzkaller.appspotmail.com
> > Closes: https://lore.kernel.org/r/0000000000006cb174060ec34502@google.com
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> 
> Jaegeuk, I'd be glad to take this through the fscrypt tree since that's where my

Ok, are you heading to push this in -rc1?

> broken commit came from.  But let me know if you want to just take this through
> the f2fs tree.
> 
> - Eric

Re: [PATCH] f2fs: fix double free of f2fs_sb_info

[Eric Biggers]

On Fri, Jan 12, 2024 at 05:28:31PM -0800, Jaegeuk Kim wrote:
> On 01/12, Eric Biggers wrote:
> > On Fri, Jan 12, 2024 at 04:57:47PM -0800, Eric Biggers wrote:
> > > From: Eric Biggers <ebiggers@google.com>
> > > 
> > > kill_f2fs_super() is called even if f2fs_fill_super() fails.
> > > f2fs_fill_super() frees the struct f2fs_sb_info, so it must set
> > > sb->s_fs_info to NULL to prevent it from being freed again.
> > > 
> > > Fixes: 275dca4630c1 ("f2fs: move release of block devices to after kill_block_super()")
> > > Reported-by: syzbot+8f477ac014ff5b32d81f@syzkaller.appspotmail.com
> > > Closes: https://lore.kernel.org/r/0000000000006cb174060ec34502@google.com
> > > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > 
> > Jaegeuk, I'd be glad to take this through the fscrypt tree since that's where my
> 
> Ok, are you heading to push this in -rc1?
> 
> > broken commit came from.  But let me know if you want to just take this through
> > the f2fs tree.
> > 

Yes, we should get this into -rc1.

- Eric

Re: [PATCH] f2fs: fix double free of f2fs_sb_info

[Jaegeuk Kim]

On 01/12, Eric Biggers wrote:
> On Fri, Jan 12, 2024 at 05:28:31PM -0800, Jaegeuk Kim wrote:
> > On 01/12, Eric Biggers wrote:
> > > On Fri, Jan 12, 2024 at 04:57:47PM -0800, Eric Biggers wrote:
> > > > From: Eric Biggers <ebiggers@google.com>
> > > > 
> > > > kill_f2fs_super() is called even if f2fs_fill_super() fails.
> > > > f2fs_fill_super() frees the struct f2fs_sb_info, so it must set
> > > > sb->s_fs_info to NULL to prevent it from being freed again.
> > > > 
> > > > Fixes: 275dca4630c1 ("f2fs: move release of block devices to after kill_block_super()")
> > > > Reported-by: syzbot+8f477ac014ff5b32d81f@syzkaller.appspotmail.com
> > > > Closes: https://lore.kernel.org/r/0000000000006cb174060ec34502@google.com
> > > > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > > 
> > > Jaegeuk, I'd be glad to take this through the fscrypt tree since that's where my
> > 
> > Ok, are you heading to push this in -rc1?
> > 
> > > broken commit came from.  But let me know if you want to just take this through
> > > the f2fs tree.
> > > 
> 
> Yes, we should get this into -rc1.

Ok, please do so.

> 
> - Eric