[PATCH V0 RESEND] f2fs: fix error map extent flag while block not allocated

[PATCH V0 RESEND] f2fs: fix error map extent flag while block not allocated

[chenzhangqi79]

From: Zhangqi Chen <chenzhangqi@xiaomi.com>

In the function f2fs_fiemap, when returning the file
map extent flag, the flag of the delayed allocation
block is set to FIEMAP_EXTENT_UNWRITTEN . At the same
time, the phy address of the file map extent reassigned
to 0 because it is not a valid address.

In this way, the file map extent with address 0 and
FIEMAP_EXTENT_UNWRITTEN flag returned by ioctl, and
may be used for writing by userspace programs, thereby
destroying the superblock of the file system.

As mentioned in /Documentation/filesystems/fiemap.txt,
FIEMAP_EXTENT_UNWRITTEN should mean that the block has
been allocated but not filled with data. However, the
actual situation in f2fs is that there is no allocated
block, so it should be changed to FIEMAP_EXTENT_UNKNOWN
and FIEMAP_EXTENT_DELALLOC.

Co-developed-by: Zhijun Li <lizhijun3@xiaomi.com>
Signed-off-by: Zhijun Li <lizhijun3@xiaomi.com>
Signed-off-by: Zhangqi Chen <chenzhangqi@xiaomi.com>
---
 fs/f2fs/data.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 2ec0cfb41260..a945d1f1d40c 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -2023,7 +2023,7 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
 				size += F2FS_BLKSIZE;
 			}
 		} else if (map.m_flags & F2FS_MAP_DELALLOC) {
-			flags = FIEMAP_EXTENT_UNWRITTEN;
+			flags = FIEMAP_EXTENT_UNKNOWN | FIEMAP_EXTENT_DELALLOC;
 		}
 
 		start_blk += F2FS_BYTES_TO_BLK(size);
-- 
2.20.1

Re: [PATCH V0 RESEND] f2fs: fix error map extent flag while block not allocated

[Chao Yu]

Hi Zhangqi,

On 1/13/25 13:57, chenzhangqi79@163.com wrote:
> From: Zhangqi Chen <chenzhangqi@xiaomi.com>
> 
> In the function f2fs_fiemap, when returning the file
> map extent flag, the flag of the delayed allocation
> block is set to FIEMAP_EXTENT_UNWRITTEN . At the same
> time, the phy address of the file map extent reassigned
> to 0 because it is not a valid address.
> 
> In this way, the file map extent with address 0 and
> FIEMAP_EXTENT_UNWRITTEN flag returned by ioctl, and
> may be used for writing by userspace programs, thereby
> destroying the superblock of the file system.

I agree with you.

FYI, there is a previous patch as below:

https://lore.kernel.org/linux-f2fs-devel/20230405144359.930253-1-chao@kernel.org/

It seems such change will fail some testcases of xfstest, have you check
this patch w/ 009, 092 and 094 in tests/generic/?

Thanks,

> 
> As mentioned in /Documentation/filesystems/fiemap.txt,
> FIEMAP_EXTENT_UNWRITTEN should mean that the block has
> been allocated but not filled with data. However, the
> actual situation in f2fs is that there is no allocated
> block, so it should be changed to FIEMAP_EXTENT_UNKNOWN
> and FIEMAP_EXTENT_DELALLOC.
> 
> Co-developed-by: Zhijun Li <lizhijun3@xiaomi.com>
> Signed-off-by: Zhijun Li <lizhijun3@xiaomi.com>
> Signed-off-by: Zhangqi Chen <chenzhangqi@xiaomi.com>
> ---
>  fs/f2fs/data.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 2ec0cfb41260..a945d1f1d40c 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -2023,7 +2023,7 @@ int f2fs_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo,
>  				size += F2FS_BLKSIZE;
>  			}
>  		} else if (map.m_flags & F2FS_MAP_DELALLOC) {
> -			flags = FIEMAP_EXTENT_UNWRITTEN;
> +			flags = FIEMAP_EXTENT_UNKNOWN | FIEMAP_EXTENT_DELALLOC;
>  		}
>  
>  		start_blk += F2FS_BYTES_TO_BLK(size);