| From: Chao Yu <chao2.yu@samsung.com> |
| Date: Sat, 30 Aug 2014 09:52:34 +0800 |
| Subject: f2fs: reposition unlock_new_inode to prevent accessing invalid inode |
| |
| commit b73e52824c8920a5ff754e3c8ff68466a7dd61f9 upstream. |
| |
| As the race condition on the inode cache, following scenario can appear: |
| [Thread a] [Thread b] |
| ->f2fs_mkdir |
| ->f2fs_add_link |
| ->__f2fs_add_link |
| ->init_inode_metadata failed here |
| ->gc_thread_func |
| ->f2fs_gc |
| ->do_garbage_collect |
| ->gc_data_segment |
| ->f2fs_iget |
| ->iget_locked |
| ->wait_on_inode |
| ->unlock_new_inode |
| ->move_data_page |
| ->make_bad_inode |
| ->iput |
| |
| When we fail in create/symlink/mkdir/mknod/tmpfile, the new allocated inode |
| should be set as bad to avoid being accessed by other thread. But in above |
| scenario, it allows f2fs to access the invalid inode before this inode was set |
| as bad. |
| This patch fix the potential problem, and this issue was found by code review. |
| |
| change log from v1: |
| o Add condition judgment in gc_data_segment() suggested by Changman Lee. |
| o use iget_failed to simplify code. |
| |
| Signed-off-by: Chao Yu <chao2.yu@samsung.com> |
| Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> |
| [bwh: Backported to 3.16: Drop changes in f2fs_tmpfile()] |
| Signed-off-by: Ben Hutchings <ben@decadent.org.uk> |
| --- |
| --- a/fs/f2fs/gc.c |
| +++ b/fs/f2fs/gc.c |
| @@ -602,7 +602,7 @@ next_step: |
| |
| if (phase == 2) { |
| inode = f2fs_iget(sb, dni.ino); |
| - if (IS_ERR(inode)) |
| + if (IS_ERR(inode) || is_bad_inode(inode)) |
| continue; |
| |
| start_bidx = start_bidx_of_node(nofs, F2FS_I(inode)); |
| --- a/fs/f2fs/namei.c |
| +++ b/fs/f2fs/namei.c |
| @@ -135,9 +135,7 @@ static int f2fs_create(struct inode *dir |
| return 0; |
| out: |
| clear_nlink(inode); |
| - unlock_new_inode(inode); |
| - make_bad_inode(inode); |
| - iput(inode); |
| + iget_failed(inode); |
| alloc_nid_failed(sbi, ino); |
| return err; |
| } |
| @@ -271,9 +269,7 @@ static int f2fs_symlink(struct inode *di |
| return err; |
| out: |
| clear_nlink(inode); |
| - unlock_new_inode(inode); |
| - make_bad_inode(inode); |
| - iput(inode); |
| + iget_failed(inode); |
| alloc_nid_failed(sbi, inode->i_ino); |
| return err; |
| } |
| @@ -312,9 +308,7 @@ static int f2fs_mkdir(struct inode *dir, |
| out_fail: |
| clear_inode_flag(F2FS_I(inode), FI_INC_LINK); |
| clear_nlink(inode); |
| - unlock_new_inode(inode); |
| - make_bad_inode(inode); |
| - iput(inode); |
| + iget_failed(inode); |
| alloc_nid_failed(sbi, inode->i_ino); |
| return err; |
| } |
| @@ -359,9 +353,7 @@ static int f2fs_mknod(struct inode *dir, |
| return 0; |
| out: |
| clear_nlink(inode); |
| - unlock_new_inode(inode); |
| - make_bad_inode(inode); |
| - iput(inode); |
| + iget_failed(inode); |
| alloc_nid_failed(sbi, inode->i_ino); |
| return err; |
| } |