| Message ID | 20231122122715.2561213-7-amir73il@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Tidy up file permission hooks | expand |
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
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 --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);