[Intel-gfx] [PATCH v7 6/6] drm/i915: expose rcs topology through query uAPI

[Chris Wilson]

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