* [f2fs-dev] [PATCH 1/2] fsck.f2fs: fix to check validation of block address @ 2020-04-07 10:01 Chao Yu 2020-04-07 10:01 ` [f2fs-dev] [PATCH 2/2] fsck.f2fs: fix to check validation of i_xattr_nid Chao Yu 0 siblings, 1 reply; 7+ messages in thread From: Chao Yu @ 2020-04-07 10:01 UTC (permalink / raw) To: linux-f2fs-devel; +Cc: kilobyte, jaegeuk Otherwise, if block address is invalid, we may access invalid memory address in is_sit_bitmap_set(). Signed-off-by: Chao Yu <yuchao0@huawei.com> --- fsck/dump.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/fsck/dump.c b/fsck/dump.c index 8481a58..144c10e 100644 --- a/fsck/dump.c +++ b/fsck/dump.c @@ -486,10 +486,15 @@ void dump_node(struct f2fs_sb_info *sbi, nid_t nid, int force) DBG(1, "nat_entry.version [0x%x]\n", ni.version); DBG(1, "nat_entry.ino [0x%x]\n", ni.ino); + if (!IS_VALID_BLK_ADDR(sbi, ni.blk_addr)) { + MSG(force, "Invalid node blkaddr: %u\n\n", ni.blk_addr); + goto out; + } + if (ni.blk_addr == 0x0) MSG(force, "Invalid nat entry\n\n"); else if (!is_sit_bitmap_set(sbi, ni.blk_addr)) - MSG(force, "Invalid node blk addr\n\n"); + MSG(force, "Invalid sit bitmap, %u\n\n", ni.blk_addr); DBG(1, "node_blk.footer.ino [0x%x]\n", le32_to_cpu(node_blk->footer.ino)); DBG(1, "node_blk.footer.nid [0x%x]\n", le32_to_cpu(node_blk->footer.nid)); @@ -504,7 +509,7 @@ void dump_node(struct f2fs_sb_info *sbi, nid_t nid, int force) print_node_info(sbi, node_blk, force); MSG(force, "Invalid (i)node block\n\n"); } - +out: free(node_blk); } -- 2.18.0.rc1 _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [f2fs-dev] [PATCH 2/2] fsck.f2fs: fix to check validation of i_xattr_nid 2020-04-07 10:01 [f2fs-dev] [PATCH 1/2] fsck.f2fs: fix to check validation of block address Chao Yu @ 2020-04-07 10:01 ` Chao Yu 2020-10-31 23:48 ` Eric Biggers 0 siblings, 1 reply; 7+ messages in thread From: Chao Yu @ 2020-04-07 10:01 UTC (permalink / raw) To: linux-f2fs-devel; +Cc: kilobyte, jaegeuk Otherwise, fsck.f2fs will access invalid memory address as below: - fsck_verify - dump_node - dump_file - dump_inode_blk - dump_xattr - read_all_xattrs - get_node_info access &(F2FS_FSCK(sbi)->entries[nid]) Signed-off-by: Chao Yu <yuchao0@huawei.com> --- fsck/dump.c | 2 ++ fsck/fsck.c | 8 ++++++++ fsck/fsck.h | 3 +++ fsck/mount.c | 8 +++++--- fsck/xattr.c | 20 ++++++++++++++++++-- 5 files changed, 36 insertions(+), 5 deletions(-) diff --git a/fsck/dump.c b/fsck/dump.c index 144c10e..001b7cb 100644 --- a/fsck/dump.c +++ b/fsck/dump.c @@ -309,6 +309,8 @@ static void dump_xattr(struct f2fs_sb_info *sbi, struct f2fs_node *node_blk) int ret; xattr = read_all_xattrs(sbi, node_blk); + if (!xattr) + return; list_for_each_xattr(ent, xattr) { char *name = strndup(ent->e_name, ent->e_name_len); diff --git a/fsck/fsck.c b/fsck/fsck.c index a08a8cb..0389146 100644 --- a/fsck/fsck.c +++ b/fsck/fsck.c @@ -487,6 +487,14 @@ static int sanity_check_nid(struct f2fs_sb_info *sbi, u32 nid, return 0; } +int fsck_sanity_check_nid(struct f2fs_sb_info *sbi, u32 nid, + struct f2fs_node *node_blk, + enum FILE_TYPE ftype, enum NODE_TYPE ntype, + struct node_info *ni) +{ + return sanity_check_nid(sbi, nid, node_blk, ftype, ntype, ni); +} + static int fsck_chk_xattr_blk(struct f2fs_sb_info *sbi, u32 ino, u32 x_nid, u32 *blk_cnt) { diff --git a/fsck/fsck.h b/fsck/fsck.h index c4432e8..2de6f62 100644 --- a/fsck/fsck.h +++ b/fsck/fsck.h @@ -144,6 +144,9 @@ static inline bool need_fsync_data_record(struct f2fs_sb_info *sbi) extern int fsck_chk_orphan_node(struct f2fs_sb_info *); extern int fsck_chk_quota_node(struct f2fs_sb_info *); extern int fsck_chk_quota_files(struct f2fs_sb_info *); +extern int fsck_sanity_check_nid(struct f2fs_sb_info *, u32, + struct f2fs_node *, enum FILE_TYPE, enum NODE_TYPE, + struct node_info *); extern int fsck_chk_node_blk(struct f2fs_sb_info *, struct f2fs_inode *, u32, enum FILE_TYPE, enum NODE_TYPE, u32 *, struct child_info *); diff --git a/fsck/mount.c b/fsck/mount.c index 4d16659..0aab071 100644 --- a/fsck/mount.c +++ b/fsck/mount.c @@ -257,10 +257,12 @@ void print_inode_info(struct f2fs_sb_info *sbi, DISP_u32(inode, i_nid[4]); /* double indirect */ xattr_addr = read_all_xattrs(sbi, node); - list_for_each_xattr(ent, xattr_addr) { - print_xattr_entry(ent); + if (xattr_addr) { + list_for_each_xattr(ent, xattr_addr) { + print_xattr_entry(ent); + } + free(xattr_addr); } - free(xattr_addr); printf("\n"); } diff --git a/fsck/xattr.c b/fsck/xattr.c index d5350e3..e9dcb52 100644 --- a/fsck/xattr.c +++ b/fsck/xattr.c @@ -22,6 +22,22 @@ void *read_all_xattrs(struct f2fs_sb_info *sbi, struct f2fs_node *inode) struct f2fs_xattr_header *header; void *txattr_addr; u64 inline_size = inline_xattr_size(&inode->i); + nid_t xnid = le32_to_cpu(inode->i.i_xattr_nid); + + if (xnid) { + struct f2fs_node *node_blk = NULL; + struct node_info ni; + int ret; + + node_blk = (struct f2fs_node *)calloc(BLOCK_SZ, 1); + ASSERT(node_blk != NULL); + + ret = fsck_sanity_check_nid(sbi, xnid, node_blk, + F2FS_FT_XATTR, TYPE_XATTR, &ni); + free(node_blk); + if (ret) + return NULL; + } txattr_addr = calloc(inline_size + BLOCK_SZ, 1); ASSERT(txattr_addr); @@ -30,11 +46,11 @@ void *read_all_xattrs(struct f2fs_sb_info *sbi, struct f2fs_node *inode) memcpy(txattr_addr, inline_xattr_addr(&inode->i), inline_size); /* Read from xattr node block. */ - if (inode->i.i_xattr_nid) { + if (xnid) { struct node_info ni; int ret; - get_node_info(sbi, le32_to_cpu(inode->i.i_xattr_nid), &ni); + get_node_info(sbi, xnid, &ni); ret = dev_read_block(txattr_addr + inline_size, ni.blk_addr); ASSERT(ret >= 0); } -- 2.18.0.rc1 _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [f2fs-dev] [PATCH 2/2] fsck.f2fs: fix to check validation of i_xattr_nid 2020-04-07 10:01 ` [f2fs-dev] [PATCH 2/2] fsck.f2fs: fix to check validation of i_xattr_nid Chao Yu @ 2020-10-31 23:48 ` Eric Biggers 2020-11-02 1:31 ` Chao Yu 0 siblings, 1 reply; 7+ messages in thread From: Eric Biggers @ 2020-10-31 23:48 UTC (permalink / raw) To: Chao Yu; +Cc: kilobyte, jaegeuk, linux-f2fs-devel Hi Chao, On Tue, Apr 07, 2020 at 06:01:07PM +0800, Chao Yu wrote: > Otherwise, fsck.f2fs will access invalid memory address as below: > > - fsck_verify > - dump_node > - dump_file > - dump_inode_blk > - dump_xattr > - read_all_xattrs > - get_node_info > access &(F2FS_FSCK(sbi)->entries[nid]) > > Signed-off-by: Chao Yu <yuchao0@huawei.com> > --- > fsck/dump.c | 2 ++ > fsck/fsck.c | 8 ++++++++ > fsck/fsck.h | 3 +++ > fsck/mount.c | 8 +++++--- > fsck/xattr.c | 20 ++++++++++++++++++-- > 5 files changed, 36 insertions(+), 5 deletions(-) > This commit caused a regression where 'dump.f2fs -i <inode> <device>' now segfaults if the inode has any extended attributes. It's because read_all_xattrs() now calls fsck_sanity_check_nid(), which eventually dereferences f2fs_fsck::main_area_bitmap, which is NULL. I'm not sure what was intended here. Here's the output from gdb: (gdb) r -i 4 ~/fstests//kvm-xfstests/disks/vdc Starting program: /usr/bin/dump.f2fs -i 4 ~/fstests//kvm-xfstests/disks/vdc Info: Segments per section = 1 Info: Sections per zone = 1 Info: sector size = 512 Info: total sectors = 10485760 (5120 MB) Info: MKFS version "Linux version 4.9.241-00003-g631a4cd718af2 (e@sol) (gcc version 10.2.0 (GCC) ) #70 SMP Sat Oct 31 16:22:38 PDT 2020" Info: FSCK version from "Linux version 4.9.241-00003-g631a4cd718af2 (e@sol) (gcc version 10.2.0 (GCC) ) #70 SMP Sat Oct 31 16:22:38 PDT 2020" to "Linux version 5.10.0-rc1-00346-gebe40414a48c (e@sol) (gcc (GCC) 10.2.0, GNU ld (GNU Binutils) 2.35.1) #1 SMP PREEMPT Fri Oct 30 20:03:27 PDT 2020" Info: superblock features = 0 : Info: superblock encrypt level = 0, salt = 00000000000000000000000000000000 Info: total FS sectors = 10485760 (5120 MB) Info: CKPT version = 6e5d0386 [print_node_info: 353] Node ID [0x4:4] is inode i_mode [0x 81a4 : 33188] i_advise [0x 0 : 0] i_uid [0x 0 : 0] i_gid [0x 0 : 0] i_links [0x 1 : 1] i_size [0x 0 : 0] i_blocks [0x 2 : 2] i_atime [0x5f9df65f : 1604187743] i_atime_nsec [0x2e869e08 : 780574216] i_ctime [0x5f9df65f : 1604187743] i_ctime_nsec [0x2e869e08 : 780574216] i_mtime [0x5f9df65f : 1604187743] i_mtime_nsec [0x2e869e08 : 780574216] i_generation [0xf41ca108 : 4095516936] i_current_depth [0x 1 : 1] i_xattr_nid [0x 5 : 5] i_flags [0x 0 : 0] i_inline [0x 2 : 2] i_pino [0x 3 : 3] i_dir_level [0x 0 : 0] i_namelen [0x 4 : 4] i_name [file] i_ext: fofs:0 blkaddr:0 len:0 i_nid[0] [0x 0 : 0] i_nid[1] [0x 0 : 0] i_nid[2] [0x 0 : 0] i_nid[3] [0x 0 : 0] i_nid[4] [0x 0 : 0] Program received signal SIGSEGV, Segmentation fault. 0x00007ffff7f750fa in f2fs_test_bit (nr=1024, p=0x0) at libf2fs.c:304 304 return (mask & *addr) != 0; (gdb) bt #0 0x00007ffff7f750fa in f2fs_test_bit (nr=1024, p=0x0) at libf2fs.c:304 #1 0x000055555555a953 in f2fs_test_main_bitmap (sbi=0x555555593d80 <gfsck>, blk=12288) at fsck.c:44 #2 0x000055555555bd9f in sanity_check_nid (sbi=0x555555593d80 <gfsck>, nid=5, node_blk=0x5555555bb3a0, ftype=F2FS_FT_XATTR, ntype=TYPE_XATTR, ni=0x7fffffffdd20) at fsck.c:449 #3 0x000055555555c013 in fsck_sanity_check_nid (sbi=0x555555593d80 <gfsck>, nid=5, node_blk=0x5555555bb3a0, ftype=F2FS_FT_XATTR, ntype=TYPE_XATTR, ni=0x7fffffffdd20) at fsck.c:495 #4 0x000055555557d4d6 in read_all_xattrs (sbi=0x555555593d80 <gfsck>, inode=0x5555555ba390) at xattr.c:35 #5 0x00005555555698ea in print_inode_info (sbi=0x555555593d80 <gfsck>, node=0x5555555ba390, name=0) at mount.c:335 #6 0x0000555555569a09 in print_node_info (sbi=0x555555593d80 <gfsck>, node_block=0x5555555ba390, verbose=0) at mount.c:354 #7 0x0000555555566b55 in dump_node (sbi=0x555555593d80 <gfsck>, nid=4, force=0) at dump.c:507 #8 0x0000555555559850 in do_dump (sbi=0x555555593d80 <gfsck>) at main.c:729 #9 0x0000555555559ee1 in main (argc=4, argv=0x7fffffffe238) at main.c:892 (gdb) _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [f2fs-dev] [PATCH 2/2] fsck.f2fs: fix to check validation of i_xattr_nid 2020-10-31 23:48 ` Eric Biggers @ 2020-11-02 1:31 ` Chao Yu 2020-11-02 2:39 ` Eric Biggers 0 siblings, 1 reply; 7+ messages in thread From: Chao Yu @ 2020-11-02 1:31 UTC (permalink / raw) To: Eric Biggers; +Cc: kilobyte, jaegeuk, linux-f2fs-devel On 2020/11/1 7:48, Eric Biggers wrote: > Hi Chao, > > On Tue, Apr 07, 2020 at 06:01:07PM +0800, Chao Yu wrote: >> Otherwise, fsck.f2fs will access invalid memory address as below: >> >> - fsck_verify >> - dump_node >> - dump_file >> - dump_inode_blk >> - dump_xattr >> - read_all_xattrs >> - get_node_info >> access &(F2FS_FSCK(sbi)->entries[nid]) >> >> Signed-off-by: Chao Yu <yuchao0@huawei.com> >> --- >> fsck/dump.c | 2 ++ >> fsck/fsck.c | 8 ++++++++ >> fsck/fsck.h | 3 +++ >> fsck/mount.c | 8 +++++--- >> fsck/xattr.c | 20 ++++++++++++++++++-- >> 5 files changed, 36 insertions(+), 5 deletions(-) >> > > This commit caused a regression where 'dump.f2fs -i <inode> <device>' > now segfaults if the inode has any extended attributes. > > It's because read_all_xattrs() now calls fsck_sanity_check_nid(), which > eventually dereferences f2fs_fsck::main_area_bitmap, which is NULL. > > I'm not sure what was intended here. Eric, could you please have a try with below commit: https://git.kernel.org/pub/scm/linux/kernel/git/chao/f2fs-tools.git/commit/?h=dev-test&id=aad80ed0099fb9530ae3af9789362353ff580252 Thanks, > > Here's the output from gdb: > > (gdb) r -i 4 ~/fstests//kvm-xfstests/disks/vdc > Starting program: /usr/bin/dump.f2fs -i 4 ~/fstests//kvm-xfstests/disks/vdc > Info: Segments per section = 1 > Info: Sections per zone = 1 > Info: sector size = 512 > Info: total sectors = 10485760 (5120 MB) > Info: MKFS version > "Linux version 4.9.241-00003-g631a4cd718af2 (e@sol) (gcc version 10.2.0 (GCC) ) #70 SMP Sat Oct 31 16:22:38 PDT 2020" > Info: FSCK version > from "Linux version 4.9.241-00003-g631a4cd718af2 (e@sol) (gcc version 10.2.0 (GCC) ) #70 SMP Sat Oct 31 16:22:38 PDT 2020" > to "Linux version 5.10.0-rc1-00346-gebe40414a48c (e@sol) (gcc (GCC) 10.2.0, GNU ld (GNU Binutils) 2.35.1) #1 SMP PREEMPT Fri Oct 30 20:03:27 PDT 2020" > Info: superblock features = 0 : > Info: superblock encrypt level = 0, salt = 00000000000000000000000000000000 > Info: total FS sectors = 10485760 (5120 MB) > Info: CKPT version = 6e5d0386 > [print_node_info: 353] Node ID [0x4:4] is inode > i_mode [0x 81a4 : 33188] > i_advise [0x 0 : 0] > i_uid [0x 0 : 0] > i_gid [0x 0 : 0] > i_links [0x 1 : 1] > i_size [0x 0 : 0] > i_blocks [0x 2 : 2] > i_atime [0x5f9df65f : 1604187743] > i_atime_nsec [0x2e869e08 : 780574216] > i_ctime [0x5f9df65f : 1604187743] > i_ctime_nsec [0x2e869e08 : 780574216] > i_mtime [0x5f9df65f : 1604187743] > i_mtime_nsec [0x2e869e08 : 780574216] > i_generation [0xf41ca108 : 4095516936] > i_current_depth [0x 1 : 1] > i_xattr_nid [0x 5 : 5] > i_flags [0x 0 : 0] > i_inline [0x 2 : 2] > i_pino [0x 3 : 3] > i_dir_level [0x 0 : 0] > i_namelen [0x 4 : 4] > i_name [file] > i_ext: fofs:0 blkaddr:0 len:0 > i_nid[0] [0x 0 : 0] > i_nid[1] [0x 0 : 0] > i_nid[2] [0x 0 : 0] > i_nid[3] [0x 0 : 0] > i_nid[4] [0x 0 : 0] > > Program received signal SIGSEGV, Segmentation fault. > 0x00007ffff7f750fa in f2fs_test_bit (nr=1024, p=0x0) at libf2fs.c:304 > 304 return (mask & *addr) != 0; > (gdb) bt > #0 0x00007ffff7f750fa in f2fs_test_bit (nr=1024, p=0x0) at libf2fs.c:304 > #1 0x000055555555a953 in f2fs_test_main_bitmap (sbi=0x555555593d80 <gfsck>, blk=12288) at fsck.c:44 > #2 0x000055555555bd9f in sanity_check_nid (sbi=0x555555593d80 <gfsck>, nid=5, node_blk=0x5555555bb3a0, ftype=F2FS_FT_XATTR, > ntype=TYPE_XATTR, ni=0x7fffffffdd20) at fsck.c:449 > #3 0x000055555555c013 in fsck_sanity_check_nid (sbi=0x555555593d80 <gfsck>, nid=5, node_blk=0x5555555bb3a0, ftype=F2FS_FT_XATTR, > ntype=TYPE_XATTR, ni=0x7fffffffdd20) at fsck.c:495 > #4 0x000055555557d4d6 in read_all_xattrs (sbi=0x555555593d80 <gfsck>, inode=0x5555555ba390) at xattr.c:35 > #5 0x00005555555698ea in print_inode_info (sbi=0x555555593d80 <gfsck>, node=0x5555555ba390, name=0) at mount.c:335 > #6 0x0000555555569a09 in print_node_info (sbi=0x555555593d80 <gfsck>, node_block=0x5555555ba390, verbose=0) at mount.c:354 > #7 0x0000555555566b55 in dump_node (sbi=0x555555593d80 <gfsck>, nid=4, force=0) at dump.c:507 > #8 0x0000555555559850 in do_dump (sbi=0x555555593d80 <gfsck>) at main.c:729 > #9 0x0000555555559ee1 in main (argc=4, argv=0x7fffffffe238) at main.c:892 > (gdb) > > . > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [f2fs-dev] [PATCH 2/2] fsck.f2fs: fix to check validation of i_xattr_nid 2020-11-02 1:31 ` Chao Yu @ 2020-11-02 2:39 ` Eric Biggers 2020-11-02 3:28 ` Chao Yu 0 siblings, 1 reply; 7+ messages in thread From: Eric Biggers @ 2020-11-02 2:39 UTC (permalink / raw) To: Chao Yu; +Cc: kilobyte, jaegeuk, linux-f2fs-devel On Mon, Nov 02, 2020 at 09:31:09AM +0800, Chao Yu wrote: > On 2020/11/1 7:48, Eric Biggers wrote: > > Hi Chao, > > > > On Tue, Apr 07, 2020 at 06:01:07PM +0800, Chao Yu wrote: > > > Otherwise, fsck.f2fs will access invalid memory address as below: > > > > > > - fsck_verify > > > - dump_node > > > - dump_file > > > - dump_inode_blk > > > - dump_xattr > > > - read_all_xattrs > > > - get_node_info > > > access &(F2FS_FSCK(sbi)->entries[nid]) > > > > > > Signed-off-by: Chao Yu <yuchao0@huawei.com> > > > --- > > > fsck/dump.c | 2 ++ > > > fsck/fsck.c | 8 ++++++++ > > > fsck/fsck.h | 3 +++ > > > fsck/mount.c | 8 +++++--- > > > fsck/xattr.c | 20 ++++++++++++++++++-- > > > 5 files changed, 36 insertions(+), 5 deletions(-) > > > > > > > This commit caused a regression where 'dump.f2fs -i <inode> <device>' > > now segfaults if the inode has any extended attributes. > > > > It's because read_all_xattrs() now calls fsck_sanity_check_nid(), which > > eventually dereferences f2fs_fsck::main_area_bitmap, which is NULL. > > > > I'm not sure what was intended here. > > Eric, could you please have a try with below commit: > > https://git.kernel.org/pub/scm/linux/kernel/git/chao/f2fs-tools.git/commit/?h=dev-test&id=aad80ed0099fb9530ae3af9789362353ff580252 > Works for me. I was wondering whether the fix would be more than that, but maybe not. - Eric _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [f2fs-dev] [PATCH 2/2] fsck.f2fs: fix to check validation of i_xattr_nid 2020-11-02 2:39 ` Eric Biggers @ 2020-11-02 3:28 ` Chao Yu 2020-11-02 16:04 ` Jaegeuk Kim 0 siblings, 1 reply; 7+ messages in thread From: Chao Yu @ 2020-11-02 3:28 UTC (permalink / raw) To: Eric Biggers, jaegeuk; +Cc: kilobyte, linux-f2fs-devel On 2020/11/2 10:39, Eric Biggers wrote: > On Mon, Nov 02, 2020 at 09:31:09AM +0800, Chao Yu wrote: >> On 2020/11/1 7:48, Eric Biggers wrote: >>> Hi Chao, >>> >>> On Tue, Apr 07, 2020 at 06:01:07PM +0800, Chao Yu wrote: >>>> Otherwise, fsck.f2fs will access invalid memory address as below: >>>> >>>> - fsck_verify >>>> - dump_node >>>> - dump_file >>>> - dump_inode_blk >>>> - dump_xattr >>>> - read_all_xattrs >>>> - get_node_info >>>> access &(F2FS_FSCK(sbi)->entries[nid]) >>>> >>>> Signed-off-by: Chao Yu <yuchao0@huawei.com> >>>> --- >>>> fsck/dump.c | 2 ++ >>>> fsck/fsck.c | 8 ++++++++ >>>> fsck/fsck.h | 3 +++ >>>> fsck/mount.c | 8 +++++--- >>>> fsck/xattr.c | 20 ++++++++++++++++++-- >>>> 5 files changed, 36 insertions(+), 5 deletions(-) >>>> >>> >>> This commit caused a regression where 'dump.f2fs -i <inode> <device>' >>> now segfaults if the inode has any extended attributes. >>> >>> It's because read_all_xattrs() now calls fsck_sanity_check_nid(), which >>> eventually dereferences f2fs_fsck::main_area_bitmap, which is NULL. >>> >>> I'm not sure what was intended here. >> >> Eric, could you please have a try with below commit: >> >> https://git.kernel.org/pub/scm/linux/kernel/git/chao/f2fs-tools.git/commit/?h=dev-test&id=aad80ed0099fb9530ae3af9789362353ff580252 >> > > Works for me. I was wondering whether the fix would be more than that, but > maybe not. Thanks for both report and test. I don't have any other concern on current solution, let's ask for Jaegeuk's opinion anyway. Jaegeuk, could you please check previous buggy fix and current fix? Thanks, > > - Eric > . > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [f2fs-dev] [PATCH 2/2] fsck.f2fs: fix to check validation of i_xattr_nid 2020-11-02 3:28 ` Chao Yu @ 2020-11-02 16:04 ` Jaegeuk Kim 0 siblings, 0 replies; 7+ messages in thread From: Jaegeuk Kim @ 2020-11-02 16:04 UTC (permalink / raw) To: Chao Yu; +Cc: Eric Biggers, kilobyte, linux-f2fs-devel On 11/02, Chao Yu wrote: > On 2020/11/2 10:39, Eric Biggers wrote: > > On Mon, Nov 02, 2020 at 09:31:09AM +0800, Chao Yu wrote: > > > On 2020/11/1 7:48, Eric Biggers wrote: > > > > Hi Chao, > > > > > > > > On Tue, Apr 07, 2020 at 06:01:07PM +0800, Chao Yu wrote: > > > > > Otherwise, fsck.f2fs will access invalid memory address as below: > > > > > > > > > > - fsck_verify > > > > > - dump_node > > > > > - dump_file > > > > > - dump_inode_blk > > > > > - dump_xattr > > > > > - read_all_xattrs > > > > > - get_node_info > > > > > access &(F2FS_FSCK(sbi)->entries[nid]) > > > > > > > > > > Signed-off-by: Chao Yu <yuchao0@huawei.com> > > > > > --- > > > > > fsck/dump.c | 2 ++ > > > > > fsck/fsck.c | 8 ++++++++ > > > > > fsck/fsck.h | 3 +++ > > > > > fsck/mount.c | 8 +++++--- > > > > > fsck/xattr.c | 20 ++++++++++++++++++-- > > > > > 5 files changed, 36 insertions(+), 5 deletions(-) > > > > > > > > > > > > > This commit caused a regression where 'dump.f2fs -i <inode> <device>' > > > > now segfaults if the inode has any extended attributes. > > > > > > > > It's because read_all_xattrs() now calls fsck_sanity_check_nid(), which > > > > eventually dereferences f2fs_fsck::main_area_bitmap, which is NULL. > > > > > > > > I'm not sure what was intended here. > > > > > > Eric, could you please have a try with below commit: > > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/chao/f2fs-tools.git/commit/?h=dev-test&id=aad80ed0099fb9530ae3af9789362353ff580252 > > > > > > > Works for me. I was wondering whether the fix would be more than that, but > > maybe not. > > Thanks for both report and test. > > I don't have any other concern on current solution, let's ask for Jaegeuk's > opinion anyway. > > Jaegeuk, could you please check previous buggy fix and current fix? Thanks. I applied the new fix. > > Thanks, > > > > > - Eric > > . > > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-11-02 16:04 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-04-07 10:01 [f2fs-dev] [PATCH 1/2] fsck.f2fs: fix to check validation of block address Chao Yu 2020-04-07 10:01 ` [f2fs-dev] [PATCH 2/2] fsck.f2fs: fix to check validation of i_xattr_nid Chao Yu 2020-10-31 23:48 ` Eric Biggers 2020-11-02 1:31 ` Chao Yu 2020-11-02 2:39 ` Eric Biggers 2020-11-02 3:28 ` Chao Yu 2020-11-02 16:04 ` Jaegeuk Kim
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).