From db866e3ded294aac1059d45d6497cf1bdbebaa04 Mon Sep 17 00:00:00 2001 From: Colin Cross <ccross@android.com> Date: Fri, 13 Dec 2013 19:26:16 -0800 Subject: [PATCH] ion: clean up ioctls Convert the ion ioctls to use _IOW instead of _IOWR where appropriate, and factor out the copy_from_user and copy_to_user based on the _IOC_DIR bits. For the existing incorrect ioctls, add a function to wrap _IOC_DIR to return the corrected value. Signed-off-by: Colin Cross <ccross@android.com> Signed-off-by: John Stultz <john.stultz@linaro.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> --- drivers/staging/android/ion/ion.c | 110 ++++++++++++++++-------------- 1 file changed, 59 insertions(+), 51 deletions(-) diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c index 57698e6e4ae6..fc77aa638a9e 100644 --- a/drivers/staging/android/ion/ion.c +++ b/drivers/staging/android/ion/ion.c @@ -1157,41 +1157,65 @@ static int ion_sync_for_device(struct ion_client *client, int fd) return 0; } +/* fix up the cases where the ioctl direction bits are incorrect */ +static unsigned int ion_ioctl_dir(unsigned int cmd) +{ + switch (cmd) { + case ION_IOC_SYNC: + case ION_IOC_FREE: + case ION_IOC_CUSTOM: + return _IOC_WRITE; + default: + return _IOC_DIR(cmd); + } +} + static long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) { struct ion_client *client = filp->private_data; + struct ion_device *dev = client->dev; + struct ion_handle *cleanup_handle = NULL; + int ret = 0; + unsigned int dir; + + union { + struct ion_fd_data fd; + struct ion_allocation_data allocation; + struct ion_handle_data handle; + struct ion_custom_data custom; + } data; + + dir = ion_ioctl_dir(cmd); + + if (_IOC_SIZE(cmd) > sizeof(data)) + return -EINVAL; + + if (dir & _IOC_WRITE) + if (copy_from_user(&data, (void __user *)arg, _IOC_SIZE(cmd))) + return -EFAULT; switch (cmd) { case ION_IOC_ALLOC: { - struct ion_allocation_data data; struct ion_handle *handle; - if (copy_from_user(&data, (void __user *)arg, sizeof(data))) - return -EFAULT; - handle = ion_alloc(client, data.len, data.align, - data.heap_id_mask, data.flags); - + handle = ion_alloc(client, data.allocation.len, + data.allocation.align, + data.allocation.heap_id_mask, + data.allocation.flags); if (IS_ERR(handle)) return PTR_ERR(handle); - data.handle = handle->id; + data.allocation.handle = handle->id; - if (copy_to_user((void __user *)arg, &data, sizeof(data))) { - ion_free(client, handle); - return -EFAULT; - } + cleanup_handle = handle; break; } case ION_IOC_FREE: { - struct ion_handle_data data; struct ion_handle *handle; - if (copy_from_user(&data, (void __user *)arg, - sizeof(struct ion_handle_data))) - return -EFAULT; - handle = ion_handle_get_by_id(client, data.handle); + handle = ion_handle_get_by_id(client, data.handle.handle); if (IS_ERR(handle)) return PTR_ERR(handle); ion_free(client, handle); @@ -1201,68 +1225,52 @@ static long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) case ION_IOC_SHARE: case ION_IOC_MAP: { - struct ion_fd_data data; struct ion_handle *handle; - if (copy_from_user(&data, (void __user *)arg, sizeof(data))) - return -EFAULT; - handle = ion_handle_get_by_id(client, data.handle); + handle = ion_handle_get_by_id(client, data.handle.handle); if (IS_ERR(handle)) return PTR_ERR(handle); - data.fd = ion_share_dma_buf_fd(client, handle); + data.fd.fd = ion_share_dma_buf_fd(client, handle); ion_handle_put(handle); - if (copy_to_user((void __user *)arg, &data, sizeof(data))) - return -EFAULT; - if (data.fd < 0) - return data.fd; + if (data.fd.fd < 0) + ret = data.fd.fd; break; } case ION_IOC_IMPORT: { - struct ion_fd_data data; struct ion_handle *handle; - int ret = 0; - if (copy_from_user(&data, (void __user *)arg, - sizeof(struct ion_fd_data))) - return -EFAULT; - handle = ion_import_dma_buf(client, data.fd); + handle = ion_import_dma_buf(client, data.fd.fd); if (IS_ERR(handle)) ret = PTR_ERR(handle); else - data.handle = handle->id; - - if (copy_to_user((void __user *)arg, &data, - sizeof(struct ion_fd_data))) - return -EFAULT; - if (ret < 0) - return ret; + data.handle.handle = handle->id; break; } case ION_IOC_SYNC: { - struct ion_fd_data data; - if (copy_from_user(&data, (void __user *)arg, - sizeof(struct ion_fd_data))) - return -EFAULT; - ion_sync_for_device(client, data.fd); + ret = ion_sync_for_device(client, data.fd.fd); break; } case ION_IOC_CUSTOM: { - struct ion_device *dev = client->dev; - struct ion_custom_data data; - if (!dev->custom_ioctl) return -ENOTTY; - if (copy_from_user(&data, (void __user *)arg, - sizeof(struct ion_custom_data))) - return -EFAULT; - return dev->custom_ioctl(client, data.cmd, data.arg); + ret = dev->custom_ioctl(client, data.custom.cmd, + data.custom.arg); + break; } default: return -ENOTTY; } - return 0; + + if (dir & _IOC_READ) { + if (copy_to_user((void __user *)arg, &data, _IOC_SIZE(cmd))) { + if (cleanup_handle) + ion_free(client, cleanup_handle); + return -EFAULT; + } + } + return ret; } static int ion_release(struct inode *inode, struct file *file) -- 2.30.9