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.
---
 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 3aa5edaebdf9..53bd9b6de518 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(points++, &point, 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

Reply via email to