f2fs: handle errors of f2fs_get_meta_page_nofail

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>
This commit is contained in:
Jaegeuk Kim 2020-10-02 14:17:35 -07:00
parent d662fad143
commit 86f33603f8
4 changed files with 12 additions and 6 deletions

View File

@ -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); 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; struct page *page;
int count = 0; int count = 0;

View File

@ -3422,7 +3422,7 @@ unsigned int f2fs_usable_blks_in_seg(struct f2fs_sb_info *sbi,
void f2fs_stop_checkpoint(struct f2fs_sb_info *sbi, bool end_io); 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_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(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); struct page *f2fs_get_tmp_page(struct f2fs_sb_info *sbi, pgoff_t index);
bool f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi, bool f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi,
block_t blkaddr, int type); block_t blkaddr, int type);

View File

@ -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) 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) static struct page *get_next_nat_page(struct f2fs_sb_info *sbi, nid_t nid)

View File

@ -2379,7 +2379,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) 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, void f2fs_update_meta_page(struct f2fs_sb_info *sbi,
@ -2669,7 +2671,11 @@ static void change_curseg(struct f2fs_sb_info *sbi, int type, bool flush)
__next_free_blkoff(sbi, curseg, 0); __next_free_blkoff(sbi, curseg, 0);
sum_page = f2fs_get_sum_page(sbi, new_segno); 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); sum_node = (struct f2fs_summary_block *)page_address(sum_page);
memcpy(curseg->sum_blk, sum_node, SUM_ENTRY_SIZE); memcpy(curseg->sum_blk, sum_node, SUM_ENTRY_SIZE);
f2fs_put_page(sum_page, 1); f2fs_put_page(sum_page, 1);
@ -3964,7 +3970,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, static struct page *get_current_sit_page(struct f2fs_sb_info *sbi,
unsigned int segno) 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, static struct page *get_next_sit_page(struct f2fs_sb_info *sbi,