shinys000114/linux
1
0
Fork 0
mirror of https://github.com/hardkernel/linux.git synced 2026-06-07 19:30:30 +09:00
Code Issues Packages Projects Releases Wiki Activity

staging: android: ion: fix ION_IOC_{MAP,SHARE} use-after-free

Browse Source 
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>
This commit is contained in:
Greg Hackmann
2018-08-31 13:06:27 -07:00
committed by Chris
parent 96e6367958
commit 78819480da
3 changed files with 40 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
drivers/staging/android/ion/ion-ioctl.c
View File

@@ -132,11 +132,15 @@ long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
{
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;

48
drivers/staging/android/ion/ion.c
View File

@@ -355,18 +355,6 @@ struct ion_handle *ion_handle_get_by_id_nolock(struct ion_client *client,
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)
{
@@ -1054,24 +1042,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;
@@ -1086,14 +1078,21 @@ struct dma_buf *ion_share_dma_buf(struct ion_client *client,
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);
@@ -1103,8 +1102,19 @@ int ion_share_dma_buf_fd(struct ion_client *client, struct ion_handle *handle)
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)
{

6
drivers/staging/android/ion/ion_priv.h
View File

@@ -480,11 +480,11 @@ void ion_free_nolock(struct ion_client *client, struct ion_handle *handle);
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 */