[f2fs-dev] [PATCH v2 1/2] f2fs: move the conditional statement after holding the inode lock in f2fs_move_file_range()

[f2fs-dev] [PATCH v2 1/2] f2fs: move the conditional statement after holding the inode lock in f2fs_move_file_range()

[Yangtao Li via Linux-f2fs-devel]

For judging the inode flag state, the inode lock must be held.
BTW, add compressd file check and to avoid 'if' nesting.

Cc: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Fixes: 4dd6f977fc77 ("f2fs: support an ioctl to move a range of data blocks")
Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
v2:
-add unlock
 fs/f2fs/file.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 78aa8cff4b41..42a9b683118c 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -2790,9 +2790,6 @@ static int f2fs_move_file_range(struct file *file_in, loff_t pos_in,
 	if (!S_ISREG(src->i_mode) || !S_ISREG(dst->i_mode))
 		return -EINVAL;
 
-	if (IS_ENCRYPTED(src) || IS_ENCRYPTED(dst))
-		return -EOPNOTSUPP;
-
 	if (pos_out < 0 || pos_in < 0)
 		return -EINVAL;
 
@@ -2804,10 +2801,19 @@ static int f2fs_move_file_range(struct file *file_in, loff_t pos_in,
 	}
 
 	inode_lock(src);
-	if (src != dst) {
+	if (src != dst && !inode_trylock(dst)) {
 		ret = -EBUSY;
-		if (!inode_trylock(dst))
-			goto out;
+		goto out;
+	}
+
+	if (IS_ENCRYPTED(src) || IS_ENCRYPTED(dst)) {
+		ret = -EOPNOTSUPP;
+		goto out_unlock;
+	}
+
+	if (f2fs_compressed_file(src) || f2fs_compressed_file(dst)) {
+		ret = -EOPNOTSUPP;
+		goto out_unlock;
 	}
 
 	ret = -EINVAL;
-- 
2.39.0



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH v2 2/2] f2fs: move the conditional statement after holding the inode lock in f2fs_fallocate()

[Yangtao Li via Linux-f2fs-devel]

For judging the inode flag state, the inode lock must be held.

Cc: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Fixes: fcc85a4d86b5 ("f2fs crypto: activate encryption support for fs APIs")
Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
v2:
-add unlock
 fs/f2fs/file.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 42a9b683118c..0dbbcb406d3f 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1801,9 +1801,18 @@ static long f2fs_fallocate(struct file *file, int mode,
 	if (!S_ISREG(inode->i_mode))
 		return -EINVAL;
 
+	if (mode & ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE |
+			FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_ZERO_RANGE |
+			FALLOC_FL_INSERT_RANGE))
+		return -EOPNOTSUPP;
+
+	inode_lock(inode);
+
 	if (IS_ENCRYPTED(inode) &&
-		(mode & (FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE)))
+		(mode & (FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_INSERT_RANGE))) {
+		inode_unlock(inode);
 		return -EOPNOTSUPP;
+	}
 
 	/*
 	 * Pinned file should not support partial truncation since the block
@@ -1811,15 +1820,10 @@ static long f2fs_fallocate(struct file *file, int mode,
 	 */
 	if ((f2fs_compressed_file(inode) || f2fs_is_pinned_file(inode)) &&
 		(mode & (FALLOC_FL_PUNCH_HOLE | FALLOC_FL_COLLAPSE_RANGE |
-			FALLOC_FL_ZERO_RANGE | FALLOC_FL_INSERT_RANGE)))
-		return -EOPNOTSUPP;
-
-	if (mode & ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE |
-			FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_ZERO_RANGE |
-			FALLOC_FL_INSERT_RANGE))
+			FALLOC_FL_ZERO_RANGE | FALLOC_FL_INSERT_RANGE))) {
+		inode_unlock(inode);
 		return -EOPNOTSUPP;
-
-	inode_lock(inode);
+	}
 
 	ret = file_modified(file);
 	if (ret)
-- 
2.39.0



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH v2 1/2] f2fs: move the conditional statement after holding the inode lock in f2fs_move_file_range()

[Chao Yu]

On 2023/5/6 22:42, Yangtao Li wrote:
> For judging the inode flag state, the inode lock must be held.
> BTW, add compressd file check and to avoid 'if' nesting.

Please describe what's the detail problem if we check the flag w/o inode
lock.

Can we use one single patch to fix all similar issues?

Thanks,

> 
> Cc: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> Fixes: 4dd6f977fc77 ("f2fs: support an ioctl to move a range of data blocks")
> Signed-off-by: Yangtao Li <frank.li@vivo.com>
> ---
> v2:
> -add unlock
>   fs/f2fs/file.c | 18 ++++++++++++------
>   1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 78aa8cff4b41..42a9b683118c 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -2790,9 +2790,6 @@ static int f2fs_move_file_range(struct file *file_in, loff_t pos_in,
>   	if (!S_ISREG(src->i_mode) || !S_ISREG(dst->i_mode))
>   		return -EINVAL;
>   
> -	if (IS_ENCRYPTED(src) || IS_ENCRYPTED(dst))
> -		return -EOPNOTSUPP;
> -
>   	if (pos_out < 0 || pos_in < 0)
>   		return -EINVAL;
>   
> @@ -2804,10 +2801,19 @@ static int f2fs_move_file_range(struct file *file_in, loff_t pos_in,
>   	}
>   
>   	inode_lock(src);
> -	if (src != dst) {
> +	if (src != dst && !inode_trylock(dst)) {
>   		ret = -EBUSY;
> -		if (!inode_trylock(dst))
> -			goto out;
> +		goto out;
> +	}
> +
> +	if (IS_ENCRYPTED(src) || IS_ENCRYPTED(dst)) {
> +		ret = -EOPNOTSUPP;
> +		goto out_unlock;
> +	}
> +
> +	if (f2fs_compressed_file(src) || f2fs_compressed_file(dst)) {
> +		ret = -EOPNOTSUPP;
> +		goto out_unlock;
>   	}
>   
>   	ret = -EINVAL;


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel