diff mbox series

[v2,06/16] remap_range: move permission hooks out of do_clone_file_range()

Message ID 20231122122715.2561213-7-amir73il@gmail.com (mailing list archive)
State New, archived
Headers show
Series Tidy up file permission hooks | expand

Commit Message

Amir Goldstein Nov. 22, 2023, 12:27 p.m. UTC
In many of the vfs helpers, file permission hook is called before
taking sb_start_write(), making them "start-write-safe".
do_clone_file_range() is an exception to this rule.

do_clone_file_range() has two callers - vfs_clone_file_range() and
overlayfs. Move remap_verify_area() checks from do_clone_file_range()
out to vfs_clone_file_range() to make them "start-write-safe".

Overlayfs already has calls to rw_verify_area() with the same security
permission hooks as remap_verify_area() has.
The rest of the checks in remap_verify_area() are irrelevant for
overlayfs that calls do_clone_file_range() offset 0 and positive length.

This is needed for fanotify "pre content" events.

Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 fs/remap_range.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

Comments

Christoph Hellwig Nov. 23, 2023, 7:47 a.m. UTC | #1
Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>
Jan Kara Nov. 23, 2023, 4:52 p.m. UTC | #2
On Wed 22-11-23 14:27:05, Amir Goldstein wrote:
> In many of the vfs helpers, file permission hook is called before
> taking sb_start_write(), making them "start-write-safe".
> do_clone_file_range() is an exception to this rule.
> 
> do_clone_file_range() has two callers - vfs_clone_file_range() and
> overlayfs. Move remap_verify_area() checks from do_clone_file_range()
> out to vfs_clone_file_range() to make them "start-write-safe".
> 
> Overlayfs already has calls to rw_verify_area() with the same security
> permission hooks as remap_verify_area() has.
> The rest of the checks in remap_verify_area() are irrelevant for
> overlayfs that calls do_clone_file_range() offset 0 and positive length.
> 
> This is needed for fanotify "pre content" events.
> 
> Reviewed-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/remap_range.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/remap_range.c b/fs/remap_range.c
> index 87ae4f0dc3aa..42f79cb2b1b1 100644
> --- a/fs/remap_range.c
> +++ b/fs/remap_range.c
> @@ -385,14 +385,6 @@ loff_t do_clone_file_range(struct file *file_in, loff_t pos_in,
>  	if (!file_in->f_op->remap_file_range)
>  		return -EOPNOTSUPP;
>  
> -	ret = remap_verify_area(file_in, pos_in, len, false);
> -	if (ret)
> -		return ret;
> -
> -	ret = remap_verify_area(file_out, pos_out, len, true);
> -	if (ret)
> -		return ret;
> -
>  	ret = file_in->f_op->remap_file_range(file_in, pos_in,
>  			file_out, pos_out, len, remap_flags);
>  	if (ret < 0)
> @@ -410,6 +402,14 @@ loff_t vfs_clone_file_range(struct file *file_in, loff_t pos_in,
>  {
>  	loff_t ret;
>  
> +	ret = remap_verify_area(file_in, pos_in, len, false);
> +	if (ret)
> +		return ret;
> +
> +	ret = remap_verify_area(file_out, pos_out, len, true);
> +	if (ret)
> +		return ret;
> +
>  	file_start_write(file_out);
>  	ret = do_clone_file_range(file_in, pos_in, file_out, pos_out, len,
>  				  remap_flags);
> -- 
> 2.34.1
>
diff mbox series

Patch

diff --git a/fs/remap_range.c b/fs/remap_range.c
index 87ae4f0dc3aa..42f79cb2b1b1 100644
--- a/fs/remap_range.c
+++ b/fs/remap_range.c
@@ -385,14 +385,6 @@  loff_t do_clone_file_range(struct file *file_in, loff_t pos_in,
 	if (!file_in->f_op->remap_file_range)
 		return -EOPNOTSUPP;
 
-	ret = remap_verify_area(file_in, pos_in, len, false);
-	if (ret)
-		return ret;
-
-	ret = remap_verify_area(file_out, pos_out, len, true);
-	if (ret)
-		return ret;
-
 	ret = file_in->f_op->remap_file_range(file_in, pos_in,
 			file_out, pos_out, len, remap_flags);
 	if (ret < 0)
@@ -410,6 +402,14 @@  loff_t vfs_clone_file_range(struct file *file_in, loff_t pos_in,
 {
 	loff_t ret;
 
+	ret = remap_verify_area(file_in, pos_in, len, false);
+	if (ret)
+		return ret;
+
+	ret = remap_verify_area(file_out, pos_out, len, true);
+	if (ret)
+		return ret;
+
 	file_start_write(file_out);
 	ret = do_clone_file_range(file_in, pos_in, file_out, pos_out, len,
 				  remap_flags);