Re: [PATCH v2 12/16] fs: move permission hook out of do_iter_read()

[Jan Kara <jack@suse.cz>]

On Wed 22-11-23 14:27:11, Amir Goldstein wrote:
> We recently moved fsnotify hook, rw_verify_area() and other checks from
> do_iter_write() out to its two callers.
> 
> for consistency, do the same thing for do_iter_read() - move the
  ^^^ For

> rw_verify_area() checks and fsnotify hook to the callers vfs_iter_read()
> and vfs_readv().
> 
> This aligns those vfs helpers with the pattern used in vfs_read() and
> vfs_iocb_iter_read() and the vfs write helpers, where all the checks are
> in the vfs helpers and the do_* or call_* helpers do the work.
> 
> This is needed for fanotify "pre content" events.
> 
> Suggested-by: Jan Kara <jack@suse.cz>
> Reviewed-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>

Also one nit below. Otherwise feel free to add:

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

> +/*
> + * Low-level helpers don't perform rw sanity checks.
> + * The caller is responsible for that.
> + */
>  static ssize_t do_iter_read(struct file *file, struct iov_iter *iter,
> -		loff_t *pos, rwf_t flags)
> +			    loff_t *pos, rwf_t flags)
> +{
> +	if (file->f_op->read_iter)
> +		return do_iter_readv_writev(file, iter, pos, READ, flags);
> +
> +	return do_loop_readv_writev(file, iter, pos, READ, flags);
> +}

Similarly as with do_iter_write() I don't think there's much point in this
helper when there's actually only one real user, the other one can just
call do_iter_readv_writev().

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR