| Message ID | 20190116085926.24420-1-abdiel.janulgue@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/i915/query: Split out query item checks | expand |
On 16/01/2019 08:59, Abdiel Janulgue wrote: > This simplifies adding new query item objects. > > v2: Use query_hdr (Tvrtko, Chris). > int instead of u32 in return (Tvrtko) > > Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_query.c | 39 ++++++++++++++++++++----------- > 1 file changed, 26 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c > index cbcb957b7141..b2449cffda51 100644 > --- a/drivers/gpu/drm/i915/i915_query.c > +++ b/drivers/gpu/drm/i915/i915_query.c > @@ -10,12 +10,34 @@ > #include "i915_query.h" > #include <uapi/drm/i915_drm.h> > > +static int init_query_item_check(void *query_hdr, size_t data_sz, AFAIR I think we talked about renaming both header related fields, so query_hdr and query_sz, no? The rest looks good to me. Although now I am also thinking about how descriptive "init_query_item_check" is.. so am tempted to suggest some more obvious name for it, maybe even shorter. :) copy_query_item? Regards, Tvrtko > + u32 total_length, > + struct drm_i915_query_item *query_item) > +{ > + if (query_item->length == 0) > + return total_length; > + > + if (query_item->length < total_length) > + return -EINVAL; > + > + if (copy_from_user(query_hdr, u64_to_user_ptr(query_item->data_ptr), > + data_sz)) > + return -EFAULT; > + > + if (!access_ok(u64_to_user_ptr(query_item->data_ptr), > + total_length)) > + return -EFAULT; > + > + return 0; > +} > + > static int query_topology_info(struct drm_i915_private *dev_priv, > struct drm_i915_query_item *query_item) > { > const struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu; > struct drm_i915_query_topology_info topo; > u32 slice_length, subslice_length, eu_length, total_length; > + int ret; > > if (query_item->flags != 0) > return -EINVAL; > @@ -33,23 +55,14 @@ static int query_topology_info(struct drm_i915_private *dev_priv, > > total_length = sizeof(topo) + slice_length + subslice_length + eu_length; > > - if (query_item->length == 0) > - return total_length; > - > - if (query_item->length < total_length) > - return -EINVAL; > - > - if (copy_from_user(&topo, u64_to_user_ptr(query_item->data_ptr), > - sizeof(topo))) > - return -EFAULT; > + ret = init_query_item_check(&topo, sizeof(topo), total_length, > + query_item); > + if (ret != 0) > + return ret; > > if (topo.flags != 0) > return -EINVAL; > > - if (!access_ok(u64_to_user_ptr(query_item->data_ptr), > - total_length)) > - return -EFAULT; > - > memset(&topo, 0, sizeof(topo)); > topo.max_slices = sseu->max_slices; > topo.max_subslices = sseu->max_subslices; >
diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c index cbcb957b7141..b2449cffda51 100644 --- a/drivers/gpu/drm/i915/i915_query.c +++ b/drivers/gpu/drm/i915/i915_query.c @@ -10,12 +10,34 @@ #include "i915_query.h" #include <uapi/drm/i915_drm.h> +static int init_query_item_check(void *query_hdr, size_t data_sz, + u32 total_length, + struct drm_i915_query_item *query_item) +{ + if (query_item->length == 0) + return total_length; + + if (query_item->length < total_length) + return -EINVAL; + + if (copy_from_user(query_hdr, u64_to_user_ptr(query_item->data_ptr), + data_sz)) + return -EFAULT; + + if (!access_ok(u64_to_user_ptr(query_item->data_ptr), + total_length)) + return -EFAULT; + + return 0; +} + static int query_topology_info(struct drm_i915_private *dev_priv, struct drm_i915_query_item *query_item) { const struct sseu_dev_info *sseu = &RUNTIME_INFO(dev_priv)->sseu; struct drm_i915_query_topology_info topo; u32 slice_length, subslice_length, eu_length, total_length; + int ret; if (query_item->flags != 0) return -EINVAL; @@ -33,23 +55,14 @@ static int query_topology_info(struct drm_i915_private *dev_priv, total_length = sizeof(topo) + slice_length + subslice_length + eu_length; - if (query_item->length == 0) - return total_length; - - if (query_item->length < total_length) - return -EINVAL; - - if (copy_from_user(&topo, u64_to_user_ptr(query_item->data_ptr), - sizeof(topo))) - return -EFAULT; + ret = init_query_item_check(&topo, sizeof(topo), total_length, + query_item); + if (ret != 0) + return ret; if (topo.flags != 0) return -EINVAL; - if (!access_ok(u64_to_user_ptr(query_item->data_ptr), - total_length)) - return -EFAULT; - memset(&topo, 0, sizeof(topo)); topo.max_slices = sseu->max_slices; topo.max_subslices = sseu->max_subslices;
This simplifies adding new query item objects. v2: Use query_hdr (Tvrtko, Chris). int instead of u32 in return (Tvrtko) Signed-off-by: Abdiel Janulgue <abdiel.janulgue@linux.intel.com> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> --- drivers/gpu/drm/i915/i915_query.c | 39 ++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 13 deletions(-)