f2fs: introduce DATA_GENERIC_ENHANCE
authorChao Yu <yuchao0@huawei.com>
Mon, 15 Apr 2019 07:26:32 +0000 (15:26 +0800)
committerJaegeuk Kim <jaegeuk@kernel.org>
Thu, 9 May 2019 04:23:13 +0000 (21:23 -0700)
Previously, f2fs_is_valid_blkaddr(, blkaddr, DATA_GENERIC) will check
whether @blkaddr locates in main area or not.

That check is weak, since the block address in range of main area can
point to the address which is not valid in segment info table, and we
can not detect such condition, we may suffer worse corruption as system
continues running.

So this patch introduce DATA_GENERIC_ENHANCE to enhance the sanity check
which trigger SIT bitmap check rather than only range check.

This patch did below changes as wel:
- set SBI_NEED_FSCK in f2fs_is_valid_blkaddr().
- get rid of is_valid_data_blkaddr() to avoid panic if blkaddr is invalid.
- introduce verify_fio_blkaddr() to wrap fio {new,old}_blkaddr validation check.
- spread blkaddr check in:
 * f2fs_get_node_info()
 * __read_out_blkaddrs()
 * f2fs_submit_page_read()
 * ra_data_block()
 * do_recover_data()

This patch can fix bug reported from bugzilla below:

https://bugzilla.kernel.org/show_bug.cgi?id=203215
https://bugzilla.kernel.org/show_bug.cgi?id=203223
https://bugzilla.kernel.org/show_bug.cgi?id=203231
https://bugzilla.kernel.org/show_bug.cgi?id=203235
https://bugzilla.kernel.org/show_bug.cgi?id=203241

= Update by Jaegeuk Kim =

DATA_GENERIC_ENHANCE enhanced to validate block addresses on read/write paths.
But, xfstest/generic/446 compalins some generated kernel messages saying invalid
bitmap was detected when reading a block. The reaons is, when we get the
block addresses from extent_cache, there is no lock to synchronize it from
truncating the blocks in parallel.

Signed-off-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
fs/f2fs/checkpoint.c
fs/f2fs/data.c
fs/f2fs/f2fs.h
fs/f2fs/file.c
fs/f2fs/gc.c
fs/f2fs/inode.c
fs/f2fs/node.c
fs/f2fs/recovery.c
fs/f2fs/segment.c
fs/f2fs/segment.h

index b8614cf77cdde47b6f010b67f0bfd794e6caae0e..805a33088e82dd4161b65285674d7bbe43a4bb28 100644 (file)
@@ -130,6 +130,30 @@ struct page *f2fs_get_tmp_page(struct f2fs_sb_info *sbi, pgoff_t index)
        return __get_meta_page(sbi, index, false);
 }
 
+static bool __is_bitmap_valid(struct f2fs_sb_info *sbi, block_t blkaddr,
+                                                       int type)
+{
+       struct seg_entry *se;
+       unsigned int segno, offset;
+       bool exist;
+
+       if (type != DATA_GENERIC_ENHANCE && type != DATA_GENERIC_ENHANCE_READ)
+               return true;
+
+       segno = GET_SEGNO(sbi, blkaddr);
+       offset = GET_BLKOFF_FROM_SEG0(sbi, blkaddr);
+       se = get_seg_entry(sbi, segno);
+
+       exist = f2fs_test_bit(offset, se->cur_valid_map);
+       if (!exist && type == DATA_GENERIC_ENHANCE) {
+               f2fs_msg(sbi->sb, KERN_ERR, "Inconsistent error "
+                       "blkaddr:%u, sit bitmap:%d", blkaddr, exist);
+               set_sbi_flag(sbi, SBI_NEED_FSCK);
+               WARN_ON(1);
+       }
+       return exist;
+}
+
 bool f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi,
                                        block_t blkaddr, int type)
 {
@@ -151,15 +175,22 @@ bool f2fs_is_valid_blkaddr(struct f2fs_sb_info *sbi,
                        return false;
                break;
        case META_POR:
+               if (unlikely(blkaddr >= MAX_BLKADDR(sbi) ||
+                       blkaddr < MAIN_BLKADDR(sbi)))
+                       return false;
+               break;
        case DATA_GENERIC:
+       case DATA_GENERIC_ENHANCE:
+       case DATA_GENERIC_ENHANCE_READ:
                if (unlikely(blkaddr >= MAX_BLKADDR(sbi) ||
-                       blkaddr < MAIN_BLKADDR(sbi))) {
-                       if (type == DATA_GENERIC) {
-                               f2fs_msg(sbi->sb, KERN_WARNING,
-                                       "access invalid blkaddr:%u", blkaddr);
-                               WARN_ON(1);
-                       }
+                               blkaddr < MAIN_BLKADDR(sbi))) {
+                       f2fs_msg(sbi->sb, KERN_WARNING,
+                               "access invalid blkaddr:%u", blkaddr);
+                       set_sbi_flag(sbi, SBI_NEED_FSCK);
+                       WARN_ON(1);
                        return false;
+               } else {
+                       return __is_bitmap_valid(sbi, blkaddr, type);
                }
                break;
        case META_GENERIC:
index cf89e36577bfac1d50b243b3ac7262eb5eecba55..d32a82f25f5a488b441b6a40128951af93821577 100644 (file)
@@ -456,8 +456,8 @@ int f2fs_submit_page_bio(struct f2fs_io_info *fio)
                        fio->encrypted_page : fio->page;
 
        if (!f2fs_is_valid_blkaddr(fio->sbi, fio->new_blkaddr,
-                       fio->is_por ? META_POR :
-                       (__is_meta_io(fio) ? META_GENERIC : DATA_GENERIC)))
+                       fio->is_por ? META_POR : (__is_meta_io(fio) ?
+                       META_GENERIC : DATA_GENERIC_ENHANCE)))
                return -EFAULT;
 
        trace_f2fs_submit_page_bio(page, fio);
@@ -507,9 +507,7 @@ next:
                spin_unlock(&io->io_lock);
        }
 
-       if (__is_valid_data_blkaddr(fio->old_blkaddr))
-               verify_block_addr(fio, fio->old_blkaddr);
-       verify_block_addr(fio, fio->new_blkaddr);
+       verify_fio_blkaddr(fio);
 
        bio_page = fio->encrypted_page ? fio->encrypted_page : fio->page;
 
@@ -566,9 +564,6 @@ static struct bio *f2fs_grab_read_bio(struct inode *inode, block_t blkaddr,
        struct bio_post_read_ctx *ctx;
        unsigned int post_read_steps = 0;
 
-       if (!f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC))
-               return ERR_PTR(-EFAULT);
-
        bio = f2fs_bio_alloc(sbi, min_t(int, nr_pages, BIO_MAX_PAGES), false);
        if (!bio)
                return ERR_PTR(-ENOMEM);
@@ -596,8 +591,10 @@ static struct bio *f2fs_grab_read_bio(struct inode *inode, block_t blkaddr,
 static int f2fs_submit_page_read(struct inode *inode, struct page *page,
                                                        block_t blkaddr)
 {
-       struct bio *bio = f2fs_grab_read_bio(inode, blkaddr, 1, 0);
+       struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
+       struct bio *bio;
 
+       bio = f2fs_grab_read_bio(inode, blkaddr, 1, 0);
        if (IS_ERR(bio))
                return PTR_ERR(bio);
 
@@ -609,8 +606,8 @@ static int f2fs_submit_page_read(struct inode *inode, struct page *page,
                return -EFAULT;
        }
        ClearPageError(page);
-       inc_page_count(F2FS_I_SB(inode), F2FS_RD_DATA);
-       __submit_bio(F2FS_I_SB(inode), bio, DATA);
+       inc_page_count(sbi, F2FS_RD_DATA);
+       __submit_bio(sbi, bio, DATA);
        return 0;
 }
 
@@ -738,6 +735,11 @@ struct page *f2fs_get_read_data_page(struct inode *inode, pgoff_t index,
 
        if (f2fs_lookup_extent_cache(inode, index, &ei)) {
                dn.data_blkaddr = ei.blk + index - ei.fofs;
+               if (!f2fs_is_valid_blkaddr(F2FS_I_SB(inode), dn.data_blkaddr,
+                                               DATA_GENERIC_ENHANCE_READ)) {
+                       err = -EFAULT;
+                       goto put_err;
+               }
                goto got_it;
        }
 
@@ -751,6 +753,13 @@ struct page *f2fs_get_read_data_page(struct inode *inode, pgoff_t index,
                err = -ENOENT;
                goto put_err;
        }
+       if (dn.data_blkaddr != NEW_ADDR &&
+                       !f2fs_is_valid_blkaddr(F2FS_I_SB(inode),
+                                               dn.data_blkaddr,
+                                               DATA_GENERIC_ENHANCE)) {
+               err = -EFAULT;
+               goto put_err;
+       }
 got_it:
        if (PageUptodate(page)) {
                unlock_page(page);
@@ -1093,12 +1102,12 @@ next_block:
        blkaddr = datablock_addr(dn.inode, dn.node_page, dn.ofs_in_node);
 
        if (__is_valid_data_blkaddr(blkaddr) &&
-               !f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC)) {
+               !f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE)) {
                err = -EFAULT;
                goto sync_out;
        }
 
-       if (is_valid_data_blkaddr(sbi, blkaddr)) {
+       if (__is_valid_data_blkaddr(blkaddr)) {
                /* use out-place-update for driect IO under LFS mode */
                if (test_opt(sbi, LFS) && flag == F2FS_GET_BLOCK_DIO &&
                                                        map->m_may_create) {
@@ -1563,7 +1572,7 @@ got_it:
                }
 
                if (!f2fs_is_valid_blkaddr(F2FS_I_SB(inode), block_nr,
-                                                       DATA_GENERIC)) {
+                                               DATA_GENERIC_ENHANCE_READ)) {
                        ret = -EFAULT;
                        goto out;
                }
@@ -1844,7 +1853,7 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
                fio->old_blkaddr = ei.blk + page->index - ei.fofs;
 
                if (!f2fs_is_valid_blkaddr(fio->sbi, fio->old_blkaddr,
-                                                       DATA_GENERIC))
+                                               DATA_GENERIC_ENHANCE))
                        return -EFAULT;
 
                ipu_force = true;
@@ -1871,7 +1880,7 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio)
 got_it:
        if (__is_valid_data_blkaddr(fio->old_blkaddr) &&
                !f2fs_is_valid_blkaddr(fio->sbi, fio->old_blkaddr,
-                                                       DATA_GENERIC)) {
+                                               DATA_GENERIC_ENHANCE)) {
                err = -EFAULT;
                goto out_writepage;
        }
@@ -1879,7 +1888,8 @@ got_it:
         * If current allocation needs SSR,
         * it had better in-place writes for updated data.
         */
-       if (ipu_force || (is_valid_data_blkaddr(fio->sbi, fio->old_blkaddr) &&
+       if (ipu_force ||
+               (__is_valid_data_blkaddr(fio->old_blkaddr) &&
                                        need_inplace_update(fio))) {
                err = encrypt_one_page(fio);
                if (err)
@@ -2524,6 +2534,11 @@ repeat:
                zero_user_segment(page, 0, PAGE_SIZE);
                SetPageUptodate(page);
        } else {
+               if (!f2fs_is_valid_blkaddr(sbi, blkaddr,
+                               DATA_GENERIC_ENHANCE_READ)) {
+                       err = -EFAULT;
+                       goto fail;
+               }
                err = f2fs_submit_page_read(inode, page, blkaddr);
                if (err)
                        goto fail;
index 1c814a309748be41065294827a646ff02a508bfb..533fafca68f436300d4bab3f42c90f36cd658986 100644 (file)
@@ -210,7 +210,14 @@ enum {
        META_SSA,
        META_MAX,
        META_POR,
-       DATA_GENERIC,
+       DATA_GENERIC,           /* check range only */
+       DATA_GENERIC_ENHANCE,   /* strong check on range and segment bitmap */
+       DATA_GENERIC_ENHANCE_READ,      /*
+                                        * strong check on range and segment
+                                        * bitmap but no warning due to race
+                                        * condition of read on truncated area
+                                        * by extent_cache
+                                        */
        META_GENERIC,
 };
 
@@ -2864,15 +2871,6 @@ static inline bool __is_valid_data_blkaddr(block_t blkaddr)
        return true;
 }
 
-static inline bool is_valid_data_blkaddr(struct f2fs_sb_info *sbi,
-                                               block_t blkaddr)
-{
-       if (!__is_valid_data_blkaddr(blkaddr))
-               return false;
-       verify_blkaddr(sbi, blkaddr, DATA_GENERIC);
-       return true;
-}
-
 static inline void f2fs_set_page_private(struct page *page,
                                                unsigned long data)
 {
index e5fa1940ed3ca2c0fbecb7b0ab1cf74a448ce887..3baa39e3e1fdced036f17c4b612b01d1e025a5c3 100644 (file)
@@ -356,7 +356,7 @@ static bool __found_offset(struct f2fs_sb_info *sbi, block_t blkaddr,
        switch (whence) {
        case SEEK_DATA:
                if ((blkaddr == NEW_ADDR && dirty == pgofs) ||
-                       is_valid_data_blkaddr(sbi, blkaddr))
+                       __is_valid_data_blkaddr(blkaddr))
                        return true;
                break;
        case SEEK_HOLE:
@@ -422,7 +422,7 @@ static loff_t f2fs_seek_block(struct file *file, loff_t offset, int whence)
 
                        if (__is_valid_data_blkaddr(blkaddr) &&
                                !f2fs_is_valid_blkaddr(F2FS_I_SB(inode),
-                                               blkaddr, DATA_GENERIC)) {
+                                       blkaddr, DATA_GENERIC_ENHANCE)) {
                                f2fs_put_dnode(&dn);
                                goto fail;
                        }
@@ -523,7 +523,8 @@ void f2fs_truncate_data_blocks_range(struct dnode_of_data *dn, int count)
                f2fs_set_data_blkaddr(dn);
 
                if (__is_valid_data_blkaddr(blkaddr) &&
-                       !f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC))
+                       !f2fs_is_valid_blkaddr(sbi, blkaddr,
+                                       DATA_GENERIC_ENHANCE))
                        continue;
 
                f2fs_invalidate_blocks(sbi, blkaddr);
@@ -1018,6 +1019,14 @@ next_dnode:
        for (i = 0; i < done; i++, blkaddr++, do_replace++, dn.ofs_in_node++) {
                *blkaddr = datablock_addr(dn.inode,
                                        dn.node_page, dn.ofs_in_node);
+
+               if (__is_valid_data_blkaddr(*blkaddr) &&
+                       !f2fs_is_valid_blkaddr(sbi, *blkaddr,
+                                       DATA_GENERIC_ENHANCE)) {
+                       f2fs_put_dnode(&dn);
+                       return -EFAULT;
+               }
+
                if (!f2fs_is_checkpointed_data(sbi, *blkaddr)) {
 
                        if (test_opt(sbi, LFS)) {
index ba30ec90952fd7b21050b63940e99bed9e47b9f2..963fb4571fd984a75e0f32fba612c416a4330db8 100644 (file)
@@ -656,6 +656,11 @@ static int ra_data_block(struct inode *inode, pgoff_t index)
 
        if (f2fs_lookup_extent_cache(inode, index, &ei)) {
                dn.data_blkaddr = ei.blk + index - ei.fofs;
+               if (unlikely(!f2fs_is_valid_blkaddr(sbi, dn.data_blkaddr,
+                                               DATA_GENERIC_ENHANCE_READ))) {
+                       err = -EFAULT;
+                       goto put_page;
+               }
                goto got_it;
        }
 
@@ -665,8 +670,12 @@ static int ra_data_block(struct inode *inode, pgoff_t index)
                goto put_page;
        f2fs_put_dnode(&dn);
 
+       if (!__is_valid_data_blkaddr(dn.data_blkaddr)) {
+               err = -ENOENT;
+               goto put_page;
+       }
        if (unlikely(!f2fs_is_valid_blkaddr(sbi, dn.data_blkaddr,
-                                               DATA_GENERIC))) {
+                                               DATA_GENERIC_ENHANCE))) {
                err = -EFAULT;
                goto put_page;
        }
index b53952a15ffae6dda51651b77bc5fc8cbc3d68c3..ccb02226dd2c0c28722ecafd61ce657975e441cd 100644 (file)
@@ -73,7 +73,7 @@ static int __written_first_block(struct f2fs_sb_info *sbi,
 
        if (!__is_valid_data_blkaddr(addr))
                return 1;
-       if (!f2fs_is_valid_blkaddr(sbi, addr, DATA_GENERIC))
+       if (!f2fs_is_valid_blkaddr(sbi, addr, DATA_GENERIC_ENHANCE))
                return -EFAULT;
        return 0;
 }
@@ -267,9 +267,10 @@ static bool sanity_check_inode(struct inode *inode, struct page *node_page)
                struct extent_info *ei = &F2FS_I(inode)->extent_tree->largest;
 
                if (ei->len &&
-                       (!f2fs_is_valid_blkaddr(sbi, ei->blk, DATA_GENERIC) ||
+                       (!f2fs_is_valid_blkaddr(sbi, ei->blk,
+                                               DATA_GENERIC_ENHANCE) ||
                        !f2fs_is_valid_blkaddr(sbi, ei->blk + ei->len - 1,
-                                                       DATA_GENERIC))) {
+                                               DATA_GENERIC_ENHANCE))) {
                        set_sbi_flag(sbi, SBI_NEED_FSCK);
                        f2fs_msg(sbi->sb, KERN_WARNING,
                                "%s: inode (ino=%lx) extent info [%u, %u, %u] "
index 057362a821a091e52c4ed07f281b1ebbccb9c639..a6960b47f39474a61e3ddc900118f6ccea0471a8 100644 (file)
@@ -454,7 +454,7 @@ static void set_node_addr(struct f2fs_sb_info *sbi, struct node_info *ni,
                        new_blkaddr == NULL_ADDR);
        f2fs_bug_on(sbi, nat_get_blkaddr(e) == NEW_ADDR &&
                        new_blkaddr == NEW_ADDR);
-       f2fs_bug_on(sbi, is_valid_data_blkaddr(sbi, nat_get_blkaddr(e)) &&
+       f2fs_bug_on(sbi, __is_valid_data_blkaddr(nat_get_blkaddr(e)) &&
                        new_blkaddr == NEW_ADDR);
 
        /* increment version no as node is removed */
@@ -465,7 +465,7 @@ static void set_node_addr(struct f2fs_sb_info *sbi, struct node_info *ni,
 
        /* change address */
        nat_set_blkaddr(e, new_blkaddr);
-       if (!is_valid_data_blkaddr(sbi, new_blkaddr))
+       if (!__is_valid_data_blkaddr(new_blkaddr))
                set_nat_flag(e, IS_CHECKPOINTED, false);
        __set_nat_cache_dirty(nm_i, e);
 
@@ -526,6 +526,7 @@ int f2fs_get_node_info(struct f2fs_sb_info *sbi, nid_t nid,
        struct f2fs_nat_entry ne;
        struct nat_entry *e;
        pgoff_t index;
+       block_t blkaddr;
        int i;
 
        ni->nid = nid;
@@ -569,6 +570,11 @@ int f2fs_get_node_info(struct f2fs_sb_info *sbi, nid_t nid,
        node_info_from_raw_nat(ni, &ne);
        f2fs_put_page(page, 1);
 cache:
+       blkaddr = le32_to_cpu(ne.block_addr);
+       if (__is_valid_data_blkaddr(blkaddr) &&
+               !f2fs_is_valid_blkaddr(sbi, blkaddr, DATA_GENERIC_ENHANCE))
+               return -EFAULT;
+
        /* cache nat entry */
        cache_nat_entry(sbi, nid, &ne);
        return 0;
@@ -1548,7 +1554,8 @@ static int __write_node_page(struct page *page, bool atomic, bool *submitted,
        }
 
        if (__is_valid_data_blkaddr(ni.blk_addr) &&
-               !f2fs_is_valid_blkaddr(sbi, ni.blk_addr, DATA_GENERIC)) {
+               !f2fs_is_valid_blkaddr(sbi, ni.blk_addr,
+                                       DATA_GENERIC_ENHANCE)) {
                up_read(&sbi->node_write);
                goto redirty_out;
        }
index b14c718139a9652c024d31e3cc65cf1e75f8b615..e04f82b3f4fc82ec0e0c943fb57970d55ce89f5f 100644 (file)
@@ -567,6 +567,18 @@ retry_dn:
                src = datablock_addr(dn.inode, dn.node_page, dn.ofs_in_node);
                dest = datablock_addr(dn.inode, page, dn.ofs_in_node);
 
+               if (__is_valid_data_blkaddr(src) &&
+                       !f2fs_is_valid_blkaddr(sbi, src, META_POR)) {
+                       err = -EFAULT;
+                       goto err;
+               }
+
+               if (__is_valid_data_blkaddr(dest) &&
+                       !f2fs_is_valid_blkaddr(sbi, dest, META_POR)) {
+                       err = -EFAULT;
+                       goto err;
+               }
+
                /* skip recovering if dest is the same as src */
                if (src == dest)
                        continue;
index d3bf7a2abbc9d4a5bd3f3d073fa119119f4a055a..8388d2abacb5d5616cbd7b2f53e75188fa2a46a6 100644 (file)
@@ -2217,7 +2217,7 @@ bool f2fs_is_checkpointed_data(struct f2fs_sb_info *sbi, block_t blkaddr)
        struct seg_entry *se;
        bool is_cp = false;
 
-       if (!is_valid_data_blkaddr(sbi, blkaddr))
+       if (!__is_valid_data_blkaddr(blkaddr))
                return true;
 
        down_read(&sit_i->sentry_lock);
@@ -3338,7 +3338,7 @@ void f2fs_wait_on_block_writeback(struct inode *inode, block_t blkaddr)
        if (!f2fs_post_read_required(inode))
                return;
 
-       if (!is_valid_data_blkaddr(sbi, blkaddr))
+       if (!__is_valid_data_blkaddr(blkaddr))
                return;
 
        cpage = find_lock_page(META_MAPPING(sbi), blkaddr);
index 6f48e0763279274af5b2bbd48da276a3b44bfed5..429007b8036ebfe6e65af989b2666be4bd76610b 100644 (file)
@@ -82,7 +82,7 @@
        (GET_SEGOFF_FROM_SEG0(sbi, blk_addr) & ((sbi)->blocks_per_seg - 1))
 
 #define GET_SEGNO(sbi, blk_addr)                                       \
-       ((!is_valid_data_blkaddr(sbi, blk_addr)) ?                      \
+       ((!__is_valid_data_blkaddr(blk_addr)) ?                 \
        NULL_SEGNO : GET_L2R_SEGNO(FREE_I(sbi),                 \
                GET_SEGNO_FROM_SEG0(sbi, blk_addr)))
 #define BLKS_PER_SEC(sbi)                                      \
@@ -656,14 +656,15 @@ static inline void check_seg_range(struct f2fs_sb_info *sbi, unsigned int segno)
        f2fs_bug_on(sbi, segno > TOTAL_SEGS(sbi) - 1);
 }
 
-static inline void verify_block_addr(struct f2fs_io_info *fio, block_t blk_addr)
+static inline void verify_fio_blkaddr(struct f2fs_io_info *fio)
 {
        struct f2fs_sb_info *sbi = fio->sbi;
 
-       if (__is_meta_io(fio))
-               verify_blkaddr(sbi, blk_addr, META_GENERIC);
-       else
-               verify_blkaddr(sbi, blk_addr, DATA_GENERIC);
+       if (__is_valid_data_blkaddr(fio->old_blkaddr))
+               verify_blkaddr(sbi, fio->old_blkaddr, __is_meta_io(fio) ?
+                                       META_GENERIC : DATA_GENERIC);
+       verify_blkaddr(sbi, fio->new_blkaddr, __is_meta_io(fio) ?
+                                       META_GENERIC : DATA_GENERIC_ENHANCE);
 }
 
 /*