f2fs: fix use-after-free issue when accessing sbi->stat_info
PATCH v2 1/1 f2fs: fix use-after-free issue when accessing sbi->stat_info
iput() on sbi->node_inode can update sbi->stat_info in the below context, if the f2fs_write_checkpoint() has failed with error.
f2fs_balance_fs_bg+0x1ac/0x1ec f2fs_write_node_pages+0x4c/0x260 do_writepages+0x80/0xbc __writeback_single_inode+0xdc/0x4ac writeback_single_inode+0x9c/0x144 write_inode_now+0xc4/0xec iput+0x194/0x22c f2fs_put_super+0x11c/0x1e8 generic_shutdown_super+0x70/0xf4 kill_block_super+0x2c/0x5c kill_f2fs_super+0x44/0x50 deactivate_locked_super+0x60/0x8c deactivate_super+0x68/0x74 cleanup_mnt+0x40/0x78
Fix this by moving f2fs_destroy_stats() further below iput() in both f2fs_put_super() and f2fs_fill_super() paths.
Signed-off-by Sahitya Tummala <[email protected]>
---
v2:
- cover f2fs_fill_super() also, as pointed out by Chao.
fs/f2fs/super.c | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-)
fs/f2fs/super.c +15 -12
@@ -1058,9 +1058,6 @@ static void f2fs_put_super(struct super_block *sb)f2fs_write_checkpoint (sbi, &cpc); } -/* f2fs_write_checkpoint can update stat informaion */ -f2fs_destroy_stats (sbi); -/* * normally superblock is clean, so we need to release this. * In addition, EIO will skip do checkpoint, we need this as well.
@@ -1080,6 +1077,12 @@ static void f2fs_put_super(struct super_block *sb)iput (sbi->node_inode);iput (sbi->meta_inode); +/* +* iput() can update stat information, if f2fs_write_checkpoint() +* above failed with error. +*/ +f2fs_destroy_stats (sbi); +/* destroy f2fs internal modules */ f2fs_destroy_node_manager (sbi);f2fs_destroy_segment_manager (sbi);
@@ -3259,30 +3262,30 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent)f2fs_build_gc_manager (sbi); + err =f2fs_build_stats (sbi); +if (err) + goto free_nm; +/* get an inode for node space */ sbi->node_inode =f2fs_iget (sb,F2FS_NODE_INO (sbi));if (IS_ERR (sbi->node_inode)) {f2fs_msg (sb,KERN_ERR ,"Failed to read node inode" ); err =PTR_ERR (sbi->node_inode); - goto free_nm; + goto free_stats; } - err =f2fs_build_stats (sbi); -if (err) - goto free_node_inode; -/* read root inode and dentry */ root =f2fs_iget (sb,F2FS_ROOT_INO (sbi));if (IS_ERR (root)) {f2fs_msg (sb,KERN_ERR ,"Failed to read root inode" ); err =PTR_ERR (root); - goto free_stats; + goto free_node_inode; }if (!S_ISDIR (root->i_mode) || !root->i_blocks || !root->i_size || !root->i_nlink) {iput (root); err = -EINVAL ; - goto free_stats; + goto free_node_inode; } sb->s_root =d_make_root (root);/* allocate root dentry */
@@ -3406,12 +3409,12 @@ static int f2fs_fill_super(struct super_block *sb, void *data, int silent) free_root_inode:dput (sb->s_root); sb->s_root =NULL ; -free_stats: -f2fs_destroy_stats (sbi); free_node_inode:f2fs_release_ino_entry (sbi, true);truncate_inode_pages_final (NODE_MAPPING (sbi));iput (sbi->node_inode); +free_stats: +f2fs_destroy_stats (sbi); free_nm:f2fs_destroy_node_manager (sbi); free_sm:
Signature · 2 lines
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project.
Quoted 19 lines · Sahitya Tummala
iput()onsbi->node_inodecan updatesbi->stat_infoin the below context, if thef2fs_write_checkpoint()has failed with error.f2fs_balance_fs_bg+0x1ac/0x1ec f2fs_write_node_pages+0x4c/0x260 do_writepages+0x80/0xbc __writeback_single_inode+0xdc/0x4ac writeback_single_inode+0x9c/0x144 write_inode_now+0xc4/0xec iput+0x194/0x22c f2fs_put_super+0x11c/0x1e8 generic_shutdown_super+0x70/0xf4 kill_block_super+0x2c/0x5c kill_f2fs_super+0x44/0x50 deactivate_locked_super+0x60/0x8c deactivate_super+0x68/0x74 cleanup_mnt+0x40/0x78
Quoted 4 lines
Fix this by moving
f2fs_destroy_stats()further belowiput()in bothf2fs_put_super()andf2fs_fill_super()paths.Signed-off-by Sahitya Tummala <[email protected]>
It looks good to me now. :)
Reviewed-by Chao Yu <[email protected]>
Thanks,