Re: [PATCH 1/4] io_uring: add validate_fixed_range() for validate fixed buffer

[Caleb Sander Mateos <csander@purestorage.com>]

On Tue, Mar 25, 2025 at 6:52 AM Ming Lei <ming.lei@redhat.com> wrote:
>
> Add helper of validate_fixed_range() for validating fixed buffer
> range.
>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>  io_uring/rsrc.c | 34 +++++++++++++++++++++++-----------
>  1 file changed, 23 insertions(+), 11 deletions(-)
>
> diff --git a/io_uring/rsrc.c b/io_uring/rsrc.c
> index 3f195e24777e..52e7492e863e 100644
> --- a/io_uring/rsrc.c
> +++ b/io_uring/rsrc.c
> @@ -1002,20 +1002,32 @@ int io_buffer_unregister_bvec(struct io_uring_cmd *cmd, unsigned int index,
>  }
>  EXPORT_SYMBOL_GPL(io_buffer_unregister_bvec);
>
> -static int io_import_fixed(int ddir, struct iov_iter *iter,
> -                          struct io_mapped_ubuf *imu,
> -                          u64 buf_addr, size_t len)
> +static int validate_fixed_range(u64 buf_addr, size_t len,
> +               const struct io_mapped_ubuf *imu)
>  {
>         u64 buf_end;
> -       size_t offset;
>
> -       if (WARN_ON_ONCE(!imu))
> -               return -EFAULT;
>         if (unlikely(check_add_overflow(buf_addr, (u64)len, &buf_end)))
>                 return -EFAULT;
>         /* not inside the mapped region */
>         if (unlikely(buf_addr < imu->ubuf || buf_end > (imu->ubuf + imu->len)))
>                 return -EFAULT;
> +
> +       return 0;

It's nice to avoid this code duplication. It looks like
validate_fixed_range() could return a bool instead and leave the
return code up to the caller, but I don't feel strongly either way.

Reviewed-by: Caleb Sander Mateos <csander@purestorage.com>

> +}
> +
> +static int io_import_fixed(int ddir, struct iov_iter *iter,
> +                          struct io_mapped_ubuf *imu,
> +                          u64 buf_addr, size_t len)
> +{
> +       size_t offset;
> +       int ret;
> +
> +       if (WARN_ON_ONCE(!imu))
> +               return -EFAULT;
> +       ret = validate_fixed_range(buf_addr, len, imu);
> +       if (ret)
> +               return ret;
>         if (!(imu->dir & (1 << ddir)))
>                 return -EFAULT;
>
> @@ -1305,12 +1317,12 @@ static int io_vec_fill_bvec(int ddir, struct iov_iter *iter,
>                 u64 buf_addr = (u64)(uintptr_t)iovec[iov_idx].iov_base;
>                 struct bio_vec *src_bvec;
>                 size_t offset;
> -               u64 buf_end;
> +               int ret;
> +
> +               ret = validate_fixed_range(buf_addr, iov_len, imu);
> +               if (unlikely(ret))
> +                       return ret;
>
> -               if (unlikely(check_add_overflow(buf_addr, (u64)iov_len, &buf_end)))
> -                       return -EFAULT;
> -               if (unlikely(buf_addr < imu->ubuf || buf_end > (imu->ubuf + imu->len)))
> -                       return -EFAULT;
>                 if (unlikely(!iov_len))
>                         return -EFAULT;
>                 if (unlikely(check_add_overflow(total_len, iov_len, &total_len)))
> --
> 2.47.0
>