| From ae2c5076a9c9cd4c6794a957452442b92e97410c Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Fri, 2 Oct 2020 14:17:35 -0700 |
| Subject: f2fs: handle errors of f2fs_get_meta_page_nofail |
| |
| From: Jaegeuk Kim <jaegeuk@kernel.org> |
| |
| [ Upstream commit 86f33603f8c51537265ff7ac0320638fd2cbdb1b ] |
| |
| First problem is we hit BUG_ON() in f2fs_get_sum_page given EIO on |
| f2fs_get_meta_page_nofail(). |
| |
| Quick fix was not to give any error with infinite loop, but syzbot caught |
| a case where it goes to that loop from fuzzed image. In turned out we abused |
| f2fs_get_meta_page_nofail() like in the below call stack. |
| |
| - f2fs_fill_super |
| - f2fs_build_segment_manager |
| - build_sit_entries |
| - get_current_sit_page |
| |
| INFO: task syz-executor178:6870 can't die for more than 143 seconds. |
| task:syz-executor178 state:R |
| stack:26960 pid: 6870 ppid: 6869 flags:0x00004006 |
| Call Trace: |
| |
| Showing all locks held in the system: |
| 1 lock held by khungtaskd/1179: |
| #0: ffffffff8a554da0 (rcu_read_lock){....}-{1:2}, at: debug_show_all_locks+0x53/0x260 kernel/locking/lockdep.c:6242 |
| 1 lock held by systemd-journal/3920: |
| 1 lock held by in:imklog/6769: |
| #0: ffff88809eebc130 (&f->f_pos_lock){+.+.}-{3:3}, at: __fdget_pos+0xe9/0x100 fs/file.c:930 |
| 1 lock held by syz-executor178/6870: |
| #0: ffff8880925120e0 (&type->s_umount_key#47/1){+.+.}-{3:3}, at: alloc_super+0x201/0xaf0 fs/super.c:229 |
| |
| Actually, we didn't have to use _nofail in this case, since we could return |
| error to mount(2) already with the error handler. |
| |
| As a result, this patch tries to 1) remove _nofail callers as much as possible, |
| 2) deal with error case in last remaining caller, f2fs_get_sum_page(). |
| |
| Reported-by: syzbot+ee250ac8137be41d7b13@syzkaller.appspotmail.com |
| Reviewed-by: Chao Yu <yuchao0@huawei.com> |
| Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| fs/f2fs/checkpoint.c | 2 +- |
| fs/f2fs/f2fs.h | 2 +- |
| fs/f2fs/node.c | 2 +- |
| fs/f2fs/segment.c | 12 +++++++++--- |
| 4 files changed, 12 insertions(+), 6 deletions(-) |
| |
| diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c |
| index 0b7aec059f112..4a97fe4ddf789 100644 |
| --- a/fs/f2fs/checkpoint.c |
| +++ b/fs/f2fs/checkpoint.c |
| @@ -107,7 +107,7 @@ struct page *f2fs_get_meta_page(struct f2fs_sb_info *sbi, pgoff_t index) |
| return __get_meta_page(sbi, index, true); |
| } |
| |
| -struct page *f2fs_get_meta_page_nofail(struct f2fs_sb_info *sbi, pgoff_t index) |
| +struct page *f2fs_get_meta_page_retry(struct f2fs_sb_info *sbi, pgoff_t index) |
| { |
| struct page *page; |
| int count = 0; |
| diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h |
| index 98c4b166f192b..d44c6c36de678 100644 |
| --- a/fs/f2fs/f2fs.h |
| +++ b/fs/f2fs/f2fs.h |
| @@ -3385,7 +3385,7 @@ enum rw_hint f2fs_io_type_to_rw_hint(struct f2fs_sb_info *sbi, |
| void f2fs_stop_checkpoint(struct f2fs_sb_info *sbi, bool end_io); |
| struct page *f2fs_grab_meta_page(struct f2fs_sb_info *sbi, pgoff_t index); |
| struct page *f2fs_get_meta_page(struct f2fs_sb_info *sbi, pgoff_t index); |
| -struct page *f2fs_get_meta_page_nofail(struct f2fs_sb_info *sbi, pgoff_t index); |
| +struct page *f2fs_get_meta_page_retry(struct f2fs_sb_info *sbi, pgoff_t index); |
| struct page *f2fs_get_tmp_page(struct f2fs_sb_info *sbi, pgoff_t index); |
| bool f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi, |
| block_t blkaddr, int type); |
| diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c |
| index cb1b5b61a1dab..cc4700f6240db 100644 |
| --- a/fs/f2fs/node.c |
| +++ b/fs/f2fs/node.c |
| @@ -109,7 +109,7 @@ static void clear_node_page_dirty(struct page *page) |
| |
| static struct page *get_current_nat_page(struct f2fs_sb_info *sbi, nid_t nid) |
| { |
| - return f2fs_get_meta_page_nofail(sbi, current_nat_addr(sbi, nid)); |
| + return f2fs_get_meta_page(sbi, current_nat_addr(sbi, nid)); |
| } |
| |
| static struct page *get_next_nat_page(struct f2fs_sb_info *sbi, nid_t nid) |
| diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c |
| index e247a5ef3713f..2628406f43f64 100644 |
| --- a/fs/f2fs/segment.c |
| +++ b/fs/f2fs/segment.c |
| @@ -2344,7 +2344,9 @@ int f2fs_npages_for_summary_flush(struct f2fs_sb_info *sbi, bool for_ra) |
| */ |
| struct page *f2fs_get_sum_page(struct f2fs_sb_info *sbi, unsigned int segno) |
| { |
| - return f2fs_get_meta_page_nofail(sbi, GET_SUM_BLOCK(sbi, segno)); |
| + if (unlikely(f2fs_cp_error(sbi))) |
| + return ERR_PTR(-EIO); |
| + return f2fs_get_meta_page_retry(sbi, GET_SUM_BLOCK(sbi, segno)); |
| } |
| |
| void f2fs_update_meta_page(struct f2fs_sb_info *sbi, |
| @@ -2616,7 +2618,11 @@ static void change_curseg(struct f2fs_sb_info *sbi, int type) |
| __next_free_blkoff(sbi, curseg, 0); |
| |
| sum_page = f2fs_get_sum_page(sbi, new_segno); |
| - f2fs_bug_on(sbi, IS_ERR(sum_page)); |
| + if (IS_ERR(sum_page)) { |
| + /* GC won't be able to use stale summary pages by cp_error */ |
| + memset(curseg->sum_blk, 0, SUM_ENTRY_SIZE); |
| + return; |
| + } |
| sum_node = (struct f2fs_summary_block *)page_address(sum_page); |
| memcpy(curseg->sum_blk, sum_node, SUM_ENTRY_SIZE); |
| f2fs_put_page(sum_page, 1); |
| @@ -3781,7 +3787,7 @@ int f2fs_lookup_journal_in_cursum(struct f2fs_journal *journal, int type, |
| static struct page *get_current_sit_page(struct f2fs_sb_info *sbi, |
| unsigned int segno) |
| { |
| - return f2fs_get_meta_page_nofail(sbi, current_sit_addr(sbi, segno)); |
| + return f2fs_get_meta_page(sbi, current_sit_addr(sbi, segno)); |
| } |
| |
| static struct page *get_next_sit_page(struct f2fs_sb_info *sbi, |
| -- |
| 2.27.0 |
| |