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