[PATCH v5 4/6] drm/syncobj: Avoid temporary allocation in drm_syncobj_timeline_signal_ioctl

[Tvrtko Ursulin]

We can avoid one of the two temporary allocations if we read the userspace
supplied timeline points as we go along.

The only new complication is to unwind unused fence chains on the error
path, but even that code was already present in the function.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursu...@igalia.com>
Reviewed-by: Maíra Canal <mca...@igalia.com> #v1
---
v2:
 * Change back to copy_from_user due 32-bit ARM not implementing 64-bit
   get_user.

v3:
* Fix argument order mixup.
---
 drivers/gpu/drm/drm_syncobj.c | 43 ++++++++++++++---------------------
 1 file changed, 17 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 497009fc66f8..9968d9429d90 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -1579,10 +1579,10 @@ drm_syncobj_timeline_signal_ioctl(struct drm_device 
*dev, void *data,
                                  struct drm_file *file_private)
 {
        struct drm_syncobj_timeline_array *args = data;
+       uint64_t __user *points = u64_to_user_ptr(args->points);
+       uint32_t i, j, count = args->count_handles;
        struct drm_syncobj **syncobjs;
        struct dma_fence_chain **chains;
-       uint64_t *points;
-       uint32_t i, j;
        int ret;
 
        if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE))
@@ -1596,31 +1596,17 @@ drm_syncobj_timeline_signal_ioctl(struct drm_device 
*dev, void *data,
 
        ret = drm_syncobj_array_find(file_private,
                                     u64_to_user_ptr(args->handles),
-                                    args->count_handles,
+                                    count,
                                     &syncobjs);
        if (ret < 0)
                return ret;
 
-       points = kmalloc_array(args->count_handles, sizeof(*points),
-                              GFP_KERNEL);
-       if (!points) {
-               ret = -ENOMEM;
-               goto out;
-       }
-       if (!u64_to_user_ptr(args->points)) {
-               memset(points, 0, args->count_handles * sizeof(uint64_t));
-       } else if (copy_from_user(points, u64_to_user_ptr(args->points),
-                                 sizeof(uint64_t) * args->count_handles)) {
-               ret = -EFAULT;
-               goto err_points;
-       }
-
-       chains = kmalloc_array(args->count_handles, sizeof(void *), GFP_KERNEL);
+       chains = kmalloc_array(count, sizeof(void *), GFP_KERNEL);
        if (!chains) {
                ret = -ENOMEM;
-               goto err_points;
+               goto out;
        }
-       for (i = 0; i < args->count_handles; i++) {
+       for (i = 0; i < count; i++) {
                chains[i] = dma_fence_chain_alloc();
                if (!chains[i]) {
                        for (j = 0; j < i; j++)
@@ -1630,19 +1616,24 @@ drm_syncobj_timeline_signal_ioctl(struct drm_device 
*dev, void *data,
                }
        }
 
-       for (i = 0; i < args->count_handles; i++) {
+       for (i = 0; i < count; i++) {
                struct dma_fence *fence = dma_fence_get_stub();
+               u64 point = 0;
 
-               drm_syncobj_add_point(syncobjs[i], chains[i],
-                                     fence, points[i]);
+               if (points && copy_from_user(&point, points++, sizeof(point))) {
+                       ret =  -EFAULT;
+                       for (j = i; j < count; j++)
+                               dma_fence_chain_free(chains[j]);
+                       goto err_chains;
+               }
+
+               drm_syncobj_add_point(syncobjs[i], chains[i], fence, point);
                dma_fence_put(fence);
        }
 err_chains:
        kfree(chains);
-err_points:
-       kfree(points);
 out:
-       drm_syncobj_array_free(syncobjs, args->count_handles);
+       drm_syncobj_array_free(syncobjs, count);
 
        return ret;
 }
-- 
2.48.0