linux-kernel - Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250911090745.2940557-1-wangzijie1@honor.com>
Date: Thu, 11 Sep 2025 17:07:45 +0800
From: wangzijie <wangzijie1@...or.com>
To: <linux-f2fs-devel@...ts.sourceforge.net>
CC: <chao@...nel.org>, <feng.han@...or.com>, <jaegeuk@...nel.org>,
	<linux-kernel@...r.kernel.org>, <wangzijie1@...or.com>
Subject: Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents

> On 9/10/25 21:58, wangzijie wrote:
> > When the data layout is like this:
> > dnode1:                     dnode2:
> > [0]      A                  [0]    NEW_ADDR
> > [1]      A+1                [1]    0x0
> > ...                         ....
> > [1016]   A+1016
> > [1017]   B (B!=A+1017)      [1017] 0x0
> > 
> > We can build this kind of layout by following steps(with i_extra_isize:36):
> > ./f2fs_io write 1 0 1881 rand dsync testfile
> > ./f2fs_io write 1 1881 1 rand buffered testfile
> > ./f2fs_io fallocate 0 7708672 4096 testfile
> > 
> > And when we map first data block in dnode2, we will get wrong extent_info data:
> > map->m_len = 1
> > ofs = start_pgofs - map->m_lblk = 1882 - 1881 = 1
> > 
> > ei.fofs = start_pgofs = 1882
> > ei.len = map->m_len - ofs = 1 - 1 = 0
> > 
> > Fix it by skipping updating this kind of extent info.
> > 
> > Signed-off-by: wangzijie <wangzijie1@...or.com>
> > ---
> >  fs/f2fs/data.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index 7961e0ddf..b8bb71852 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -1649,6 +1649,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
> >  
> >  		switch (flag) {
> >  		case F2FS_GET_BLOCK_PRECACHE:
> > +			if (__is_valid_data_blkaddr(map->m_pblk) &&
> > +				start_pgofs - map->m_lblk == map->m_len)
> > +				map->m_flags &= ~F2FS_MAP_MAPPED;
> 
> It looks we missed to reset value for map variable in f2fs_precache_extents(),
> what do you think of this?
> 
> ---
>  fs/f2fs/file.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 1aae4361d0a8..2b14151d4130 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -3599,7 +3599,7 @@ static int f2fs_ioc_io_prio(struct file *filp, unsigned long arg)
>  int f2fs_precache_extents(struct inode *inode)
>  {
>  	struct f2fs_inode_info *fi = F2FS_I(inode);
> -	struct f2fs_map_blocks map;
> +	struct f2fs_map_blocks map = { 0 };
>  	pgoff_t m_next_extent;
>  	loff_t end;
>  	int err;
> @@ -3617,6 +3617,8 @@ int f2fs_precache_extents(struct inode *inode)
> 
>  	while (map.m_lblk < end) {
>  		map.m_len = end - map.m_lblk;
> +		map.m_pblk = 0;
> +		map.m_flags = 0;
> 
>  		f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
>  		err = f2fs_map_blocks(inode, &map, F2FS_GET_BLOCK_PRECACHE);
> -- 
> 2.49.0
> 
> Thanks,
> 
> >  			goto sync_out;
> >  		case F2FS_GET_BLOCK_BMAP:
> >  			map->m_pblk = 0;


We have already reset m_flags (map->m_flags = 0) in f2fs_map_blocks().

I think that this bug is caused by we missed to reset m_flags when we
goto next_dnode in below case：

Data layout is something like this:
dnode1:                     dnode2:
[0]      A                  [0]    NEW_ADDR
[1]      A+1                [1]    0x0
...
[1016]   A+1016
[1017]   B (B!=A+1017)      [1017] 0x0

we map the last block(valid blkaddr) in dnode1:
map->m_flags |= F2FS_MAP_MAPPED;
map->m_pblk = blkaddr(valid blkaddr);
map->m_len = 1;
then we goto next_dnode, meet the first block in dnode2(hole), goto sync_out:
map->m_flags & F2FS_MAP_MAPPED == true, and we make wrong blkaddr/len for extent_info.