From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1423185AbcBZVA0 (ORCPT ); Fri, 26 Feb 2016 16:00:26 -0500 Received: from mail-yw0-f177.google.com ([209.85.161.177]:36170 "EHLO mail-yw0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755002AbcBZVAY (ORCPT ); Fri, 26 Feb 2016 16:00:24 -0500 From: Gustavo Padovan To: Greg Kroah-Hartman Cc: linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org, dri-devel@lists.freedesktop.org, Daniel Stone , =?UTF-8?q?Arve=20Hj=C3=B8nnev=C3=A5g?= , Riley Andrews , Daniel Vetter , Rob Clark , Greg Hackmann , John Harrison , Maarten Lankhorst , Gustavo Padovan Subject: [PATCH] staging/android: refactor SYNC_IOC_FILE_INFO Date: Fri, 26 Feb 2016 18:00:10 -0300 Message-Id: <1456520410-28195-1-git-send-email-gustavo@padovan.org> X-Mailer: git-send-email 2.5.0 In-Reply-To: <1456511507-2534-4-git-send-email-gustavo@padovan.org> References: <1456511507-2534-4-git-send-email-gustavo@padovan.org> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Gustavo Padovan Change SYNC_IOC_FILE_INFO behaviour to avoid future API breaks and optimize buffer allocation. In the new approach the ioctl needs to be called twice to retrieve the array of fence_infos pointed by info->sync_fence_info. The first call should pass num_fences = 0, the kernel will then fill info->num_fences. Userspace receives back the number of fences and allocates a buffer size num_fences * sizeof(struct sync_fence_info) on info->sync_fence_info. It then call the ioctl again passing num_fences received in info->num_fences. The kernel checks if info->num_fences > 0 and if yes it fill info->sync_fence_info with an array containing all fence_infos. info->len now represents the length of the buffer sync_fence_info points to. Also, info->sync_fence_info was converted to __u64 pointer. An example userspace code would be: struct sync_file_info *info; int err, size, num_fences; info = malloc(sizeof(*info)); memset(info, 0, sizeof(*info)); err = ioctl(fd, SYNC_IOC_FILE_INFO, info); num_fences = info->num_fences; if (num_fences) { memset(info, 0, sizeof(*info)); size = sizeof(struct sync_fence_info) * num_fences; info->len = size; info->num_fences = num_fences; info->sync_fence_info = (uint64_t) calloc(num_fences, sizeof(struct sync_fence_info)); err = ioctl(fd, SYNC_IOC_FILE_INFO, info); } v2: fix fence_info memory leak Signed-off-by: Gustavo Padovan --- drivers/staging/android/sync.c | 52 +++++++++++++++++++++++++++++-------- drivers/staging/android/uapi/sync.h | 9 +++---- 2 files changed, 45 insertions(+), 16 deletions(-) diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c index dc5f382..2379f23 100644 --- a/drivers/staging/android/sync.c +++ b/drivers/staging/android/sync.c @@ -502,21 +502,22 @@ static int sync_fill_fence_info(struct fence *fence, void *data, int size) static long sync_file_ioctl_fence_info(struct sync_file *sync_file, unsigned long arg) { - struct sync_file_info *info; + struct sync_file_info in, *info; + struct sync_fence_info *fence_info = NULL; __u32 size; __u32 len = 0; int ret, i; - if (copy_from_user(&size, (void __user *)arg, sizeof(size))) + if (copy_from_user(&in, (void __user *)arg, sizeof(*info))) return -EFAULT; - if (size < sizeof(struct sync_file_info)) - return -EINVAL; + if (in.status || strcmp(in.name, "\0")) + return -EFAULT; - if (size > 4096) - size = 4096; + if (in.num_fences && !in.sync_fence_info) + return -EFAULT; - info = kzalloc(size, GFP_KERNEL); + info = kzalloc(sizeof(*info), GFP_KERNEL); if (!info) return -ENOMEM; @@ -525,14 +526,33 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file, if (info->status >= 0) info->status = !info->status; - info->num_fences = sync_file->num_fences; + /* + * Passing num_fences = 0 means that userspace want to know how + * many fences are in the sync_file to be able to allocate a buffer to + * fit all sync_fence_infos and call the ioctl again with the buffer + * assigned to info->sync_fence_info. The second call pass the + * num_fences value received in the first call. + */ + if (!in.num_fences) + goto no_fences; + + size = sync_file->num_fences * sizeof(*fence_info); + if (in.len != size) { + ret = -EFAULT; + goto out; + } - len = sizeof(struct sync_file_info); + fence_info = kzalloc(size, GFP_KERNEL); + if (!fence_info) { + ret = -ENOMEM; + goto out; + } for (i = 0; i < sync_file->num_fences; ++i) { struct fence *fence = sync_file->cbs[i].fence; - ret = sync_fill_fence_info(fence, (u8 *)info + len, size - len); + ret = sync_fill_fence_info(fence, (u8 *)fence_info + len, + size - len); if (ret < 0) goto out; @@ -540,14 +560,24 @@ static long sync_file_ioctl_fence_info(struct sync_file *sync_file, len += ret; } + if (copy_to_user((void __user *)in.sync_fence_info, fence_info, size)) { + ret = -EFAULT; + goto out; + } + info->len = len; + info->sync_fence_info = (__u64) in.sync_fence_info; + +no_fences: + info->num_fences = sync_file->num_fences; - if (copy_to_user((void __user *)arg, info, len)) + if (copy_to_user((void __user *)arg, info, sizeof(*info))) ret = -EFAULT; else ret = 0; out: + kfree(fence_info); kfree(info); return ret; diff --git a/drivers/staging/android/uapi/sync.h b/drivers/staging/android/uapi/sync.h index f0b41ce..9aad623 100644 --- a/drivers/staging/android/uapi/sync.h +++ b/drivers/staging/android/uapi/sync.h @@ -42,21 +42,20 @@ struct sync_fence_info { /** * struct sync_file_info - data returned from fence info ioctl - * @len: ioctl caller writes the size of the buffer its passing in. - * ioctl returns length of sync_file_info returned to - * userspace including pt_info. * @name: name of fence * @status: status of fence. 1: signaled 0:active <0:error * @num_fences number of fences in the sync_file + * @len: ioctl caller writes the size of the buffer its passing in. + * ioctl returns length of all fence_infos summed. * @sync_fence_info: array of sync_fence_info for every fence in the sync_file */ struct sync_file_info { - __u32 len; char name[32]; __s32 status; __u32 num_fences; + __u32 len; - __u8 sync_fence_info[0]; + __u64 sync_fence_info; }; #define SYNC_IOC_MAGIC '>' -- 2.5.0