[f2fs-dev] [PATCH 1/2] fsck.f2fs: fix to check validation of block address

[f2fs-dev] [PATCH 1/2] fsck.f2fs: fix to check validation of block address

[Chao Yu]

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

[f2fs-dev] [PATCH 2/2] fsck.f2fs: fix to check validation of i_xattr_nid

[Chao Yu]

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

Re: [f2fs-dev] [PATCH 2/2] fsck.f2fs: fix to check validation of i_xattr_nid

[Eric Biggers]

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

Re: [f2fs-dev] [PATCH 2/2] fsck.f2fs: fix to check validation of i_xattr_nid

[Chao Yu]

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

Re: [f2fs-dev] [PATCH 2/2] fsck.f2fs: fix to check validation of i_xattr_nid

[Eric Biggers]

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

Re: [f2fs-dev] [PATCH 2/2] fsck.f2fs: fix to check validation of i_xattr_nid

[Chao Yu]

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

Re: [f2fs-dev] [PATCH 2/2] fsck.f2fs: fix to check validation of i_xattr_nid

[Jaegeuk Kim]

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