Quoting Lionel Landwerlin (2018-01-16 19:18:24)
> diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c
> index 51736af7f573..038f292e1f2a 100644
> --- a/drivers/gpu/drm/i915/i915_query.c
> +++ b/drivers/gpu/drm/i915/i915_query.c
> @@ -25,8 +25,102 @@
> #include "i915_drv.h"
> #include <uapi/drm/i915_drm.h>
>
> +static int copy_query_data(struct drm_i915_query_item *query_item,
> + const void *item_ptr, u32 item_length,
> + const void *data_ptr, u32 data_length)
> +{
> + u32 total_length = item_length + data_length;
BUG_ON(add_overflows(item_length, data_length)) ?
> +
> + if (query_item->length == 0)
> + return total_length;
> +
> + if (query_item->length < total_length)
> + return -EINVAL;
> +
> + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr),
> + item_ptr, item_length))
> + return -EFAULT;
> +
> + if (copy_to_user(u64_to_user_ptr(query_item->data_ptr + item_length),
> + data_ptr, data_length))
> + return -EFAULT;
Unchecked overflow (data_ptr + item_length) here.
if (!access_ok(VERIFY_WRITE, u64_to_user_ptr(query_item->data_ptr), total_length))
return -EFAULT;
Would cover it, and then if you wanted you could then use
__copy_to_user().
> +
> + return total_length;
> +}
> +
> +static int query_slice_info(struct drm_i915_private *dev_priv,
> + struct drm_i915_query_item *query_item)
> +{
> + const struct sseu_dev_info *sseu = &INTEL_INFO(dev_priv)->sseu;
> + struct drm_i915_query_slice_info slice_info;
> +
> + if (sseu->max_slices == 0)
> + return -ENODEV;
Brr. This would then abort all queries not just this one, that doesn't
seem very user friendly. EFAULT are definite abort, can't continue, this
is just factual.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
