Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 684f1b83 authored by Chao Yu's avatar Chao Yu Committed by Jaegeuk Kim
Browse files

f2fs: introduce DATA_GENERIC_ENHANCE 

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: default avatarChao Yu <yuchao0@huawei.com>
Signed-off-by: default avatarJaegeuk Kim <jaegeuk@kernel.org>
parent 65a1ebeb

fs/f2fs/checkpoint.c

+37 −6
Original line number Diff line number Diff line
@@ -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);

			set_sbi_flag(sbi, SBI_NEED_FSCK);

			WARN_ON(1);

			}

			return false;

		} else {

			return __is_bitmap_valid(sbi, blkaddr, type);

		}

		break;

	case META_GENERIC:

fs/f2fs/data.c

+32 −17
Original line number Diff line number Diff line
@@ -453,8 +453,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);
@@ -504,9 +504,7 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio) 
		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;
@@ -563,9 +561,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);
@@ -593,8 +588,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);
@@ -606,8 +603,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;

}
@@ -735,6 +732,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;

	}
@@ -748,6 +750,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);
@@ -1090,12 +1099,12 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, 
	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) {
@@ -1560,7 +1569,7 @@ static int f2fs_read_single_page(struct inode *inode, struct page *page, 
		}



		if (!f2fs_is_valid_blkaddr(F2FS_I_SB(inode), block_nr,

							DATA_GENERIC)) {

						DATA_GENERIC_ENHANCE_READ)) {

			ret = -EFAULT;

			goto out;

		}
@@ -1841,7 +1850,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;
@@ -1868,7 +1877,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;

	}
@@ -1876,7 +1885,8 @@ int f2fs_do_write_data_page(struct f2fs_io_info *fio) 
	 * 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)
@@ -2521,6 +2531,11 @@ static int f2fs_write_begin(struct file *file, struct address_space *mapping, 
		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;

fs/f2fs/f2fs.h

+8 −10
Original line number Diff line number Diff line
@@ -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)

{

fs/f2fs/file.c

+12 −3
Original line number Diff line number Diff line
@@ -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 @@ static int __read_out_blkaddrs(struct inode *inode, block_t *blkaddr, 
	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)) {

fs/f2fs/gc.c

+10 −1
Original line number Diff line number Diff line
@@ -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;

	}