--- /dev/null
+From ghackmann@android.com Fri Aug 31 13:10:56 2018
+From: Greg Hackmann <ghackmann@android.com>
+Date: Fri, 31 Aug 2018 13:06:27 -0700
+Subject: staging: android: ion: fix ION_IOC_{MAP,SHARE} use-after-free
+To: linux-kernel@vger.kernel.org
+Cc: Laura Abbott <labbott@redhat.com>, Sumit Semwal <sumit.semwal@linaro.org>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, devel@driverdev.osuosl.org, kernel-team@android.com, Greg Hackmann <ghackmann@google.com>, stable@vger.kernel.org
+Message-ID: <20180831200627.59712-1-ghackmann@google.com>
+
+From: Greg Hackmann <ghackmann@android.com>
+
+This patch is 4.9.y only. Kernels 4.12 and later are unaffected, since
+all the underlying ion_handle infrastructure has been ripped out.
+
+The ION_IOC_{MAP,SHARE} ioctls drop and reacquire client->lock several
+times while operating on one of the client's ion_handles. This creates
+windows where userspace can call ION_IOC_FREE on the same client with
+the same handle, and effectively make the kernel drop its own reference.
+For example:
+
+- thread A: ION_IOC_ALLOC creates an ion_handle with refcount 1
+- thread A: starts ION_IOC_MAP and increments the refcount to 2
+- thread B: ION_IOC_FREE decrements the refcount to 1
+- thread B: ION_IOC_FREE decrements the refcount to 0 and frees the
+ handle
+- thread A: continues ION_IOC_MAP with a dangling ion_handle * to
+ freed memory
+
+Fix this by holding client->lock for the duration of
+ION_IOC_{MAP,SHARE}, preventing the concurrent ION_IOC_FREE. Also
+remove ion_handle_get_by_id(), since there's literally no way to use it
+safely.
+
+Cc: stable@vger.kernel.org # v4.11-
+Signed-off-by: Greg Hackmann <ghackmann@google.com>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+---
+ drivers/staging/android/ion/ion-ioctl.c | 12 +++++---
+ drivers/staging/android/ion/ion.c | 48 +++++++++++++++++++-------------
+ drivers/staging/android/ion/ion_priv.h | 6 ++--
+ 3 files changed, 40 insertions(+), 26 deletions(-)
+
+--- a/drivers/staging/android/ion/ion-ioctl.c
++++ b/drivers/staging/android/ion/ion-ioctl.c
+@@ -128,11 +128,15 @@ long ion_ioctl(struct file *filp, unsign
+ {
+ struct ion_handle *handle;
+
+- handle = ion_handle_get_by_id(client, data.handle.handle);
+- if (IS_ERR(handle))
++ mutex_lock(&client->lock);
++ handle = ion_handle_get_by_id_nolock(client, data.handle.handle);
++ if (IS_ERR(handle)) {
++ mutex_unlock(&client->lock);
+ return PTR_ERR(handle);
+- data.fd.fd = ion_share_dma_buf_fd(client, handle);
+- ion_handle_put(handle);
++ }
++ data.fd.fd = ion_share_dma_buf_fd_nolock(client, handle);
++ ion_handle_put_nolock(handle);
++ mutex_unlock(&client->lock);
+ if (data.fd.fd < 0)
+ ret = data.fd.fd;
+ break;
+--- a/drivers/staging/android/ion/ion.c
++++ b/drivers/staging/android/ion/ion.c
+@@ -352,18 +352,6 @@ struct ion_handle *ion_handle_get_by_id_
+ return handle ? handle : ERR_PTR(-EINVAL);
+ }
+
+-struct ion_handle *ion_handle_get_by_id(struct ion_client *client,
+- int id)
+-{
+- struct ion_handle *handle;
+-
+- mutex_lock(&client->lock);
+- handle = ion_handle_get_by_id_nolock(client, id);
+- mutex_unlock(&client->lock);
+-
+- return handle;
+-}
+-
+ static bool ion_handle_validate(struct ion_client *client,
+ struct ion_handle *handle)
+ {
+@@ -1029,24 +1017,28 @@ static struct dma_buf_ops dma_buf_ops =
+ .kunmap = ion_dma_buf_kunmap,
+ };
+
+-struct dma_buf *ion_share_dma_buf(struct ion_client *client,
+- struct ion_handle *handle)
++static struct dma_buf *__ion_share_dma_buf(struct ion_client *client,
++ struct ion_handle *handle,
++ bool lock_client)
+ {
+ DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
+ struct ion_buffer *buffer;
+ struct dma_buf *dmabuf;
+ bool valid_handle;
+
+- mutex_lock(&client->lock);
++ if (lock_client)
++ mutex_lock(&client->lock);
+ valid_handle = ion_handle_validate(client, handle);
+ if (!valid_handle) {
+ WARN(1, "%s: invalid handle passed to share.\n", __func__);
+- mutex_unlock(&client->lock);
++ if (lock_client)
++ mutex_unlock(&client->lock);
+ return ERR_PTR(-EINVAL);
+ }
+ buffer = handle->buffer;
+ ion_buffer_get(buffer);
+- mutex_unlock(&client->lock);
++ if (lock_client)
++ mutex_unlock(&client->lock);
+
+ exp_info.ops = &dma_buf_ops;
+ exp_info.size = buffer->size;
+@@ -1061,14 +1053,21 @@ struct dma_buf *ion_share_dma_buf(struct
+
+ return dmabuf;
+ }
++
++struct dma_buf *ion_share_dma_buf(struct ion_client *client,
++ struct ion_handle *handle)
++{
++ return __ion_share_dma_buf(client, handle, true);
++}
+ EXPORT_SYMBOL(ion_share_dma_buf);
+
+-int ion_share_dma_buf_fd(struct ion_client *client, struct ion_handle *handle)
++static int __ion_share_dma_buf_fd(struct ion_client *client,
++ struct ion_handle *handle, bool lock_client)
+ {
+ struct dma_buf *dmabuf;
+ int fd;
+
+- dmabuf = ion_share_dma_buf(client, handle);
++ dmabuf = __ion_share_dma_buf(client, handle, lock_client);
+ if (IS_ERR(dmabuf))
+ return PTR_ERR(dmabuf);
+
+@@ -1078,8 +1077,19 @@ int ion_share_dma_buf_fd(struct ion_clie
+
+ return fd;
+ }
++
++int ion_share_dma_buf_fd(struct ion_client *client, struct ion_handle *handle)
++{
++ return __ion_share_dma_buf_fd(client, handle, true);
++}
+ EXPORT_SYMBOL(ion_share_dma_buf_fd);
+
++int ion_share_dma_buf_fd_nolock(struct ion_client *client,
++ struct ion_handle *handle)
++{
++ return __ion_share_dma_buf_fd(client, handle, false);
++}
++
+ struct ion_handle *ion_import_dma_buf(struct ion_client *client,
+ struct dma_buf *dmabuf)
+ {
+--- a/drivers/staging/android/ion/ion_priv.h
++++ b/drivers/staging/android/ion/ion_priv.h
+@@ -463,11 +463,11 @@ void ion_free_nolock(struct ion_client *
+
+ int ion_handle_put_nolock(struct ion_handle *handle);
+
+-struct ion_handle *ion_handle_get_by_id(struct ion_client *client,
+- int id);
+-
+ int ion_handle_put(struct ion_handle *handle);
+
+ int ion_query_heaps(struct ion_client *client, struct ion_heap_query *query);
+
++int ion_share_dma_buf_fd_nolock(struct ion_client *client,
++ struct ion_handle *handle);
++
+ #endif /* _ION_PRIV_H */
projects / thirdparty / kernel / stable-queue.git / commitdiff
