Re: [f2fs-dev] [PATCH] fsck.f2fs: fix to skip repairing initialized i_gc_failures
On 2018/11/27 11:52, Jaegeuk Kim wrote: > On 11/27, Chao Yu wrote: >> On 2018/11/27 8:04, Jaegeuk Kim wrote: >>> On 11/26, Chao Yu wrote: From: Chao Yu As Michael reported: after updating to f2fs-tools 1.12.0, a routine fsck of my file systems took quite a while and output ten-thousands instances of the following line: > [FIX] (fsck_chk_inode_blk: 954) --> Regular: 0xXYZ reset i_gc_failures > from 0x1 to 0x00 In old kernel, we initialized i_gc_failures as 0x01, let's skip resetting such unchanged initialized value to avoid unnecessary repairing. Reported-by: Michael Laß Signed-off-by: Chao Yu --- fsck/fsck.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/fsck/fsck.c b/fsck/fsck.c index 970d150..db0e72f 100644 --- a/fsck/fsck.c +++ b/fsck/fsck.c @@ -941,7 +941,9 @@ skip_blkcnt_fix: } i_gc_failures = le16_to_cpu(node_blk->i.i_gc_failures); - if (ftype == F2FS_FT_REG_FILE && i_gc_failures) { + + /* old kernel initialized i_gc_failures as 0x01, skip repairing */ + if (ftype == F2FS_FT_REG_FILE && i_gc_failures > 1) { >>> >>> This will break the current implementation. >> >> Yeah, but I doubt that after repairing i_gc_failures, old kernel can still >> continue creating inodes in where .i_gc_failures equals to 0x01, then fsck >> will report such info each time..., can we relief fsck in such condition? > > How about adding another preen mode? > > For example, > - 2: same as 0, but skip some checks for old kernel Good idea, let me change to add this in v2. :) Thanks, > >> >> Thanks, >> >>> DBG(1, "Regular Inode: 0x%x [%s] depth: %d\n\n", le32_to_cpu(node_blk->footer.ino), en, -- 2.18.0 >>> >>> . >>> > > . > ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] fsck.f2fs: fix to skip repairing initialized i_gc_failures
On 11/27, Chao Yu wrote: > On 2018/11/27 8:04, Jaegeuk Kim wrote: > > On 11/26, Chao Yu wrote: > >> From: Chao Yu > >> > >> As Michael reported: > >> > >> after updating to f2fs-tools 1.12.0, a routine fsck of my file systems > >> took quite a while and output ten-thousands instances of the following > >> line: > >> > >>> [FIX] (fsck_chk_inode_blk: 954) --> Regular: 0xXYZ reset i_gc_failures > >>> from 0x1 to 0x00 > >> > >> In old kernel, we initialized i_gc_failures as 0x01, let's skip > >> resetting such unchanged initialized value to avoid unnecessary > >> repairing. > >> > >> Reported-by: Michael Laß > >> Signed-off-by: Chao Yu > >> --- > >> fsck/fsck.c | 4 +++- > >> 1 file changed, 3 insertions(+), 1 deletion(-) > >> > >> diff --git a/fsck/fsck.c b/fsck/fsck.c > >> index 970d150..db0e72f 100644 > >> --- a/fsck/fsck.c > >> +++ b/fsck/fsck.c > >> @@ -941,7 +941,9 @@ skip_blkcnt_fix: > >>} > >> > >>i_gc_failures = le16_to_cpu(node_blk->i.i_gc_failures); > >> - if (ftype == F2FS_FT_REG_FILE && i_gc_failures) { > >> + > >> + /* old kernel initialized i_gc_failures as 0x01, skip repairing */ > >> + if (ftype == F2FS_FT_REG_FILE && i_gc_failures > 1) { > > > > This will break the current implementation. > > Yeah, but I doubt that after repairing i_gc_failures, old kernel can still > continue creating inodes in where .i_gc_failures equals to 0x01, then fsck > will report such info each time..., can we relief fsck in such condition? How about adding another preen mode? For example, - 2: same as 0, but skip some checks for old kernel > > Thanks, > > > > >> > >>DBG(1, "Regular Inode: 0x%x [%s] depth: %d\n\n", > >>le32_to_cpu(node_blk->footer.ino), en, > >> -- > >> 2.18.0 > > > > . > > ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: read page index before freeing
On Tue, Nov 27, 2018 at 11:12:40AM +0800, Chao Yu wrote: > On 2018/11/27 8:22, PanBian wrote: > > On Mon, Nov 26, 2018 at 07:07:08PM +0800, Chao Yu wrote: > >> On 2018/11/26 18:28, PanBian wrote: > >>> On Mon, Nov 26, 2018 at 05:13:53PM +0800, Chao Yu wrote: > Hi Pan, > > On 2018/11/22 18:58, Pan Bian wrote: > > The function truncate_node frees the page with f2fs_put_page. However, > > the page index is read after that. So, the patch reads the index before > > freeing the page. > > I notice that you found another use-after-free bug in ext4, out of > curiosity, I'd like to ask how do you find those bugs? by tool or code > review? > >>> > >>> I found such bugs by the aid of a tool I wrote recently. I designed a > >>> method > >>> to automatically find paired alloc/free functions. With such functions, I > >>> wrote two checkers, one to check mismatched alloc/free bugs, the other to > >>> check use-after-free and double-free bugs. > >> > >> Excellent! Do you have any plan to open its source or announce it w/ binary > >> to linux kernel developers, I think w/ it we can help to improve kernel's > >> code quality efficiently. > > > > Yes. I am now writing a paper about the method. I will open the source code > > as soon as I complete the paper and some optimizations. > > Cool, if there is any progress, please let f2fs guys know, thank you in > advance. :) No problem. It's my honor to apply my tool to the Linux kernel. > > Thanks, > ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v2] f2fs: fix m_may_create to make OPU DIO write correctly
Ping, On 2018/11/20 19:58, Chao Yu wrote: > On 2018-11-20 4:29, Jia Zhu wrote: >> Previously, we added a parameter @map.m_may_create to trigger OPU >> allocation and call f2fs_balance_fs() correctly. >> >> But in get_more_blocks(), @create has been overwritten by below code. >> So the function f2fs_map_blocks() will not allocate new block address >> but directly go out. Meanwile,there are several functions calling >> f2fs_map_blocks() directly and @map.m_may_create not initialized. > > Oh, I missed to check all f2fs_map_blocks structure referrers, sorry. > >> CODE: >> create = dio->op == REQ_OP_WRITE; >> if (dio->flags & DIO_SKIP_HOLES) { >> if (fs_startblk <= ((i_size_read(dio->inode) - 1) >> >> i_blkbits)) >> create = 0; >> } >> >> This patch fixes it. >> >> Signed-off-by: Jia Zhu > --- > > It will be better to add simple change logs here to indicate how you modify > your > patch comparing to previous one, please keep that rule for your next patch. ;) > > Reviewed-by: Chao Yu > > Thanks, > >> fs/f2fs/data.c | 5 + >> fs/f2fs/file.c | 4 +++- >> 2 files changed, 8 insertions(+), 1 deletion(-) >> >> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c >> index aa8843a..7226300 100644 >> --- a/fs/f2fs/data.c >> +++ b/fs/f2fs/data.c >> @@ -1052,6 +1052,10 @@ int f2fs_map_blocks(struct inode *inode, struct >> f2fs_map_blocks *map, >> end = pgofs + maxblocks; >> >> if (!create && f2fs_lookup_extent_cache(inode, pgofs, )) { >> +if (test_opt(sbi, LFS) && flag == F2FS_GET_BLOCK_DIO && >> +map->m_may_create) >> +goto next_dnode; >> + >> map->m_pblk = ei.blk + pgofs - ei.fofs; >> map->m_len = min((pgoff_t)maxblocks, ei.fofs + ei.len - pgofs); >> map->m_flags = F2FS_MAP_MAPPED; >> @@ -1261,6 +1265,7 @@ bool f2fs_overwrite_io(struct inode *inode, loff_t >> pos, size_t len) >> map.m_next_pgofs = NULL; >> map.m_next_extent = NULL; >> map.m_seg_type = NO_CHECK_TYPE; >> +map.m_may_create = false; >> last_lblk = F2FS_BLK_ALIGN(pos + len); >> >> while (map.m_lblk < last_lblk) { >> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c >> index 3271830..ff82350 100644 >> --- a/fs/f2fs/file.c >> +++ b/fs/f2fs/file.c >> @@ -2201,7 +2201,8 @@ static int f2fs_defragment_range(struct f2fs_sb_info >> *sbi, >> { >> struct inode *inode = file_inode(filp); >> struct f2fs_map_blocks map = { .m_next_extent = NULL, >> -.m_seg_type = NO_CHECK_TYPE }; >> +.m_seg_type = NO_CHECK_TYPE , >> +.m_may_create = false }; >> struct extent_info ei = {0, 0, 0}; >> pgoff_t pg_start, pg_end, next_pgofs; >> unsigned int blk_per_seg = sbi->blocks_per_seg; >> @@ -2935,6 +2936,7 @@ int f2fs_precache_extents(struct inode *inode) >> map.m_next_pgofs = NULL; >> map.m_next_extent = _next_extent; >> map.m_seg_type = NO_CHECK_TYPE; >> +map.m_may_create = false; >> end = F2FS_I_SB(inode)->max_file_blocks; >> >> while (map.m_lblk < end) { >> > > . > ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: read page index before freeing
On 2018/11/27 8:22, PanBian wrote: > On Mon, Nov 26, 2018 at 07:07:08PM +0800, Chao Yu wrote: >> On 2018/11/26 18:28, PanBian wrote: >>> On Mon, Nov 26, 2018 at 05:13:53PM +0800, Chao Yu wrote: Hi Pan, On 2018/11/22 18:58, Pan Bian wrote: > The function truncate_node frees the page with f2fs_put_page. However, > the page index is read after that. So, the patch reads the index before > freeing the page. I notice that you found another use-after-free bug in ext4, out of curiosity, I'd like to ask how do you find those bugs? by tool or code review? >>> >>> I found such bugs by the aid of a tool I wrote recently. I designed a >>> method >>> to automatically find paired alloc/free functions. With such functions, I >>> wrote two checkers, one to check mismatched alloc/free bugs, the other to >>> check use-after-free and double-free bugs. >> >> Excellent! Do you have any plan to open its source or announce it w/ binary >> to linux kernel developers, I think w/ it we can help to improve kernel's >> code quality efficiently. > > Yes. I am now writing a paper about the method. I will open the source code > as soon as I complete the paper and some optimizations. Cool, if there is any progress, please let f2fs guys know, thank you in advance. :) Thanks, > > Best, > Pan > > > . > ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] fsck.f2fs: fix to skip repairing initialized i_gc_failures
On 2018/11/27 8:04, Jaegeuk Kim wrote: > On 11/26, Chao Yu wrote: >> From: Chao Yu >> >> As Michael reported: >> >> after updating to f2fs-tools 1.12.0, a routine fsck of my file systems >> took quite a while and output ten-thousands instances of the following >> line: >> >>> [FIX] (fsck_chk_inode_blk: 954) --> Regular: 0xXYZ reset i_gc_failures >>> from 0x1 to 0x00 >> >> In old kernel, we initialized i_gc_failures as 0x01, let's skip >> resetting such unchanged initialized value to avoid unnecessary >> repairing. >> >> Reported-by: Michael Laß >> Signed-off-by: Chao Yu >> --- >> fsck/fsck.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/fsck/fsck.c b/fsck/fsck.c >> index 970d150..db0e72f 100644 >> --- a/fsck/fsck.c >> +++ b/fsck/fsck.c >> @@ -941,7 +941,9 @@ skip_blkcnt_fix: >> } >> >> i_gc_failures = le16_to_cpu(node_blk->i.i_gc_failures); >> -if (ftype == F2FS_FT_REG_FILE && i_gc_failures) { >> + >> +/* old kernel initialized i_gc_failures as 0x01, skip repairing */ >> +if (ftype == F2FS_FT_REG_FILE && i_gc_failures > 1) { > > This will break the current implementation. Yeah, but I doubt that after repairing i_gc_failures, old kernel can still continue creating inodes in where .i_gc_failures equals to 0x01, then fsck will report such info each time..., can we relief fsck in such condition? Thanks, > >> >> DBG(1, "Regular Inode: 0x%x [%s] depth: %d\n\n", >> le32_to_cpu(node_blk->footer.ino), en, >> -- >> 2.18.0 > > . > ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 5/7] ext4: use IS_VERITY() to check inode's fsverity status
On Monday, November 26, 2018 11:06:15 PM IST Theodore Y. Ts'o wrote: > On Mon, Nov 19, 2018 at 10:53:22AM +0530, Chandan Rajendra wrote: > > This commit now uses IS_VERITY() macro to check if fsverity is > > enabled on an inode. > > > > Signed-off-by: Chandan Rajendra > > This patch causes a massive number of fsverity tests. I suspect it's > due to a mismatch between the ext4's inode flags as opposed to the VFS > inode's flags. I'll take a closer look in the next day or two. > I will check this and report back soon. -- chandan ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v2] f2fs: fix sbi->extent_list corruption issue
On 2018/11/27 8:30, Jaegeuk Kim wrote: > On 11/26, Sahitya Tummala wrote: >> When there is a failure in f2fs_fill_super() after/during >> the recovery of fsync'd nodes, it frees the current sbi and >> retries again. This time the mount is successful, but the files >> that got recovered before retry, still holds the extent tree, >> whose extent nodes list is corrupted since sbi and sbi->extent_list >> is freed up. The list_del corruption issue is observed when the >> file system is getting unmounted and when those recoverd files extent >> node is being freed up in the below context. >> >> list_del corruption. prev->next should be fff1e1ef5480, but was (null) >> <...> >> kernel BUG at kernel/msm-4.14/lib/list_debug.c:53! >> task: fff1f46f2280 task.stack: ff8008068000 >> lr : __list_del_entry_valid+0x94/0xb4 >> pc : __list_del_entry_valid+0x94/0xb4 >> <...> >> Call trace: >> __list_del_entry_valid+0x94/0xb4 >> __release_extent_node+0xb0/0x114 >> __free_extent_tree+0x58/0x7c >> f2fs_shrink_extent_tree+0xdc/0x3b0 >> f2fs_leave_shrinker+0x28/0x7c >> f2fs_put_super+0xfc/0x1e0 >> generic_shutdown_super+0x70/0xf4 >> kill_block_super+0x2c/0x5c >> kill_f2fs_super+0x44/0x50 >> deactivate_locked_super+0x60/0x8c >> deactivate_super+0x68/0x74 >> cleanup_mnt+0x40/0x78 >> __cleanup_mnt+0x1c/0x28 >> task_work_run+0x48/0xd0 >> do_notify_resume+0x678/0xe98 >> work_pending+0x8/0x14 >> >> Fix this by cleaning up inodes, extent tree and nodes of those >> recovered files before freeing up sbi and before next retry. >> >> Signed-off-by: Sahitya Tummala >> --- >> v2: >> -call evict_inodes() and f2fs_shrink_extent_tree() to cleanup inodes >> >> fs/f2fs/f2fs.h | 1 + >> fs/f2fs/shrinker.c | 2 +- >> fs/f2fs/super.c| 13 - >> 3 files changed, 14 insertions(+), 2 deletions(-) >> >> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h >> index 1e03197..aaee63b 100644 >> --- a/fs/f2fs/f2fs.h >> +++ b/fs/f2fs/f2fs.h >> @@ -3407,6 +3407,7 @@ struct rb_entry *f2fs_lookup_rb_tree_ret(struct >> rb_root_cached *root, >> bool f2fs_check_rb_tree_consistence(struct f2fs_sb_info *sbi, >> struct rb_root_cached *root); >> unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int >> nr_shrink); >> +unsigned long __count_extent_cache(struct f2fs_sb_info *sbi); >> bool f2fs_init_extent_tree(struct inode *inode, struct f2fs_extent *i_ext); >> void f2fs_drop_extent_tree(struct inode *inode); >> unsigned int f2fs_destroy_extent_node(struct inode *inode); >> diff --git a/fs/f2fs/shrinker.c b/fs/f2fs/shrinker.c >> index 9e13db9..7e3c13b 100644 >> --- a/fs/f2fs/shrinker.c >> +++ b/fs/f2fs/shrinker.c >> @@ -30,7 +30,7 @@ static unsigned long __count_free_nids(struct f2fs_sb_info >> *sbi) >> return count > 0 ? count : 0; >> } >> >> -static unsigned long __count_extent_cache(struct f2fs_sb_info *sbi) >> +unsigned long __count_extent_cache(struct f2fs_sb_info *sbi) >> { >> return atomic_read(>total_zombie_tree) + >> atomic_read(>total_ext_node); >> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c >> index af58b2c..769e7b1 100644 >> --- a/fs/f2fs/super.c >> +++ b/fs/f2fs/super.c >> @@ -3016,6 +3016,16 @@ static void f2fs_tuning_parameters(struct >> f2fs_sb_info *sbi) >> sbi->readdir_ra = 1; >> } >> >> +static void f2fs_cleanup_inodes(struct f2fs_sb_info *sbi) >> +{ >> +struct super_block *sb = sbi->sb; >> + >> +sync_filesystem(sb); > > This writes another checkpoint, which would not be what this retrial intended. Actually, checkpoint will not be triggered due to SBI_POR_DOING flag check as below: int f2fs_sync_fs(struct super_block *sb, int sync) { ... if (unlikely(is_sbi_flag_set(sbi, SBI_POR_DOING))) return -EAGAIN; ... } And also all dirty data/node won't be persisted due to SBI_POR_DOING flag, IIUC. Thanks, > How about adding a condition in f2fs_may_extent_tree() when adding extents? > Likewise, if (shrinker is not registered) return false; > > >> +shrink_dcache_sb(sb); >> +evict_inodes(sb); >> +f2fs_shrink_extent_tree(sbi, __count_extent_cache(sbi)); >> +} >> + >> static int f2fs_fill_super(struct super_block *sb, void *data, int silent) >> { >> struct f2fs_sb_info *sbi; >> @@ -3402,6 +3412,8 @@ static int f2fs_fill_super(struct super_block *sb, >> void *data, int silent) >> * falls into an infinite loop in f2fs_sync_meta_pages(). >> */ >> truncate_inode_pages_final(META_MAPPING(sbi)); >> +/* cleanup recovery and quota inodes */ >> +f2fs_cleanup_inodes(sbi); >> f2fs_unregister_sysfs(sbi); >> free_root_inode: >> dput(sb->s_root); >> @@ -3445,7 +3457,6 @@ static int f2fs_fill_super(struct super_block *sb, >> void *data, int silent) >> /* give only one another chance */ >> if (retry) { >> retry = false; >> -shrink_dcache_sb(sb); >> goto try_onemore; >> } >> return err;
Re: [f2fs-dev] [PATCH 7/7] fsverity: Remove filesystem specific build config option
Hi Chandan, On Mon, Nov 19, 2018 at 10:53:24AM +0530, Chandan Rajendra wrote: > In order to have a common code base for fsverity "post read" processing > for all filesystems which support per-file verity, this commit removes > filesystem specific build config option (e.g. CONFIG_EXT4_FS_VERITY) and > replaces it with a build option (i.e. CONFIG_FS_VERITY) whose value > affects all the filesystems making use of fsverity. > > Signed-off-by: Chandan Rajendra Like the corresponding fscrypt patch, this is missing changing #if IS_ENABLED(CONFIG_FS_VERITY) in include/linux/fs.h to #ifdef CONFIG_FS_VERITY There are also references to the filesystem-specific config options in Documentation/filesystems/fsverity.rst that need to be updated. I also suggest updating the Kconfig help text for CONFIG_FS_VERITY and CONFIG_FS_ENCRYPTION to mention the supported filesystems, similar to how CONFIG_QUOTA lists the filesystems it supports. Thanks! - Eric > --- > fs/ext4/Kconfig | 20 > fs/ext4/ext4.h | 2 -- > fs/ext4/readpage.c | 4 ++-- > fs/ext4/super.c | 6 +++--- > fs/ext4/sysfs.c | 4 ++-- > fs/f2fs/Kconfig | 20 > fs/f2fs/data.c | 2 +- > fs/f2fs/f2fs.h | 2 -- > fs/f2fs/super.c | 6 +++--- > fs/f2fs/sysfs.c | 4 ++-- > fs/verity/Kconfig| 2 +- > include/linux/fsverity.h | 3 +-- > 12 files changed, 15 insertions(+), 60 deletions(-) > > diff --git a/fs/ext4/Kconfig b/fs/ext4/Kconfig > index e1002bbf35bf..031e5a82d556 100644 > --- a/fs/ext4/Kconfig > +++ b/fs/ext4/Kconfig > @@ -96,26 +96,6 @@ config EXT4_FS_SECURITY > If you are not using a security module that requires using > extended attributes for file security labels, say N. > > -config EXT4_FS_VERITY > - bool "Ext4 Verity" > - depends on EXT4_FS > - select FS_VERITY > - help > - This option enables fs-verity for ext4. fs-verity is the > - dm-verity mechanism implemented at the file level. Userspace > - can append a Merkle tree (hash tree) to a file, then enable > - fs-verity on the file. ext4 will then transparently verify > - any data read from the file against the Merkle tree. The file > - is also made read-only. > - > - This serves as an integrity check, but the availability of the > - Merkle tree root hash also allows efficiently supporting > - various use cases where normally the whole file would need to > - be hashed at once, such as auditing and authenticity > - verification (appraisal). > - > - If unsure, say N. > - > config EXT4_DEBUG > bool "EXT4 debugging support" > depends on EXT4_FS > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h > index 64bf9fb7ef18..bff8d639dd0c 100644 > --- a/fs/ext4/ext4.h > +++ b/fs/ext4/ext4.h > @@ -41,8 +41,6 @@ > #endif > > #include > - > -#define __FS_HAS_VERITY IS_ENABLED(CONFIG_EXT4_FS_VERITY) > #include > > #include > diff --git a/fs/ext4/readpage.c b/fs/ext4/readpage.c > index 2c037df629dd..8717ac0a5bb2 100644 > --- a/fs/ext4/readpage.c > +++ b/fs/ext4/readpage.c > @@ -158,7 +158,7 @@ static struct bio_post_read_ctx > *get_bio_post_read_ctx(struct inode *inode, > > if (IS_ENCRYPTED(inode) && S_ISREG(inode->i_mode)) > post_read_steps |= 1 << STEP_DECRYPT; > -#ifdef CONFIG_EXT4_FS_VERITY > +#ifdef CONFIG_FS_VERITY > if (inode->i_verity_info != NULL && > (index < ((i_size_read(inode) + PAGE_SIZE - 1) >> PAGE_SHIFT))) > post_read_steps |= 1 << STEP_VERITY; > @@ -205,7 +205,7 @@ static void mpage_end_io(struct bio *bio) > > static inline loff_t ext4_readpage_limit(struct inode *inode) > { > -#ifdef CONFIG_EXT4_FS_VERITY > +#ifdef CONFIG_FS_VERITY > if (IS_VERITY(inode)) { > if (inode->i_verity_info) > /* limit to end of metadata region */ > diff --git a/fs/ext4/super.c b/fs/ext4/super.c > index 16fb483a6f4a..472338c7cd03 100644 > --- a/fs/ext4/super.c > +++ b/fs/ext4/super.c > @@ -1316,7 +1316,7 @@ static const struct fscrypt_operations ext4_cryptops = { > }; > #endif > > -#ifdef CONFIG_EXT4_FS_VERITY > +#ifdef CONFIG_FS_VERITY > static int ext4_set_verity(struct inode *inode, loff_t data_i_size) > { > int err; > @@ -1401,7 +1401,7 @@ static const struct fsverity_operations ext4_verityops > = { > .set_verity = ext4_set_verity, > .get_metadata_end = ext4_get_metadata_end, > }; > -#endif /* CONFIG_EXT4_FS_VERITY */ > +#endif /* CONFIG_FS_VERITY */ > > #ifdef CONFIG_QUOTA > static const char * const quotatypes[] = INITQFNAMES; > @@ -4234,7 +4234,7 @@ static int ext4_fill_super(struct super_block *sb, void > *data, int silent) > #ifdef CONFIG_FS_ENCRYPTION > sb->s_cop = _cryptops; > #endif > -#ifdef CONFIG_EXT4_FS_VERITY > +#ifdef CONFIG_FS_VERITY > sb->s_vop = _verityops; >
Re: [f2fs-dev] [PATCH v2] f2fs: fix to update new block address correctly for OPU
Hi Jia, On 11/27, Jia Zhu wrote: > Previously, we allocated a new block address for OPU mode in direct_IO. > > But the new address couldn't be assigned to @map->m_pblk correctly. > > This patch fix it. > > Fixes: 511f52d02f05 ('f2fs: allow out-place-update for direct IO in LFS mode') I've marked this patch for -stable merge. Thanks, > > Signed-off-by: Jia Zhu > --- > v2: > - update commit message. > > fs/f2fs/data.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > index 7226300..a3a567a 100644 > --- a/fs/f2fs/data.c > +++ b/fs/f2fs/data.c > @@ -1110,8 +1110,10 @@ int f2fs_map_blocks(struct inode *inode, struct > f2fs_map_blocks *map, > if (test_opt(sbi, LFS) && flag == F2FS_GET_BLOCK_DIO && > map->m_may_create) { > err = __allocate_data_block(, map->m_seg_type); > - if (!err) > + if (!err) { > + blkaddr = dn.data_blkaddr; > set_inode_flag(inode, FI_APPEND_WRITE); > + } > } > } else { > if (create) { > -- > 2.10.1 ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 6/7] f2fs: use IS_VERITY() to check inode's fsverity status
Hi Chandan, On Mon, Nov 19, 2018 at 10:53:23AM +0530, Chandan Rajendra wrote: > This commit now uses IS_VERITY() macro to check if fsverity is > enabled on an inode. > > Signed-off-by: Chandan Rajendra > --- > fs/f2fs/file.c | 6 +++--- > fs/f2fs/inode.c | 4 +++- > 2 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c > index 6c7ad15000b9..2eb4821d95d1 100644 > --- a/fs/f2fs/file.c > +++ b/fs/f2fs/file.c > @@ -491,7 +491,7 @@ static int f2fs_file_open(struct inode *inode, struct > file *filp) > if (err) > return err; > > - if (f2fs_verity_file(inode)) { > + if (IS_VERITY(inode)) { > err = fsverity_file_open(inode, filp); > if (err) > return err; > @@ -701,7 +701,7 @@ int f2fs_getattr(const struct path *path, struct kstat > *stat, > struct f2fs_inode *ri; > unsigned int flags; > > - if (f2fs_verity_file(inode)) { > + if (IS_VERITY(inode)) { > /* >* For fs-verity we need to override i_size with the original >* data i_size. This requires I/O to the file which with > @@ -800,7 +800,7 @@ int f2fs_setattr(struct dentry *dentry, struct iattr > *attr) > if (err) > return err; > > - if (f2fs_verity_file(inode)) { > + if (IS_VERITY(inode)) { > err = fsverity_prepare_setattr(dentry, attr); > if (err) > return err; > diff --git a/fs/f2fs/inode.c b/fs/f2fs/inode.c > index ddef483ad689..02806feed64c 100644 > --- a/fs/f2fs/inode.c > +++ b/fs/f2fs/inode.c > @@ -45,9 +45,11 @@ void f2fs_set_inode_flags(struct inode *inode) > new_fl |= S_DIRSYNC; > if (f2fs_encrypted_inode(inode)) > new_fl |= S_ENCRYPTED; > + if (f2fs_verity_file(inode)) > + new_fl |= S_VERITY; > inode_set_flags(inode, new_fl, > S_SYNC|S_APPEND|S_IMMUTABLE|S_NOATIME|S_DIRSYNC| > - S_ENCRYPTED); > + S_ENCRYPTED|S_VERITY); > } > > static void __get_inode_rdev(struct inode *inode, struct f2fs_inode *ri) > -- > 2.19.1 Like the ext4 patch, it's missing syncing the verity flag to the VFS inode during FS_IOC_ENABLE_VERITY: diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index 4f58d5840d17..841019d8bc96 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -2207,6 +2207,7 @@ static int f2fs_set_verity(struct inode *inode, loff_t data_i_size) return err; file_set_verity(inode); + f2fs_set_inode_flags(inode); f2fs_mark_inode_dirty_sync(inode, true); return 0; } ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 1/7] ext4: use IS_ENCRYPTED() to check encryption status
Hi Chandan, On Mon, Nov 19, 2018 at 10:53:18AM +0530, Chandan Rajendra wrote: > @@ -4724,7 +4724,7 @@ static bool ext4_should_use_dax(struct inode *inode) > return false; > if (ext4_has_inline_data(inode)) > return false; > - if (ext4_encrypted_inode(inode)) > + if (IS_ENCRYPTED(inode)) > return false; > if (ext4_verity_inode(inode)) > return false; I think this part is wrong. See how ext4_should_use_dax() is called from ext4_set_inode_flags(), from ext4_set_context(). In this case, ext4_set_inode_flags() should set S_ENCRYPTED and clear S_DAX. However, you've changed ext4_should_use_dax() to check S_ENCRYPTED instead of EXT4_ENCRYPT_FL; but S_ENCRYPTED isn't set until later in ext4_set_inode_flags(), so S_DAX won't be cleared anymore. So I think you need to use ext4_test_inode_flag() here instead. Similarly for the verity check. - Eric ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v2] f2fs: fix sbi->extent_list corruption issue
On 11/26, Sahitya Tummala wrote: > When there is a failure in f2fs_fill_super() after/during > the recovery of fsync'd nodes, it frees the current sbi and > retries again. This time the mount is successful, but the files > that got recovered before retry, still holds the extent tree, > whose extent nodes list is corrupted since sbi and sbi->extent_list > is freed up. The list_del corruption issue is observed when the > file system is getting unmounted and when those recoverd files extent > node is being freed up in the below context. > > list_del corruption. prev->next should be fff1e1ef5480, but was (null) > <...> > kernel BUG at kernel/msm-4.14/lib/list_debug.c:53! > task: fff1f46f2280 task.stack: ff8008068000 > lr : __list_del_entry_valid+0x94/0xb4 > pc : __list_del_entry_valid+0x94/0xb4 > <...> > Call trace: > __list_del_entry_valid+0x94/0xb4 > __release_extent_node+0xb0/0x114 > __free_extent_tree+0x58/0x7c > f2fs_shrink_extent_tree+0xdc/0x3b0 > f2fs_leave_shrinker+0x28/0x7c > f2fs_put_super+0xfc/0x1e0 > generic_shutdown_super+0x70/0xf4 > kill_block_super+0x2c/0x5c > kill_f2fs_super+0x44/0x50 > deactivate_locked_super+0x60/0x8c > deactivate_super+0x68/0x74 > cleanup_mnt+0x40/0x78 > __cleanup_mnt+0x1c/0x28 > task_work_run+0x48/0xd0 > do_notify_resume+0x678/0xe98 > work_pending+0x8/0x14 > > Fix this by cleaning up inodes, extent tree and nodes of those > recovered files before freeing up sbi and before next retry. > > Signed-off-by: Sahitya Tummala > --- > v2: > -call evict_inodes() and f2fs_shrink_extent_tree() to cleanup inodes > > fs/f2fs/f2fs.h | 1 + > fs/f2fs/shrinker.c | 2 +- > fs/f2fs/super.c| 13 - > 3 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > index 1e03197..aaee63b 100644 > --- a/fs/f2fs/f2fs.h > +++ b/fs/f2fs/f2fs.h > @@ -3407,6 +3407,7 @@ struct rb_entry *f2fs_lookup_rb_tree_ret(struct > rb_root_cached *root, > bool f2fs_check_rb_tree_consistence(struct f2fs_sb_info *sbi, > struct rb_root_cached *root); > unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int > nr_shrink); > +unsigned long __count_extent_cache(struct f2fs_sb_info *sbi); > bool f2fs_init_extent_tree(struct inode *inode, struct f2fs_extent *i_ext); > void f2fs_drop_extent_tree(struct inode *inode); > unsigned int f2fs_destroy_extent_node(struct inode *inode); > diff --git a/fs/f2fs/shrinker.c b/fs/f2fs/shrinker.c > index 9e13db9..7e3c13b 100644 > --- a/fs/f2fs/shrinker.c > +++ b/fs/f2fs/shrinker.c > @@ -30,7 +30,7 @@ static unsigned long __count_free_nids(struct f2fs_sb_info > *sbi) > return count > 0 ? count : 0; > } > > -static unsigned long __count_extent_cache(struct f2fs_sb_info *sbi) > +unsigned long __count_extent_cache(struct f2fs_sb_info *sbi) > { > return atomic_read(>total_zombie_tree) + > atomic_read(>total_ext_node); > diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c > index af58b2c..769e7b1 100644 > --- a/fs/f2fs/super.c > +++ b/fs/f2fs/super.c > @@ -3016,6 +3016,16 @@ static void f2fs_tuning_parameters(struct f2fs_sb_info > *sbi) > sbi->readdir_ra = 1; > } > > +static void f2fs_cleanup_inodes(struct f2fs_sb_info *sbi) > +{ > + struct super_block *sb = sbi->sb; > + > + sync_filesystem(sb); This writes another checkpoint, which would not be what this retrial intended. How about adding a condition in f2fs_may_extent_tree() when adding extents? Likewise, if (shrinker is not registered) return false; > + shrink_dcache_sb(sb); > + evict_inodes(sb); > + f2fs_shrink_extent_tree(sbi, __count_extent_cache(sbi)); > +} > + > static int f2fs_fill_super(struct super_block *sb, void *data, int silent) > { > struct f2fs_sb_info *sbi; > @@ -3402,6 +3412,8 @@ static int f2fs_fill_super(struct super_block *sb, void > *data, int silent) >* falls into an infinite loop in f2fs_sync_meta_pages(). >*/ > truncate_inode_pages_final(META_MAPPING(sbi)); > + /* cleanup recovery and quota inodes */ > + f2fs_cleanup_inodes(sbi); > f2fs_unregister_sysfs(sbi); > free_root_inode: > dput(sb->s_root); > @@ -3445,7 +3457,6 @@ static int f2fs_fill_super(struct super_block *sb, void > *data, int silent) > /* give only one another chance */ > if (retry) { > retry = false; > - shrink_dcache_sb(sb); > goto try_onemore; > } > return err; > -- > Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux > Foundation Collaborative Project. ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 5/7] ext4: use IS_VERITY() to check inode's fsverity status
On Mon, Nov 26, 2018 at 12:36:15PM -0500, Theodore Y. Ts'o wrote: > On Mon, Nov 19, 2018 at 10:53:22AM +0530, Chandan Rajendra wrote: > > This commit now uses IS_VERITY() macro to check if fsverity is > > enabled on an inode. > > > > Signed-off-by: Chandan Rajendra > > This patch causes a massive number of fsverity tests. I suspect it's > due to a mismatch between the ext4's inode flags as opposed to the VFS > inode's flags. I'll take a closer look in the next day or two. > > Cheers, > > - Ted It's missing the following to set S_VERITY during the FS_IOC_ENABLE_VERITY ioctl: diff --git a/fs/ext4/super.c b/fs/ext4/super.c index ed933e64e95f..82b45cceb39b 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -1344,6 +1344,11 @@ static int ext4_set_verity(struct inode *inode, loff_t data_i_size) err = ext4_reserve_inode_write(handle, inode, ); if (err == 0) { ext4_set_inode_flag(inode, EXT4_INODE_VERITY); + /* +* Update inode->i_flags - S_VERITY will be enabled, +* S_DAX may be disabled +*/ + ext4_set_inode_flags(inode); EXT4_I(inode)->i_disksize = data_i_size; err = ext4_mark_iloc_dirty(handle, inode, ); } ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 3/7] fscrypt: Remove filesystem specific build config option
Hi Chandan, On Mon, Nov 19, 2018 at 10:53:20AM +0530, Chandan Rajendra wrote: > In order to have a common code base for fscrypt "post read" processing > for all filesystems which support encryption, this commit removes > filesystem specific build config option (e.g. CONFIG_EXT4_FS_ENCRYPTION) > and replaces it with a build option (i.e. CONFIG_FS_ENCRYPTION) whose > value affects all the filesystems making use of fscrypt. > > Signed-off-by: Chandan Rajendra Can you grep for EXT4_ENCRYPTION? There are still some references left, one in Documentation/filesystems/fscrypt.rst and some in defconfig files. Also in include/linux/fs.h, there are tests of #if IS_ENABLED(CONFIG_FS_ENCRYPTION) which after this change should be #ifdef CONFIG_FS_ENCRYPTION like is done elsewhere. Thanks, - Eric > --- > fs/crypto/Kconfig | 2 +- > fs/ext4/Kconfig | 15 -- > fs/ext4/dir.c | 2 - > fs/ext4/ext4.h | 7 +- > fs/ext4/inode.c | 8 +- > fs/ext4/ioctl.c | 4 +- > fs/ext4/namei.c | 10 +- > fs/ext4/page-io.c | 6 +- > fs/ext4/readpage.c | 2 +- > fs/ext4/super.c | 6 +- > fs/ext4/sysfs.c | 4 +- > fs/f2fs/Kconfig | 11 - > fs/f2fs/f2fs.h | 7 +- > fs/f2fs/super.c | 8 +- > fs/f2fs/sysfs.c | 4 +- > include/linux/fscrypt.h | 416 +++- > include/linux/fscrypt_notsupp.h | 231 -- > include/linux/fscrypt_supp.h| 204 > 18 files changed, 441 insertions(+), 506 deletions(-) > delete mode 100644 include/linux/fscrypt_notsupp.h > delete mode 100644 include/linux/fscrypt_supp.h > > diff --git a/fs/crypto/Kconfig b/fs/crypto/Kconfig > index 02b7d91c9231..6e9ae566a8fc 100644 > --- a/fs/crypto/Kconfig > +++ b/fs/crypto/Kconfig > @@ -1,5 +1,5 @@ > config FS_ENCRYPTION > - tristate "FS Encryption (Per-file encryption)" > + bool "FS Encryption (Per-file encryption)" > select CRYPTO > select CRYPTO_AES > select CRYPTO_CBC > diff --git a/fs/ext4/Kconfig b/fs/ext4/Kconfig > index 5a76125ac0f8..e1002bbf35bf 100644 > --- a/fs/ext4/Kconfig > +++ b/fs/ext4/Kconfig > @@ -96,21 +96,6 @@ config EXT4_FS_SECURITY > If you are not using a security module that requires using > extended attributes for file security labels, say N. > > -config EXT4_ENCRYPTION > - bool "Ext4 Encryption" > - depends on EXT4_FS > - select FS_ENCRYPTION > - help > - Enable encryption of ext4 files and directories. This > - feature is similar to ecryptfs, but it is more memory > - efficient since it avoids caching the encrypted and > - decrypted pages in the page cache. > - > -config EXT4_FS_ENCRYPTION > - bool > - default y > - depends on EXT4_ENCRYPTION > - > config EXT4_FS_VERITY > bool "Ext4 Verity" > depends on EXT4_FS > diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c > index fb7a64ea5679..0ccd51f72048 100644 > --- a/fs/ext4/dir.c > +++ b/fs/ext4/dir.c > @@ -283,9 +283,7 @@ static int ext4_readdir(struct file *file, struct > dir_context *ctx) > done: > err = 0; > errout: > -#ifdef CONFIG_EXT4_FS_ENCRYPTION > fscrypt_fname_free_buffer(); > -#endif > brelse(bh); > return err; > } > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h > index 2ae6ab88f218..db21df885186 100644 > --- a/fs/ext4/ext4.h > +++ b/fs/ext4/ext4.h > @@ -40,7 +40,6 @@ > #include > #endif > > -#define __FS_HAS_ENCRYPTION IS_ENABLED(CONFIG_EXT4_FS_ENCRYPTION) > #include > > #define __FS_HAS_VERITY IS_ENABLED(CONFIG_EXT4_FS_VERITY) > @@ -1341,7 +1340,7 @@ struct ext4_super_block { > #define EXT4_MF_FS_ABORTED 0x0002 /* Fatal error detected */ > #define EXT4_MF_TEST_DUMMY_ENCRYPTION0x0004 > > -#ifdef CONFIG_EXT4_FS_ENCRYPTION > +#ifdef CONFIG_FS_ENCRYPTION > #define DUMMY_ENCRYPTION_ENABLED(sbi) (unlikely((sbi)->s_mount_flags & \ > EXT4_MF_TEST_DUMMY_ENCRYPTION)) > #else > @@ -2069,7 +2068,7 @@ struct ext4_filename { > const struct qstr *usr_fname; > struct fscrypt_str disk_name; > struct dx_hash_info hinfo; > -#ifdef CONFIG_EXT4_FS_ENCRYPTION > +#ifdef CONFIG_FS_ENCRYPTION > struct fscrypt_str crypto_buf; > #endif > }; > @@ -2306,7 +2305,7 @@ static inline bool ext4_verity_inode(struct inode > *inode) > #endif > } > > -#ifdef CONFIG_EXT4_FS_ENCRYPTION > +#ifdef CONFIG_FS_ENCRYPTION > static inline int ext4_fname_setup_filename(struct inode *dir, > const struct qstr *iname, > int lookup, struct ext4_filename *fname) > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index 72572f32..ae6794649817 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -1150,7 +1150,7 @@ int
Re: [f2fs-dev] fsck: reset i_gc_failures from 0x1 to 0x00
Hi Michael, On 11/26, Michael Laß wrote: > Hi, > > > Am 26.11.2018 um 15:09 schrieb Chao Yu : > > On 2018-11-26 7:09, Michael Laß wrote: > >> Hi, > >> > >> after updating to f2fs-tools 1.12.0, a routine fsck of my file systems > >> took quite a while and output ten-thousands instances of the following > >> line: > >> > >>> [FIX] (fsck_chk_inode_blk: 954) --> Regular: 0xXYZ reset i_gc_failures > >>> from 0x1 to 0x00 > > > > Do you use -f or -p 1 option when doing fsck on image? > > One of the devices was automatically checked during boot-up. As far as I can > see, the following command is issued inside the initrd: > fsck -Ta -C"$FSCK_FD" "$1” > where $1 is the device. The other one I checked manually calling fsck.f2fs > without any additional arguments. From my experience, the checks are always > performed when the last check was done with an older kernel version (which > was the case here). > > > We start to support reseting .i_gc_failures's value to zero in fsck since > > 91bb7b21f740 ("f2fs-tools: fix to reset i_gc_failures offline"), this is > > because > > if .i_gc_failures continues increasing and exceed threshold, it can make > > f2fs > > break atomic_write semantics during GC, so I added that patch to avoid such > > condition. > > > > But the problem here is even .i_gc_failures's value is one which was > > initialized > > duing inode creation by old kernel, and it never be increased by GC flow, we > > will still trigger such fix in fsck. I think it's not necessary, anyway, > > let me > > send one patch to fix it. > > This is a very likely cause. The filesystems are both from early 2015, so > probably were used with Linux 3.18 or 3.19 at that time. Just in case, is this Android device? If you don't use ioctl(F2FS_PIN_FILE), this onetime fix wont' hurt any filesystem metadata. Thanks, > > Thanks for the explanation and the proposed patch! > > Best regards, > Michael > > ___ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 4/7] Add S_VERITY and IS_VERITY()
Hi Chandan, On Mon, Nov 19, 2018 at 10:53:21AM +0530, Chandan Rajendra wrote: > Similar to S_ENCRYPTED/IS_ENCRYPTED(), this commit adds > S_VERITY/IS_VERITY() to be able to check if a VFS inode has verity > information associated with it. > > Signed-off-by: Chandan Rajendra > --- > include/linux/fs.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/linux/fs.h b/include/linux/fs.h > index bcfc40062757..8129617c9718 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -1938,6 +1938,7 @@ struct super_operations { > #define S_DAX0 /* Make all the DAX code disappear */ > #endif > #define S_ENCRYPTED 16384 /* Encrypted file (using fs/crypto/) */ > +#define S_VERITY 32768 /* File with fsverity info (using fs/verity) */ > The comment for S_VERITY is misleading because IS_VERITY() is used to check whether the verity bit is set *before* the fsverity_info is created. Can you change it to just mirror the fscrypt comment? #define S_VERITY32768 /* Verity file (using fs/verity/) */ > /* > * Note that nosuid etc flags are inode-specific: setting some file-system > @@ -1978,6 +1979,7 @@ static inline bool sb_rdonly(const struct super_block > *sb) { return sb->s_flags > #define IS_NOSEC(inode) ((inode)->i_flags & S_NOSEC) > #define IS_DAX(inode)((inode)->i_flags & S_DAX) > #define IS_ENCRYPTED(inode) ((inode)->i_flags & S_ENCRYPTED) > +#define IS_VERITY(inode) ((inode)->i_flags & S_VERITY) > > #define IS_WHITEOUT(inode) (S_ISCHR(inode->i_mode) && \ >(inode)->i_rdev == WHITEOUT_DEV) > -- > 2.19.1 > Thanks, - Eric ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] fsck.f2fs: fix to skip repairing initialized i_gc_failures
On 11/26, Chao Yu wrote: > From: Chao Yu > > As Michael reported: > > after updating to f2fs-tools 1.12.0, a routine fsck of my file systems > took quite a while and output ten-thousands instances of the following > line: > > > [FIX] (fsck_chk_inode_blk: 954) --> Regular: 0xXYZ reset i_gc_failures > > from 0x1 to 0x00 > > In old kernel, we initialized i_gc_failures as 0x01, let's skip > resetting such unchanged initialized value to avoid unnecessary > repairing. > > Reported-by: Michael Laß > Signed-off-by: Chao Yu > --- > fsck/fsck.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/fsck/fsck.c b/fsck/fsck.c > index 970d150..db0e72f 100644 > --- a/fsck/fsck.c > +++ b/fsck/fsck.c > @@ -941,7 +941,9 @@ skip_blkcnt_fix: > } > > i_gc_failures = le16_to_cpu(node_blk->i.i_gc_failures); > - if (ftype == F2FS_FT_REG_FILE && i_gc_failures) { > + > + /* old kernel initialized i_gc_failures as 0x01, skip repairing */ > + if (ftype == F2FS_FT_REG_FILE && i_gc_failures > 1) { This will break the current implementation. > > DBG(1, "Regular Inode: 0x%x [%s] depth: %d\n\n", > le32_to_cpu(node_blk->footer.ino), en, > -- > 2.18.0 ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 2/7] f2fs: use IS_ENCRYPTED() to check encryption status
Hi Ted, On 11/26, Theodore Y. Ts'o wrote: > On Sun, Nov 25, 2018 at 11:00:38PM -0500, Theodore Y. Ts'o wrote: > > > > It might be that the simplest way to solve things is to merge the f2fs > > dev branch up to 79c66e75720c. This will have the net effect of > > including the five patches listed above onto the fscrypt git tree. So > > long you don't plan to rebase or otherwise change these five patches, > > it should avoid any merge conflicts. > > I've set up a git branch which has the f2fs dev branch, 4.20-rc4, the > fsverity patches, and part of Chandan's patch series here: > > git://git.kernel.org/pub/scm/linux/kernel/git/tytso/fscrypt.git test-working > > There is a minor conflict when I did a trial merge with f2fs.git's dev > branch, but it's pretty obvious to how to resolve it. > > Jaegeuk, Eric, Chandan, please take a look and let me know what you > think. I was about to rebase f2fs-dev branch to catch up -rc4, so could you please update test-working with the rebased one? Then, I'll test Eric & Chandan's patches in the test-working branch locally. If there is no issue, I'm okay to push all of their work via fscrypt.git, if you prefer. Afterward, merging f2fs patches till "f2fs: clean up f2fs_sb_has_##feature_name" into the test-working branch would be fine as well. Thanks, > > - Ted ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: check memory boundary by insane namelen
On 11/23, Sheng Yong wrote: > Hi, Jaegeuk and Chao, > > On 2018/11/15 15:50, Jaegeuk Kim wrote: > > If namelen is corrupted to have very long value, fill_dentries can copy > > wrong memory area. > > > Is there any scenario that could hit this corruption? Or this is triggered > by fuzzing injection? Hi Sheng, It's from a fuzzing test. Thanks, > > thanks, > Sheng Yong > > > Signed-off-by: Jaegeuk Kim > > --- > > fs/f2fs/dir.c | 12 +++- > > 1 file changed, 11 insertions(+), 1 deletion(-) > > > > diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c > > index bacc667950b6..c0c845da12fa 100644 > > --- a/fs/f2fs/dir.c > > +++ b/fs/f2fs/dir.c > > @@ -808,6 +808,17 @@ int f2fs_fill_dentries(struct dir_context *ctx, struct > > f2fs_dentry_ptr *d, > > de_name.name = d->filename[bit_pos]; > > de_name.len = le16_to_cpu(de->name_len); > > + /* check memory boundary before moving forward */ > > + bit_pos += GET_DENTRY_SLOTS(le16_to_cpu(de->name_len)); > > + if (unlikely(bit_pos > d->max)) { > > + f2fs_msg(sbi->sb, KERN_WARNING, > > + "%s: corrupted namelen=%d, run fsck to fix.", > > + __func__, le16_to_cpu(de->name_len)); > > + set_sbi_flag(sbi, SBI_NEED_FSCK); > > + err = -EINVAL; > > + goto out; > > + } > > + > > if (f2fs_encrypted_inode(d->inode)) { > > int save_len = fstr->len; > > @@ -830,7 +841,6 @@ int f2fs_fill_dentries(struct dir_context *ctx, struct > > f2fs_dentry_ptr *d, > > if (readdir_ra) > > f2fs_ra_node_page(sbi, le32_to_cpu(de->ino)); > > - bit_pos += GET_DENTRY_SLOTS(le16_to_cpu(de->name_len)); > > ctx->pos = start_pos + bit_pos; > > } > > out: > > ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 5/7] ext4: use IS_VERITY() to check inode's fsverity status
On Mon, Nov 19, 2018 at 10:53:22AM +0530, Chandan Rajendra wrote: > This commit now uses IS_VERITY() macro to check if fsverity is > enabled on an inode. > > Signed-off-by: Chandan Rajendra This patch causes a massive number of fsverity tests. I suspect it's due to a mismatch between the ext4's inode flags as opposed to the VFS inode's flags. I'll take a closer look in the next day or two. Cheers, - Ted ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH 2/7] f2fs: use IS_ENCRYPTED() to check encryption status
On Sun, Nov 25, 2018 at 11:00:38PM -0500, Theodore Y. Ts'o wrote: > > It might be that the simplest way to solve things is to merge the f2fs > dev branch up to 79c66e75720c. This will have the net effect of > including the five patches listed above onto the fscrypt git tree. So > long you don't plan to rebase or otherwise change these five patches, > it should avoid any merge conflicts. I've set up a git branch which has the f2fs dev branch, 4.20-rc4, the fsverity patches, and part of Chandan's patch series here: git://git.kernel.org/pub/scm/linux/kernel/git/tytso/fscrypt.git test-working There is a minor conflict when I did a trial merge with f2fs.git's dev branch, but it's pretty obvious to how to resolve it. Jaegeuk, Eric, Chandan, please take a look and let me know what you think. - Ted ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] fsck: reset i_gc_failures from 0x1 to 0x00
Hi, > Am 26.11.2018 um 15:09 schrieb Chao Yu : > On 2018-11-26 7:09, Michael Laß wrote: >> Hi, >> >> after updating to f2fs-tools 1.12.0, a routine fsck of my file systems >> took quite a while and output ten-thousands instances of the following >> line: >> >>> [FIX] (fsck_chk_inode_blk: 954) --> Regular: 0xXYZ reset i_gc_failures >>> from 0x1 to 0x00 > > Do you use -f or -p 1 option when doing fsck on image? One of the devices was automatically checked during boot-up. As far as I can see, the following command is issued inside the initrd: fsck -Ta -C"$FSCK_FD" "$1” where $1 is the device. The other one I checked manually calling fsck.f2fs without any additional arguments. From my experience, the checks are always performed when the last check was done with an older kernel version (which was the case here). > We start to support reseting .i_gc_failures's value to zero in fsck since > 91bb7b21f740 ("f2fs-tools: fix to reset i_gc_failures offline"), this is > because > if .i_gc_failures continues increasing and exceed threshold, it can make f2fs > break atomic_write semantics during GC, so I added that patch to avoid such > condition. > > But the problem here is even .i_gc_failures's value is one which was > initialized > duing inode creation by old kernel, and it never be increased by GC flow, we > will still trigger such fix in fsck. I think it's not necessary, anyway, let > me > send one patch to fix it. This is a very likely cause. The filesystems are both from early 2015, so probably were used with Linux 3.18 or 3.19 at that time. Thanks for the explanation and the proposed patch! Best regards, Michael ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH] fsck.f2fs: fix to skip repairing initialized i_gc_failures
From: Chao Yu As Michael reported: after updating to f2fs-tools 1.12.0, a routine fsck of my file systems took quite a while and output ten-thousands instances of the following line: > [FIX] (fsck_chk_inode_blk: 954) --> Regular: 0xXYZ reset i_gc_failures from > 0x1 to 0x00 In old kernel, we initialized i_gc_failures as 0x01, let's skip resetting such unchanged initialized value to avoid unnecessary repairing. Reported-by: Michael Laß Signed-off-by: Chao Yu --- fsck/fsck.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/fsck/fsck.c b/fsck/fsck.c index 970d150..db0e72f 100644 --- a/fsck/fsck.c +++ b/fsck/fsck.c @@ -941,7 +941,9 @@ skip_blkcnt_fix: } i_gc_failures = le16_to_cpu(node_blk->i.i_gc_failures); - if (ftype == F2FS_FT_REG_FILE && i_gc_failures) { + + /* old kernel initialized i_gc_failures as 0x01, skip repairing */ + if (ftype == F2FS_FT_REG_FILE && i_gc_failures > 1) { DBG(1, "Regular Inode: 0x%x [%s] depth: %d\n\n", le32_to_cpu(node_blk->footer.ino), en, -- 2.18.0 ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] fsck: reset i_gc_failures from 0x1 to 0x00
Hi Michael, On 2018-11-26 7:09, Michael Laß wrote: > Hi, > > after updating to f2fs-tools 1.12.0, a routine fsck of my file systems > took quite a while and output ten-thousands instances of the following > line: > >> [FIX] (fsck_chk_inode_blk: 954) --> Regular: 0xXYZ reset i_gc_failures from >> 0x1 to 0x00 Do you use -f or -p 1 option when doing fsck on image? > > where XYZ varied from line to line. I assume this is related to > 91bb7b21f740d61cde2bb27da3dccb0afdd5a15b but I'm not sure about the > implications. Is this something to worry about or some deliberate > change of the on-disk format which was now braught up to date by fsck? We start to support reseting .i_gc_failures's value to zero in fsck since 91bb7b21f740 ("f2fs-tools: fix to reset i_gc_failures offline"), this is because if .i_gc_failures continues increasing and exceed threshold, it can make f2fs break atomic_write semantics during GC, so I added that patch to avoid such condition. But the problem here is even .i_gc_failures's value is one which was initialized duing inode creation by old kernel, and it never be increased by GC flow, we will still trigger such fix in fsck. I think it's not necessary, anyway, let me send one patch to fix it. Thanks, > > Best regards, > Michael > > > > > > ___ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel > ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] Bug in f2fs-tools-1.12.0
Hi Perfect Gentleman, On 2018-11-26 21:47, Perfect Gentleman wrote: > Hi Chao > > With that patch fsck.f2fs works for RO-partition with -f option. Thanks for your test. :) Thanks, > > ~ $ sudo mount -o remount,ro /dev/sda1 > > ~ $ sudo fsck.f2fs -a /dev/sda1 > Info: Fix the reported corruption. > Info: Mounted device! > Info: Check FS only due to RO > Error: Failed to open the device! > > ~ $ sudo fsck.f2fs -af /dev/sda1 > Info: Fix the reported corruption. > Info: Force to fix corruption > Info: Mounted device! > Info: Check FS only due to RO > Info: [/dev/sda1] Disk Model: SAMSUNG MZ7WD240 > Info: Segments per section = 1 > Info: Sections per zone = 1 > Info: sector size = 512 > Info: total sectors = 468860047 (228935 MB) > Info: MKFS version > "Linux version 4.16.6-gentoo (root@kubuntu) (gcc version 7.3.0 (Gentoo > 7.3.0-r1 p1.1)) #1 ZEN SMP PREEMPT Tue May 1 18:28:05 +07 2018" > Info: FSCK version > from "Linux version 4.19.4-gentoo (root@HOST) (gcc version 8.2.0 (Gentoo > 8.2.0-r4 p1.5)) #1 ZEN SMP PREEMPT Fri Nov 23 22:15:59 +07 2018" > to "Linux version 4.19.4-gentoo (root@HOST) (gcc version 8.2.0 (Gentoo > 8.2.0-r4 p1.5)) #1 ZEN SMP PREEMPT Fri Nov 23 22:15:59 +07 2018" > Info: superblock features = 0 : > Info: superblock encrypt level = 0, salt = > Info: total FS sectors = 468860040 (228935 MB) > Info: CKPT version = 40e50ae1 > Info: Checked valid nat_bits in checkpoint > Info: checkpoint state = c5 : nat_bits crc compacted_summary unmount > > [FSCK] Unreachable nat entries [Ok..] [0x0] > [FSCK] SIT valid block bitmap checking [Ok..] > [FSCK] Hard link checking for regular file [Ok..] [0x0] > [FSCK] valid_block_count matching with CP [Ok..] [0x334ce10] > [FSCK] valid_node_count matcing with CP (de lookup) [Ok..] [0x1fe41] > [FSCK] valid_node_count matcing with CP (nat lookup) [Ok..] [0x1fe41] > [FSCK] valid_inode_count matched with CP [Ok..] [0x12fb1] > [FSCK] free segment_count matched with CP [Ok..] [0x41f0] > [FSCK] next block offset is free [Ok..] > [FSCK] fixing SIT types > [FSCK] other corrupted bugs [Ok..] > > Done. > > Regards > > On 11/26/18 8:35 PM, Chao Yu wrote: >> Hi Perfect Gentleman, >> >> On 2018-11-26 19:11, Perfect Gentleman wrote: >>> Hi Chao >>> >>> What patch do you mean? >> I mean >> >> https://git.kernel.org/pub/scm/linux/kernel/git/chao/f2fs-tools.git/commit/?h=dev-test=a6160c3e21f43b89b49802cc4a956d1c4b65ae44 >> >> >> Thanks, >> >>> Redards >>> >>> P.S. It's reply to all :) >>> >>> On 11/26/18 5:52 PM, Chao Yu wrote: Hi Perfect Gentleman, Thanks for the report. On 2018/11/25 13:24, Perfect Gentleman wrote: > Hi f2fs-team, > > there is bug: fsck.f2fs cannot check read-only filesystem. But 1.11.0 > can do that. > > fsck |Info: Fix the reported corruption. > fsck |Info: Mounted device! > fsck |Info: Check FS only due to RO > fsck |Error: Failed to open the device! I guess this is due to below commit, which stop opening device in f2fs-tools if the device has already been opened by other one, like mount. https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs-tools.git/commit/?h=dev-test=eb9d8037ed3b37a647d514470f1a1df91daedb64 I tried e2fsprogs, w/o -f option, the result is the same: fsck 1.44.4 (18-Aug-2018) e2fsck 1.44.4 (18-Aug-2018) /dev/zram0 is mounted. e2fsck: Cannot continue, aborting. But still we can trigger forced fsck.ext4 by using -f option. Could you have a try with following patch? [PATCH] fsck.f2fs: allow to fsck readonly image w/ -f option Thanks, > fsck | * Filesystems couldn't be fixed > [ !! ] > fsck | * ERROR: fsck failed to start > > Regards, > Alex > > > > ___ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel > >>> >>> ___ >>> Linux-f2fs-devel mailing list >>> Linux-f2fs-devel@lists.sourceforge.net >>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] Bug in f2fs-tools-1.12.0
Hi Chao With that patch fsck.f2fs works for RO-partition with -f option. ~ $ sudo mount -o remount,ro /dev/sda1 ~ $ sudo fsck.f2fs -a /dev/sda1 Info: Fix the reported corruption. Info: Mounted device! Info: Check FS only due to RO Error: Failed to open the device! ~ $ sudo fsck.f2fs -af /dev/sda1 Info: Fix the reported corruption. Info: Force to fix corruption Info: Mounted device! Info: Check FS only due to RO Info: [/dev/sda1] Disk Model: SAMSUNG MZ7WD240 Info: Segments per section = 1 Info: Sections per zone = 1 Info: sector size = 512 Info: total sectors = 468860047 (228935 MB) Info: MKFS version "Linux version 4.16.6-gentoo (root@kubuntu) (gcc version 7.3.0 (Gentoo 7.3.0-r1 p1.1)) #1 ZEN SMP PREEMPT Tue May 1 18:28:05 +07 2018" Info: FSCK version from "Linux version 4.19.4-gentoo (root@HOST) (gcc version 8.2.0 (Gentoo 8.2.0-r4 p1.5)) #1 ZEN SMP PREEMPT Fri Nov 23 22:15:59 +07 2018" to "Linux version 4.19.4-gentoo (root@HOST) (gcc version 8.2.0 (Gentoo 8.2.0-r4 p1.5)) #1 ZEN SMP PREEMPT Fri Nov 23 22:15:59 +07 2018" Info: superblock features = 0 : Info: superblock encrypt level = 0, salt = Info: total FS sectors = 468860040 (228935 MB) Info: CKPT version = 40e50ae1 Info: Checked valid nat_bits in checkpoint Info: checkpoint state = c5 : nat_bits crc compacted_summary unmount [FSCK] Unreachable nat entries [Ok..] [0x0] [FSCK] SIT valid block bitmap checking [Ok..] [FSCK] Hard link checking for regular file [Ok..] [0x0] [FSCK] valid_block_count matching with CP [Ok..] [0x334ce10] [FSCK] valid_node_count matcing with CP (de lookup) [Ok..] [0x1fe41] [FSCK] valid_node_count matcing with CP (nat lookup) [Ok..] [0x1fe41] [FSCK] valid_inode_count matched with CP [Ok..] [0x12fb1] [FSCK] free segment_count matched with CP [Ok..] [0x41f0] [FSCK] next block offset is free [Ok..] [FSCK] fixing SIT types [FSCK] other corrupted bugs [Ok..] Done. Regards On 11/26/18 8:35 PM, Chao Yu wrote: Hi Perfect Gentleman, On 2018-11-26 19:11, Perfect Gentleman wrote: Hi Chao What patch do you mean? I mean https://git.kernel.org/pub/scm/linux/kernel/git/chao/f2fs-tools.git/commit/?h=dev-test=a6160c3e21f43b89b49802cc4a956d1c4b65ae44 Thanks, Redards P.S. It's reply to all :) On 11/26/18 5:52 PM, Chao Yu wrote: Hi Perfect Gentleman, Thanks for the report. On 2018/11/25 13:24, Perfect Gentleman wrote: Hi f2fs-team, there is bug: fsck.f2fs cannot check read-only filesystem. But 1.11.0 can do that. fsck |Info: Fix the reported corruption. fsck |Info: Mounted device! fsck |Info: Check FS only due to RO fsck |Error: Failed to open the device! I guess this is due to below commit, which stop opening device in f2fs-tools if the device has already been opened by other one, like mount. https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs-tools.git/commit/?h=dev-test=eb9d8037ed3b37a647d514470f1a1df91daedb64 I tried e2fsprogs, w/o -f option, the result is the same: fsck 1.44.4 (18-Aug-2018) e2fsck 1.44.4 (18-Aug-2018) /dev/zram0 is mounted. e2fsck: Cannot continue, aborting. But still we can trigger forced fsck.ext4 by using -f option. Could you have a try with following patch? [PATCH] fsck.f2fs: allow to fsck readonly image w/ -f option Thanks, fsck | * Filesystems couldn't be fixed [ !! ] fsck | * ERROR: fsck failed to start Regards, Alex ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH] f2fs-tools: fix to check return value of {c, m}alloc()
From: Chao Yu It needs to fix to handle error case of {c,m}alloc(). Signed-off-by: Chao Yu --- fsck/dump.c | 4 fsck/fsck.c | 2 ++ fsck/mount.c | 13 + lib/libf2fs.c| 4 lib/libf2fs_io.c | 4 5 files changed, 27 insertions(+) diff --git a/fsck/dump.c b/fsck/dump.c index 8cf431c..d0e3355 100644 --- a/fsck/dump.c +++ b/fsck/dump.c @@ -277,6 +277,8 @@ static void dump_node_blk(struct f2fs_sb_info *sbi, int ntype, get_node_info(sbi, nid, ); node_blk = calloc(BLOCK_SZ, 1); + ASSERT(node_blk); + dev_read_block(node_blk, ni.blk_addr); for (i = 0; i < idx; i++, (*ofs)++) { @@ -475,6 +477,8 @@ void dump_node(struct f2fs_sb_info *sbi, nid_t nid, int force) get_node_info(sbi, nid, ); node_blk = calloc(BLOCK_SZ, 1); + ASSERT(node_blk); + dev_read_block(node_blk, ni.blk_addr); DBG(1, "Node ID [0x%x]\n", nid); diff --git a/fsck/fsck.c b/fsck/fsck.c index 366ba13..970d150 100644 --- a/fsck/fsck.c +++ b/fsck/fsck.c @@ -1420,6 +1420,8 @@ static int __chk_dentries(struct f2fs_sb_info *sbi, struct child_info *child, continue; } name = calloc(name_len + 1, 1); + ASSERT(name); + memcpy(name, filenames[i], name_len); slots = (name_len + F2FS_SLOT_LEN - 1) / F2FS_SLOT_LEN; diff --git a/fsck/mount.c b/fsck/mount.c index 861e5fb..d853fcf 100644 --- a/fsck/mount.c +++ b/fsck/mount.c @@ -664,6 +664,8 @@ int validate_super_block(struct f2fs_sb_info *sbi, enum SB_ADDR sb_addr) char buf[F2FS_BLKSIZE]; sbi->raw_super = malloc(sizeof(struct f2fs_super_block)); + if (!sbi->raw_super) + return -ENOMEM; if (dev_read_block(buf, sb_addr)) return -1; @@ -779,6 +781,8 @@ void *validate_checkpoint(struct f2fs_sb_info *sbi, block_t cp_addr, /* Read the 1st cp block in this CP pack */ cp_page_1 = malloc(PAGE_SIZE); + ASSERT(cp_page_1); + if (dev_read_block(cp_page_1, cp_addr) < 0) goto invalid_cp1; @@ -798,6 +802,8 @@ void *validate_checkpoint(struct f2fs_sb_info *sbi, block_t cp_addr, /* Read the 2nd cp block in this CP pack */ cp_page_2 = malloc(PAGE_SIZE); + ASSERT(cp_page_2); + cp_addr += get_cp(cp_pack_total_block_count) - 1; if (dev_read_block(cp_page_2, cp_addr) < 0) @@ -1320,6 +1326,9 @@ int build_sit_info(struct f2fs_sb_info *sbi) src_bitmap = __bitmap_ptr(sbi, SIT_BITMAP); dst_bitmap = malloc(bitmap_size); + if (!dst_bitmap) + return -ENOMEM; + memcpy(dst_bitmap, src_bitmap, bitmap_size); sit_i->sit_base_addr = get_sb(sit_blkaddr); @@ -1361,6 +1370,8 @@ static void read_compacted_summaries(struct f2fs_sb_info *sbi) start = start_sum_block(sbi); kaddr = (char *)malloc(PAGE_SIZE); + ASSERT(kaddr); + ret = dev_read_block(kaddr, start++); ASSERT(ret >= 0); @@ -1452,6 +1463,8 @@ static void read_normal_summaries(struct f2fs_sb_info *sbi, int type) } sum_blk = (struct f2fs_summary_block *)malloc(PAGE_SIZE); + ASSERT(sum_blk); + ret = dev_read_block(sum_blk, blk_addr); ASSERT(ret >= 0); diff --git a/lib/libf2fs.c b/lib/libf2fs.c index 498f6c0..c692bf2 100644 --- a/lib/libf2fs.c +++ b/lib/libf2fs.c @@ -566,6 +566,8 @@ char *get_rootdev() } uevent = malloc(ret + 1); + ASSERT(uevent); + uevent[ret] = '\0'; ret = read(fd, uevent, ret); @@ -709,6 +711,8 @@ int f2fs_dev_is_umounted(char *path) * the file system. In this case, we should not format. */ st_buf = malloc(sizeof(struct stat)); + ASSERT(st_buf); + if (stat(path, st_buf) == 0 && S_ISBLK(st_buf->st_mode)) { int fd = open(path, O_RDONLY | O_EXCL); diff --git a/lib/libf2fs_io.c b/lib/libf2fs_io.c index 47917ab..b40c3d2 100644 --- a/lib/libf2fs_io.c +++ b/lib/libf2fs_io.c @@ -302,6 +302,10 @@ int f2fs_init_sparse_file(void) } blocks_count = c.device_size / F2FS_BLKSIZE; blocks = calloc(blocks_count, sizeof(char *)); + if (!blocks) { + MSG(0, "\tError: Calloc Failed for blocks!!!\n"); + return -1; + } return sparse_file_foreach_chunk(f2fs_sparse_file, true, false, sparse_import_segment, NULL); -- 2.18.0 ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] Bug in f2fs-tools-1.12.0
Hi Perfect Gentleman, On 2018-11-26 19:11, Perfect Gentleman wrote: > Hi Chao > > What patch do you mean? I mean https://git.kernel.org/pub/scm/linux/kernel/git/chao/f2fs-tools.git/commit/?h=dev-test=a6160c3e21f43b89b49802cc4a956d1c4b65ae44 Thanks, > > Redards > > P.S. It's reply to all :) > > On 11/26/18 5:52 PM, Chao Yu wrote: >> Hi Perfect Gentleman, >> >> Thanks for the report. >> >> On 2018/11/25 13:24, Perfect Gentleman wrote: >>> Hi f2fs-team, >>> >>> there is bug: fsck.f2fs cannot check read-only filesystem. But 1.11.0 >>> can do that. >>> >>> fsck |Info: Fix the reported corruption. >>> fsck |Info: Mounted device! >>> fsck |Info: Check FS only due to RO >>> fsck |Error: Failed to open the device! >> I guess this is due to below commit, which stop opening device in >> f2fs-tools if the device has already been opened by other one, like mount. >> >> https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs-tools.git/commit/?h=dev-test=eb9d8037ed3b37a647d514470f1a1df91daedb64 >> >> >> I tried e2fsprogs, w/o -f option, the result is the same: >> >> fsck 1.44.4 (18-Aug-2018) >> e2fsck 1.44.4 (18-Aug-2018) >> /dev/zram0 is mounted. >> e2fsck: Cannot continue, aborting. >> >> But still we can trigger forced fsck.ext4 by using -f option. >> >> Could you have a try with following patch? >> >> [PATCH] fsck.f2fs: allow to fsck readonly image w/ -f option >> >> Thanks, >> >>> fsck | * Filesystems couldn't be fixed >>> [ !! ] >>> fsck | * ERROR: fsck failed to start >>> >>> Regards, >>> Alex >>> >>> >>> >>> ___ >>> Linux-f2fs-devel mailing list >>> Linux-f2fs-devel@lists.sourceforge.net >>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel >>> > > > ___ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] Bug in f2fs-tools-1.12.0
Hi Chao What patch do you mean? Redards P.S. It's reply to all :) On 11/26/18 5:52 PM, Chao Yu wrote: Hi Perfect Gentleman, Thanks for the report. On 2018/11/25 13:24, Perfect Gentleman wrote: Hi f2fs-team, there is bug: fsck.f2fs cannot check read-only filesystem. But 1.11.0 can do that. fsck |Info: Fix the reported corruption. fsck |Info: Mounted device! fsck |Info: Check FS only due to RO fsck |Error: Failed to open the device! I guess this is due to below commit, which stop opening device in f2fs-tools if the device has already been opened by other one, like mount. https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs-tools.git/commit/?h=dev-test=eb9d8037ed3b37a647d514470f1a1df91daedb64 I tried e2fsprogs, w/o -f option, the result is the same: fsck 1.44.4 (18-Aug-2018) e2fsck 1.44.4 (18-Aug-2018) /dev/zram0 is mounted. e2fsck: Cannot continue, aborting. But still we can trigger forced fsck.ext4 by using -f option. Could you have a try with following patch? [PATCH] fsck.f2fs: allow to fsck readonly image w/ -f option Thanks, fsck | * Filesystems couldn't be fixed [ !! ] fsck | * ERROR: fsck failed to start Regards, Alex ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: read page index before freeing
On 2018/11/26 18:28, PanBian wrote: > On Mon, Nov 26, 2018 at 05:13:53PM +0800, Chao Yu wrote: >> Hi Pan, >> >> On 2018/11/22 18:58, Pan Bian wrote: >>> The function truncate_node frees the page with f2fs_put_page. However, >>> the page index is read after that. So, the patch reads the index before >>> freeing the page. >> >> I notice that you found another use-after-free bug in ext4, out of >> curiosity, I'd like to ask how do you find those bugs? by tool or code >> review? > > I found such bugs by the aid of a tool I wrote recently. I designed a method > to automatically find paired alloc/free functions. With such functions, I > wrote two checkers, one to check mismatched alloc/free bugs, the other to > check use-after-free and double-free bugs. Excellent! Do you have any plan to open its source or announce it w/ binary to linux kernel developers, I think w/ it we can help to improve kernel's code quality efficiently. Thanks, > > Best regards, > Pan Bian > > > . > ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH] fsck.f2fs: allow to fsck readonly image w/ -f option
To keep line with e2fsprogs, let's allow to fsck mounted image as readonly w/ -f option. Reported-by: Perfect Gentleman Signed-off-by: Chao Yu --- fsck/main.c | 1 + include/f2fs_fs.h | 1 + lib/libf2fs.c | 2 +- 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/fsck/main.c b/fsck/main.c index 675c603b1a76..bb79f6e9fc89 100644 --- a/fsck/main.c +++ b/fsck/main.c @@ -249,6 +249,7 @@ void f2fs_parse_options(int argc, char *argv[]) case 'f': case 'y': c.fix_on = 1; + c.force = 1; MSG(0, "Info: Force to fix corruption\n"); break; case 'q': diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h index 65cc8fdb3c22..6eebb3ac44e1 100644 --- a/include/f2fs_fs.h +++ b/include/f2fs_fs.h @@ -369,6 +369,7 @@ struct f2fs_configuration { void *private; int dry_run; int fix_on; + int force; int defset; int bug_on; int alloc_failed; diff --git a/lib/libf2fs.c b/lib/libf2fs.c index cc335dbb48ee..498f6c0968d8 100644 --- a/lib/libf2fs.c +++ b/lib/libf2fs.c @@ -821,7 +821,7 @@ int get_device_info(int i) return -1; } - if (S_ISBLK(stat_buf->st_mode)) + if (S_ISBLK(stat_buf->st_mode) && !c.force) fd = open(dev->path, O_RDWR | O_EXCL); else fd = open(dev->path, O_RDWR); -- 2.18.0.rc1 ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] Bug in f2fs-tools-1.12.0
Hi Perfect Gentleman, Thanks for the report. On 2018/11/25 13:24, Perfect Gentleman wrote: > Hi f2fs-team, > > there is bug: fsck.f2fs cannot check read-only filesystem. But 1.11.0 > can do that. > > fsck |Info: Fix the reported corruption. > fsck |Info: Mounted device! > fsck |Info: Check FS only due to RO > fsck |Error: Failed to open the device! I guess this is due to below commit, which stop opening device in f2fs-tools if the device has already been opened by other one, like mount. https://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs-tools.git/commit/?h=dev-test=eb9d8037ed3b37a647d514470f1a1df91daedb64 I tried e2fsprogs, w/o -f option, the result is the same: fsck 1.44.4 (18-Aug-2018) e2fsck 1.44.4 (18-Aug-2018) /dev/zram0 is mounted. e2fsck: Cannot continue, aborting. But still we can trigger forced fsck.ext4 by using -f option. Could you have a try with following patch? [PATCH] fsck.f2fs: allow to fsck readonly image w/ -f option Thanks, > fsck | * Filesystems couldn't be fixed > [ !! ] > fsck | * ERROR: fsck failed to start > > Regards, > Alex > > > > ___ > Linux-f2fs-devel mailing list > Linux-f2fs-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel > ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v2] f2fs: fix to update new block address correctly for OPU
On Tue, Nov 27, 2018 at 02:32:32AM +0800, Jia Zhu wrote: > Previously, we allocated a new block address for OPU mode in direct_IO. > > But the new address couldn't be assigned to @map->m_pblk correctly. > > This patch fix it. > > Fixes: 511f52d02f05 ('f2fs: allow out-place-update for direct IO in LFS mode') > > Signed-off-by: Jia Zhu > --- > v2: > - update commit message. > > fs/f2fs/data.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) This is not the correct way to submit patches for inclusion in the stable kernel tree. Please read: https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html for how to do this properly. ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: read page index before freeing
On Mon, Nov 26, 2018 at 05:13:53PM +0800, Chao Yu wrote: > Hi Pan, > > On 2018/11/22 18:58, Pan Bian wrote: > > The function truncate_node frees the page with f2fs_put_page. However, > > the page index is read after that. So, the patch reads the index before > > freeing the page. > > I notice that you found another use-after-free bug in ext4, out of > curiosity, I'd like to ask how do you find those bugs? by tool or code review? I found such bugs by the aid of a tool I wrote recently. I designed a method to automatically find paired alloc/free functions. With such functions, I wrote two checkers, one to check mismatched alloc/free bugs, the other to check use-after-free and double-free bugs. Best regards, Pan Bian ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH v2] f2fs: fix to update new block address correctly for OPU
Previously, we allocated a new block address for OPU mode in direct_IO. But the new address couldn't be assigned to @map->m_pblk correctly. This patch fix it. Fixes: 511f52d02f05 ('f2fs: allow out-place-update for direct IO in LFS mode') Signed-off-by: Jia Zhu --- v2: - update commit message. fs/f2fs/data.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index 7226300..a3a567a 100644 --- a/fs/f2fs/data.c +++ b/fs/f2fs/data.c @@ -1110,8 +1110,10 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, if (test_opt(sbi, LFS) && flag == F2FS_GET_BLOCK_DIO && map->m_may_create) { err = __allocate_data_block(, map->m_seg_type); - if (!err) + if (!err) { + blkaddr = dn.data_blkaddr; set_inode_flag(inode, FI_APPEND_WRITE); + } } } else { if (create) { -- 2.10.1 ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v3 2/2] f2fs: adjust trace print in f2fs_get_victim() to cover all paths
On 2018/11/26 16:01, Sahitya Tummala wrote: > Adjust the trace print in f2fs_get_victim() to cover GC done by > F2FS_IOC_GARBAGE_COLLECT_RANGE. > > Signed-off-by: Sahitya Tummala Reviewed-by: Chao Yu Thanks, ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v3 1/2] f2fs: fix to allow node segment for GC by ioctl path
On 2018/11/26 16:01, Sahitya Tummala wrote: > Allow node type segments also to be GC'd via f2fs ioctl > F2FS_IOC_GARBAGE_COLLECT_RANGE. > > Signed-off-by: Sahitya Tummala Reviewed-by: Chao Yu Thanks, ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH] f2fs: read page index before freeing
Hi Pan, On 2018/11/22 18:58, Pan Bian wrote: > The function truncate_node frees the page with f2fs_put_page. However, > the page index is read after that. So, the patch reads the index before > freeing the page. I notice that you found another use-after-free bug in ext4, out of curiosity, I'd like to ask how do you find those bugs? by tool or code review? Thanks, ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH v3 2/2] f2fs: adjust trace print in f2fs_get_victim() to cover all paths
Adjust the trace print in f2fs_get_victim() to cover GC done by F2FS_IOC_GARBAGE_COLLECT_RANGE. Signed-off-by: Sahitya Tummala --- fs/f2fs/gc.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c index d720551..e4689c6 100644 --- a/fs/f2fs/gc.c +++ b/fs/f2fs/gc.c @@ -403,11 +403,12 @@ static int get_victim_by_default(struct f2fs_sb_info *sbi, } *result = (p.min_segno / p.ofs_unit) * p.ofs_unit; + } +out: + if (p.min_segno != NULL_SEGNO) trace_f2fs_get_victim(sbi->sb, type, gc_type, , sbi->cur_victim_sec, prefree_segments(sbi), free_segments(sbi)); - } -out: mutex_unlock(_i->seglist_lock); return (p.min_segno == NULL_SEGNO) ? 0 : 1; -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project. ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH v3 1/2] f2fs: fix to allow node segment for GC by ioctl path
Allow node type segments also to be GC'd via f2fs ioctl F2FS_IOC_GARBAGE_COLLECT_RANGE. Signed-off-by: Sahitya Tummala --- v3: seperate the trace print change from this patch fs/f2fs/gc.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c index a07241f..d720551 100644 --- a/fs/f2fs/gc.c +++ b/fs/f2fs/gc.c @@ -323,8 +323,7 @@ static int get_victim_by_default(struct f2fs_sb_info *sbi, p.min_cost = get_max_cost(sbi, ); if (*result != NULL_SEGNO) { - if (IS_DATASEG(get_seg_entry(sbi, *result)->type) && - get_valid_blocks(sbi, *result, false) && + if (get_valid_blocks(sbi, *result, false) && !sec_usage_check(sbi, GET_SEC_FROM_SEG(sbi, *result))) p.min_segno = *result; goto out; -- Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project. ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel