On 09/01/2019 07:51, Abdiel Janulgue wrote:
This simplifies adding new query item objects.
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@xxxxxxxxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/i915_query.c | 40 ++++++++++++++++++++-----------
1 file changed, 26 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c
index cbcb957b7141..b4f26605f617 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -10,12 +10,33 @@
#include "i915_query.h"
#include <uapi/drm/i915_drm.h>
+static int init_query_item_check(void* data_ptr, size_t data_sz,
void *ptr
data_ prefix is not ideal since this is not the trailing data but the
query header. Maybe query_ ?
+ 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(data_ptr, u64_to_user_ptr(query_item->data_ptr),
+ data_sz))
+ return -EFAULT;
Is lost type information a concern with copy_from_user.. let me check..
I am not sure TBH.. there seems to be type based object size check in
there but does it work when indirected via void*?
+
+ if (!access_ok(u64_to_user_ptr(query_item->data_ptr),
+ total_length))
+ return -EFAULT;
Kind of OK, but prevents the code for checking the whole user supplied
buffer in queries which carry the length field. (Engine discovery is one
of them.) Argument there was why check more than we will write - to
catch userspace accidentally passing in a smaller buffer than it thinks
it is passing in. But it is probably not an important enough concern to
prevent code consolidation.
So I think for me that leaves the question of type safety on the first
check.
+
+ 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;
+ u32 ret, slice_length, subslice_length, eu_length, total_length;
int ret;
if (query_item->flags != 0)
return -EINVAL;
@@ -33,23 +54,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;
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
