From e9fde8664651a566df43c7439e27d59cc5d60460 Mon Sep 17 00:00:00 2001 From: Daniel Rosenberg Date: Wed, 2 Nov 2016 17:43:51 -0700 Subject: [PATCH] ion: Fix use after free during ION_IOC_ALLOC If a user happens to call ION_IOC_FREE during an ION_IOC_ALLOC on the just allocated id, and the copy_to_user fails, the cleanup code will attempt to free an already freed handle. This adds a wrapper for ion_alloc that adds an ion_handle_get to avoid this. Bug: 31568617 Change-Id: I476e5bd5372b5178a213f1fea143d270cf9361ed Signed-off-by: Daniel Rosenberg --- drivers/staging/android/ion/ion.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c index 8bbbb38dc7c41..63e6b7d795f47 100755 --- a/drivers/staging/android/ion/ion.c +++ b/drivers/staging/android/ion/ion.c @@ -507,9 +507,9 @@ static int ion_handle_add(struct ion_client *client, struct ion_handle *handle) return 0; } -struct ion_handle *ion_alloc(struct ion_client *client, size_t len, +static struct ion_handle *__ion_alloc(struct ion_client *client, size_t len, size_t align, unsigned int heap_id_mask, - unsigned int flags) + unsigned int flags, bool grab_handle) { struct ion_handle *handle; struct ion_device *dev = client->dev; @@ -604,6 +604,8 @@ struct ion_handle *ion_alloc(struct ion_client *client, size_t len, return handle; mutex_lock(&client->lock); + if (grab_handle) + ion_handle_get(handle); ret = ion_handle_add(client, handle); mutex_unlock(&client->lock); if (ret) { @@ -613,6 +615,13 @@ struct ion_handle *ion_alloc(struct ion_client *client, size_t len, return handle; } + +struct ion_handle *ion_alloc(struct ion_client *client, size_t len, + size_t align, unsigned int heap_id_mask, + unsigned int flags) +{ + return __ion_alloc(client, len, align, heap_id_mask, flags, false); +} EXPORT_SYMBOL(ion_alloc); static void ion_free_nolock(struct ion_client *client, struct ion_handle *handle) @@ -1488,10 +1497,10 @@ static long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) { struct ion_handle *handle; - handle = ion_alloc(client, data.allocation.len, + handle = __ion_alloc(client, data.allocation.len, data.allocation.align, data.allocation.heap_id_mask, - data.allocation.flags); + data.allocation.flags, true); if (IS_ERR(handle)) return PTR_ERR(handle); @@ -1568,11 +1577,15 @@ static long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) if (dir & _IOC_READ) { if (copy_to_user((void __user *)arg, &data, _IOC_SIZE(cmd))) { - if (cleanup_handle) + if (cleanup_handle) { ion_free(client, cleanup_handle); + ion_handle_put(cleanup_handle); + } return -EFAULT; } } + if (cleanup_handle) + ion_handle_put(cleanup_handle); return ret; }