[igt-dev] [PATCH i-g-t v8 3/3] tests: add i915 query tests

[Chris Wilson]

Quoting Tvrtko Ursulin (2018-03-09 14:21:45)
> 
> On 09/03/2018 14:07, Lionel Landwerlin wrote:
> > On 09/03/18 13:23, Tvrtko Ursulin wrote:
> >>> +    /* Test a couple of invalid pointers. */
> >>> +    i915_query_items_err(fd, (void *) ULONG_MAX, 1, EFAULT);
> >>> +    i915_query_items_err(fd, (void *) 0, 1, EFAULT);
> >>
> >> Also need one where struct drm_i915_query itself it invalid or not 
> >> mapped.
> > 
> > Doesn't the tests below (unmapping the items pointer) cover that case?
> 
> Don't think so. You need to hit the failure in :
> 
> +int i915_query_ioctl(struct drm_device *dev, void *data, struct 
> drm_file *file)
> +{
> +       struct drm_i915_private *dev_priv = to_i915(dev);
> +       struct drm_i915_query *args = data;
> +       struct drm_i915_query_item __user *user_item_ptr =
> +               u64_to_user_ptr(args->items_ptr);
> 
> This dereference here. Or is DRM core already covering for that? I mean 
> the fact...
> 
> +static int
> +__i915_query_items(int fd, struct drm_i915_query_item *items, uint32_t 
> n_items)
> +{
> +    struct drm_i915_query q = {
> +        .num_items = n_items,
> +        .items_ptr = to_user_pointer(items),
> +    };
> +    return __i915_query(fd, &q);
> 
> ... &q always points to good memory?

Yes, in this case struct drm_i915_query is already checked and copied by
drm_ioctl() onto the stack/heap.

Sensible suggestion for a test though, it's one that is rarely checked.
-Chris