On Wed 22-11-23 14:27:10, Amir Goldstein wrote:
> In many of the vfs helpers, the rw_verity_area() checks are called before
^^ verify
> taking sb_start_write(), making them "start-write-safe".
> do_iter_write() is an exception to this rule.
>
> do_iter_write() has two callers - vfs_iter_write() and vfs_writev().
> Move rw_verify_area() and other checks from do_iter_write() out to
> its callers to make them "start-write-safe".
>
> Move also the fsnotify_modify() hook to align with similar pattern
> used in vfs_write() and other vfs helpers.
>
> This is needed for fanotify "pre content" events.
>
> Suggested-by: Jan Kara <jack@xxxxxxx>
> Reviewed-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
Just one more nit below. Otherwise feel free to add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
> diff --git a/fs/read_write.c b/fs/read_write.c
> index 87ca50f16a23..6c40468efe19 100644
> --- a/fs/read_write.c
> +++ b/fs/read_write.c
> @@ -852,28 +852,10 @@ EXPORT_SYMBOL(vfs_iter_read);
> static ssize_t do_iter_write(struct file *file, struct iov_iter *iter,
> loff_t *pos, rwf_t flags)
> {
> - size_t tot_len;
> - ssize_t ret = 0;
> -
> - if (!(file->f_mode & FMODE_WRITE))
> - return -EBADF;
> - if (!(file->f_mode & FMODE_CAN_WRITE))
> - return -EINVAL;
> -
> - tot_len = iov_iter_count(iter);
> - if (!tot_len)
> - return 0;
> - ret = rw_verify_area(WRITE, file, pos, tot_len);
> - if (ret < 0)
> - return ret;
> -
> if (file->f_op->write_iter)
> - ret = do_iter_readv_writev(file, iter, pos, WRITE, flags);
> - else
> - ret = do_loop_readv_writev(file, iter, pos, WRITE, flags);
> - if (ret > 0)
> - fsnotify_modify(file);
> - return ret;
> + return do_iter_readv_writev(file, iter, pos, WRITE, flags);
> +
> + return do_loop_readv_writev(file, iter, pos, WRITE, flags);
> }
Since do_iter_write() is now trivial and has only two callers, one of which
has made sure file->f_op->write_iter != NULL, can we perhaps just fold this
into the callers? One less wrapper in the maze...
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
