f2fs: fix to propagate return value of scan_nat_page()
authorChao Yu <yuchao0@huawei.com>
Fri, 15 Jun 2018 06:45:57 +0000 (14:45 +0800)
committerJaegeuk Kim <jaegeuk@kernel.org>
Fri, 27 Jul 2018 09:03:59 +0000 (18:03 +0900)
As Anatoly Trosinenko reported in bugzilla:

How to reproduce:
1. Compile the 73fcb1a370c76 version of the kernel using the config attached
2. Unpack and mount the attached filesystem image as F2FS
3. The kernel will BUG() on mount (BUGs are explicitly enabled in config)

[    2.233612] F2FS-fs (sda): Found nat_bits in checkpoint
[    2.248422] ------------[ cut here ]------------
[    2.248857] kernel BUG at fs/f2fs/node.c:1967!
[    2.249760] invalid opcode: 0000 [#1] SMP NOPTI
[    2.250219] Modules linked in:
[    2.251848] CPU: 0 PID: 944 Comm: mount Not tainted 4.17.0-rc5+ #1
[    2.252331] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1ubuntu1 04/01/2014
[    2.253305] RIP: 0010:build_free_nids+0x337/0x3f0
[    2.253672] RSP: 0018:ffffae7fc0857c50 EFLAGS: 00000246
[    2.254080] RAX: 00000000ffffffff RBX: 0000000000000123 RCX: 0000000000000001
[    2.254638] RDX: ffff9aa7063d5c00 RSI: 0000000000000122 RDI: ffff9aa705852e00
[    2.255190] RBP: ffff9aa705852e00 R08: 0000000000000001 R09: ffff9aa7059090c0
[    2.255719] R10: 0000000000000000 R11: 0000000000000000 R12: ffff9aa705852e00
[    2.256242] R13: ffff9aa7063ad000 R14: ffff9aa705919000 R15: 0000000000000123
[    2.256809] FS:  00000000023078c0(0000) GS:ffff9aa707800000(0000) knlGS:0000000000000000
[    2.258654] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    2.259153] CR2: 00000000005511ae CR3: 0000000005872000 CR4: 00000000000006f0
[    2.259801] Call Trace:
[    2.260583]  build_node_manager+0x5cd/0x600
[    2.260963]  f2fs_fill_super+0x66a/0x17c0
[    2.261300]  ? f2fs_commit_super+0xe0/0xe0
[    2.261622]  mount_bdev+0x16e/0x1a0
[    2.261899]  mount_fs+0x30/0x150
[    2.262398]  vfs_kern_mount.part.28+0x4f/0xf0
[    2.262743]  do_mount+0x5d0/0xc60
[    2.263010]  ? _copy_from_user+0x37/0x60
[    2.263313]  ? memdup_user+0x39/0x60
[    2.263692]  ksys_mount+0x7b/0xd0
[    2.263960]  __x64_sys_mount+0x1c/0x20
[    2.264268]  do_syscall_64+0x43/0xf0
[    2.264560]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[    2.265095] RIP: 0033:0x48d31a
[    2.265502] RSP: 002b:00007ffc6fe60a08 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
[    2.266089] RAX: ffffffffffffffda RBX: 0000000000008000 RCX: 000000000048d31a
[    2.266607] RDX: 00007ffc6fe62fa5 RSI: 00007ffc6fe62f9d RDI: 00007ffc6fe62f94
[    2.267130] RBP: 00000000023078a0 R08: 0000000000000000 R09: 0000000000000000
[    2.267670] R10: 0000000000008000 R11: 0000000000000246 R12: 0000000000000000
[    2.268192] R13: 0000000000000000 R14: 00007ffc6fe60c78 R15: 0000000000000000
[    2.268767] Code: e8 5f c3 ff ff 83 c3 01 41 83 c7 01 81 fb c7 01 00 00 74 48 44 39 7d 04 76 42 48 63 c3 48 8d 04 c0 41 8b 44 06 05 83 f8 ff 75 c1 <0f> 0b 49 8b 45 50 48 8d b8 b0 00 00 00 e8 37 59 69 00 b9 01 00
[    2.270434] RIP: build_free_nids+0x337/0x3f0 RSP: ffffae7fc0857c50
[    2.271426] ---[ end trace ab20c06cd3c8fde4 ]---

During loading NAT entries, we will do sanity check, once the entry info
is corrupted, it will cause BUG_ON directly to protect user data from
being overwrited.

In this case, it will be better to just return failure on mount() instead
of panic, so that user can get hint from kmsg and try fsck for recovery
immediately rather than after an abnormal reboot.

https://bugzilla.kernel.org/show_bug.cgi?id=199769

Reported-by: Anatoly Trosinenko <anatoly.trosinenko@gmail.com>
Signed-off-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
fs/f2fs/f2fs.h
fs/f2fs/node.c

index fe80eb6..0acf888 100644 (file)
@@ -2813,7 +2813,7 @@ int f2fs_fsync_node_pages(struct f2fs_sb_info *sbi, struct inode *inode,
 int f2fs_sync_node_pages(struct f2fs_sb_info *sbi,
                        struct writeback_control *wbc,
                        bool do_balance, enum iostat_type io_type);
-void f2fs_build_free_nids(struct f2fs_sb_info *sbi, bool sync, bool mount);
+int f2fs_build_free_nids(struct f2fs_sb_info *sbi, bool sync, bool mount);
 bool f2fs_alloc_nid(struct f2fs_sb_info *sbi, nid_t *nid);
 void f2fs_alloc_nid_done(struct f2fs_sb_info *sbi, nid_t nid);
 void f2fs_alloc_nid_failed(struct f2fs_sb_info *sbi, nid_t nid);
index 3d12409..1d590c6 100644 (file)
@@ -1977,7 +1977,7 @@ static void remove_free_nid(struct f2fs_sb_info *sbi, nid_t nid)
                kmem_cache_free(free_nid_slab, i);
 }
 
-static void scan_nat_page(struct f2fs_sb_info *sbi,
+static int scan_nat_page(struct f2fs_sb_info *sbi,
                        struct page *nat_page, nid_t start_nid)
 {
        struct f2fs_nm_info *nm_i = NM_I(sbi);
@@ -1995,7 +1995,10 @@ static void scan_nat_page(struct f2fs_sb_info *sbi,
                        break;
 
                blk_addr = le32_to_cpu(nat_blk->entries[i].block_addr);
-               f2fs_bug_on(sbi, blk_addr == NEW_ADDR);
+
+               if (blk_addr == NEW_ADDR)
+                       return -EINVAL;
+
                if (blk_addr == NULL_ADDR) {
                        add_free_nid(sbi, start_nid, true, true);
                } else {
@@ -2004,6 +2007,8 @@ static void scan_nat_page(struct f2fs_sb_info *sbi,
                        spin_unlock(&NM_I(sbi)->nid_list_lock);
                }
        }
+
+       return 0;
 }
 
 static void scan_curseg_cache(struct f2fs_sb_info *sbi)
@@ -2059,11 +2064,11 @@ out:
        up_read(&nm_i->nat_tree_lock);
 }
 
-static void __f2fs_build_free_nids(struct f2fs_sb_info *sbi,
+static int __f2fs_build_free_nids(struct f2fs_sb_info *sbi,
                                                bool sync, bool mount)
 {
        struct f2fs_nm_info *nm_i = NM_I(sbi);
-       int i = 0;
+       int i = 0, ret;
        nid_t nid = nm_i->next_scan_nid;
 
        if (unlikely(nid >= nm_i->max_nid))
@@ -2071,17 +2076,17 @@ static void __f2fs_build_free_nids(struct f2fs_sb_info *sbi,
 
        /* Enough entries */
        if (nm_i->nid_cnt[FREE_NID] >= NAT_ENTRY_PER_BLOCK)
-               return;
+               return 0;
 
        if (!sync && !f2fs_available_free_memory(sbi, FREE_NIDS))
-               return;
+               return 0;
 
        if (!mount) {
                /* try to find free nids in free_nid_bitmap */
                scan_free_nid_bits(sbi);
 
                if (nm_i->nid_cnt[FREE_NID] >= NAT_ENTRY_PER_BLOCK)
-                       return;
+                       return 0;
        }
 
        /* readahead nat pages to be scanned */
@@ -2095,8 +2100,16 @@ static void __f2fs_build_free_nids(struct f2fs_sb_info *sbi,
                                                nm_i->nat_block_bitmap)) {
                        struct page *page = get_current_nat_page(sbi, nid);
 
-                       scan_nat_page(sbi, page, nid);
+                       ret = scan_nat_page(sbi, page, nid);
                        f2fs_put_page(page, 1);
+
+                       if (ret) {
+                               up_read(&nm_i->nat_tree_lock);
+                               f2fs_bug_on(sbi, !mount);
+                               f2fs_msg(sbi->sb, KERN_ERR,
+                                       "NAT is corrupt, run fsck to fix it");
+                               return -EINVAL;
+                       }
                }
 
                nid += (NAT_ENTRY_PER_BLOCK - (nid % NAT_ENTRY_PER_BLOCK));
@@ -2117,13 +2130,19 @@ static void __f2fs_build_free_nids(struct f2fs_sb_info *sbi,
 
        f2fs_ra_meta_pages(sbi, NAT_BLOCK_OFFSET(nm_i->next_scan_nid),
                                        nm_i->ra_nid_pages, META_NAT, false);
+
+       return 0;
 }
 
-void f2fs_build_free_nids(struct f2fs_sb_info *sbi, bool sync, bool mount)
+int f2fs_build_free_nids(struct f2fs_sb_info *sbi, bool sync, bool mount)
 {
+       int ret;
+
        mutex_lock(&NM_I(sbi)->build_lock);
-       __f2fs_build_free_nids(sbi, sync, mount);
+       ret = __f2fs_build_free_nids(sbi, sync, mount);
        mutex_unlock(&NM_I(sbi)->build_lock);
+
+       return ret;
 }
 
 /*
@@ -2817,8 +2836,7 @@ int f2fs_build_node_manager(struct f2fs_sb_info *sbi)
        /* load free nid status from nat_bits table */
        load_free_nid_bitmap(sbi);
 
-       f2fs_build_free_nids(sbi, true, true);
-       return 0;
+       return f2fs_build_free_nids(sbi, true, true);
 }
 
 void f2fs_destroy_node_manager(struct f2fs_sb_info *sbi)