This is a note to let you know that I've just added the patch titled
staging: android: ion: fix ION_IOC_{MAP,SHARE} use-after-free
to the 4.4-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
staging-android-ion-fix-ion_ioc_-map-share-use-after-free.patch
and it can be found in the queue-4.4 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <[email protected]> know about it.
>From [email protected] Tue Sep 4 18:34:26 2018
From: Greg Hackmann <[email protected]>
Date: Tue, 4 Sep 2018 09:33:36 -0700
Subject: staging: android: ion: fix ION_IOC_{MAP,SHARE} use-after-free
To: [email protected]
Cc: Laura Abbott <[email protected]>, Sumit Semwal <[email protected]>,
Greg Kroah-Hartman <[email protected]>, [email protected],
[email protected], Greg Hackmann <[email protected]>,
[email protected]
Message-ID: <[email protected]>
From: Greg Hackmann <[email protected]>
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.
This patch is applied on top of 4.4.y, and applies to older kernels
too. 4.9.y was fixed separately. Kernels 4.12 and later are
unaffected, since all the underlying ion_handle infrastructure has been
ripped out.
Cc: [email protected] # v4.4-
Signed-off-by: Greg Hackmann <[email protected]>
Acked-by: Laura Abbott <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
v2: remove Change-Id line from commit message
drivers/staging/android/ion/ion.c | 60 +++++++++++++++++++++++---------------
1 file changed, 37 insertions(+), 23 deletions(-)
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -449,18 +449,6 @@ static struct ion_handle *ion_handle_get
return 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)
{
@@ -1138,24 +1126,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;
@@ -1170,14 +1162,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);
@@ -1187,8 +1186,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);
+static 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, int fd)
{
struct dma_buf *dmabuf;
@@ -1335,11 +1345,15 @@ static long ion_ioctl(struct file *filp,
{
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;
Patches currently in stable-queue which might be from
[email protected] are
queue-4.4/staging-android-ion-fix-ion_ioc_-map-share-use-after-free.patch
_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
