Re: [f2fs-dev] [PATCH] f2fs: disallow mmap write and data-modifying fallocate on atomic files

[Chao Yu]

On 10/3/26 01:17, Daeho Jeong wrote:
> From: Daeho Jeong <daehojeong@xxxxxxxxxx>
> 
> Stores through a shared writable mapping and fallocate with
> FALLOC_FL_PUNCH_HOLE, FALLOC_FL_ZERO_RANGE, FALLOC_FL_COLLAPSE_RANGE or
> FALLOC_FL_INSERT_RANGE do not go through ->write_begin, so they bypass
> the COW inode and modify the original inode directly. Reject them while
> the file is in atomic write mode, as fallocate already does for pinned
> and compressed files. A write fault gets SIGBUS and fallocate gets
> -EOPNOTSUPP.
> 
> fallocate without these flags only preallocates blocks and stays
> allowed.
> 
> Fixes: 3db1de0e582c ("f2fs: change the current atomic write way")
> Signed-off-by: Daeho Jeong <daehojeong@xxxxxxxxxx>
> ---
>  fs/f2fs/file.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index ef4d218e694..56c686b8580 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -140,6 +140,10 @@ static vm_fault_t f2fs_vm_page_mkwrite(struct vm_fault *vmf)
>  		return VM_FAULT_SIGBUS;
>  	}
>  
> +	/* mmap stores bypass the COW inode and would break atomicity */
> +	if (f2fs_is_atomic_file(inode))
> +		return VM_FAULT_SIGBUS;

Will we suffer race issue due to the check w/o lock?

Thanks,

> +
>  	if (is_inode_flag_set(inode, FI_COMPRESS_RELEASED)) {
>  		err = -EIO;
>  		goto out;
> @@ -2113,9 +2117,11 @@ static long f2fs_fallocate(struct file *file, int mode,
>  
>  	/*
>  	 * Pinned file should not support partial truncation since the block
> -	 * can be used by applications.
> +	 * can be used by applications. Atomic files should not either, since
> +	 * these modify the original inode directly and break atomicity.
>  	 */
> -	if ((f2fs_compressed_file(inode) || f2fs_is_pinned_file(inode)) &&
> +	if ((f2fs_compressed_file(inode) || f2fs_is_pinned_file(inode) ||
> +	     f2fs_is_atomic_file(inode)) &&
>  		(mode & (FALLOC_FL_PUNCH_HOLE | FALLOC_FL_COLLAPSE_RANGE |
>  			FALLOC_FL_ZERO_RANGE | FALLOC_FL_INSERT_RANGE))) {
>  		ret = -EOPNOTSUPP;