Commit 52ad8f0a authored by Srinivasarao P's avatar Srinivasarao P
Browse files

ion: remove unsafe function ion_handle_get_by_id()

The function ion_handle_get_by_id() is called from function
msm_ion_custom_ioctl(), so we retained it even though it got
deleted in commit 2c155709


("staging: android: ion: fix ION_IOC_{MAP,SHARE} use-after-free").

This can lead to same use-after-free scenario with
ION_IOC_{CLEAN_CACHES,INV_CACHES,CLEAN_INV_CACHES} so removing
this unsafe function and holding client->lock for entire operation.

Change-Id: I536cfa69465ad692794500e3b31ac137d04940ff
Signed-off-by: default avatarSrinivasarao P <spathi@codeaurora.org>
parent a36b6328
......@@ -3,7 +3,7 @@
* drivers/staging/android/ion/ion.c
*
* Copyright (C) 2011 Google, Inc.
* Copyright (c) 2011-2017, The Linux Foundation. All rights reserved.
* Copyright (c) 2011-2018, The Linux Foundation. All rights reserved.
*
* This software is licensed under the terms of the GNU General Public
* License version 2, as published by the Free Software Foundation, and
......@@ -495,8 +495,8 @@ static struct ion_handle *ion_handle_lookup(struct ion_client *client,
return ERR_PTR(-EINVAL);
}
static struct ion_handle *ion_handle_get_by_id_nolock(struct ion_client *client,
int id)
struct ion_handle *ion_handle_get_by_id_nolock(struct ion_client *client,
int id)
{
struct ion_handle *handle;
......@@ -507,20 +507,7 @@ static struct ion_handle *ion_handle_get_by_id_nolock(struct ion_client *client,
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)
bool ion_handle_validate(struct ion_client *client, struct ion_handle *handle)
{
WARN_ON(!mutex_is_locked(&client->lock));
return idr_find(&client->idr, handle->id) == handle;
......@@ -674,7 +661,7 @@ struct ion_handle *ion_alloc(struct ion_client *client, size_t len,
}
EXPORT_SYMBOL(ion_alloc);
static void ion_free_nolock(struct ion_client *client, struct ion_handle *handle)
void ion_free_nolock(struct ion_client *client, struct ion_handle *handle)
{
bool valid_handle;
......@@ -717,15 +704,17 @@ void ion_free(struct ion_client *client, struct ion_handle *handle)
}
EXPORT_SYMBOL(ion_free);
int ion_phys(struct ion_client *client, struct ion_handle *handle,
ion_phys_addr_t *addr, size_t *len)
static int __ion_phys(struct ion_client *client, struct ion_handle *handle,
ion_phys_addr_t *addr, size_t *len, bool lock_client)
{
struct ion_buffer *buffer;
int ret;
mutex_lock(&client->lock);
if (lock_client)
mutex_lock(&client->lock);
if (!ion_handle_validate(client, handle)) {
mutex_unlock(&client->lock);
if (lock_client)
mutex_unlock(&client->lock);
return -EINVAL;
}
......@@ -734,15 +723,29 @@ int ion_phys(struct ion_client *client, struct ion_handle *handle,
if (!buffer->heap->ops->phys) {
pr_err("%s: ion_phys is not implemented by this heap (name=%s, type=%d).\n",
__func__, buffer->heap->name, buffer->heap->type);
mutex_unlock(&client->lock);
if (lock_client)
mutex_unlock(&client->lock);
return -ENODEV;
}
mutex_unlock(&client->lock);
if (lock_client)
mutex_unlock(&client->lock);
ret = buffer->heap->ops->phys(buffer->heap, buffer, addr, len);
return ret;
}
int ion_phys(struct ion_client *client, struct ion_handle *handle,
ion_phys_addr_t *addr, size_t *len)
{
return __ion_phys(client, handle, addr, len, true);
}
EXPORT_SYMBOL(ion_phys);
int ion_phys_nolock(struct ion_client *client, struct ion_handle *handle,
ion_phys_addr_t *addr, size_t *len)
{
return __ion_phys(client, handle, addr, len, false);
}
static void *ion_buffer_kmap_get(struct ion_buffer *buffer)
{
void *vaddr;
......@@ -1511,7 +1514,8 @@ static int ion_share_dma_buf_fd_nolock(struct ion_client *client,
return __ion_share_dma_buf_fd(client, handle, false);
}
struct ion_handle *ion_import_dma_buf(struct ion_client *client, int fd)
static struct ion_handle *__ion_import_dma_buf(struct ion_client *client,
int fd, bool lock_client)
{
struct dma_buf *dmabuf;
struct ion_buffer *buffer;
......@@ -1531,25 +1535,32 @@ struct ion_handle *ion_import_dma_buf(struct ion_client *client, int fd)
}
buffer = dmabuf->priv;
mutex_lock(&client->lock);
if (lock_client)
mutex_lock(&client->lock);
/* if a handle exists for this buffer just take a reference to it */
handle = ion_handle_lookup(client, buffer);
if (!IS_ERR(handle)) {
handle = ion_handle_get_check_overflow(handle);
mutex_unlock(&client->lock);
if (lock_client)
mutex_unlock(&client->lock);
goto end;
}
handle = ion_handle_create(client, buffer);
if (IS_ERR(handle)) {
mutex_unlock(&client->lock);
if (lock_client)
mutex_unlock(&client->lock);
goto end;
}
ret = ion_handle_add(client, handle);
mutex_unlock(&client->lock);
if (lock_client)
mutex_unlock(&client->lock);
if (ret) {
ion_handle_put(handle);
if (lock_client)
ion_handle_put(handle);
else
ion_handle_put_nolock(handle);
handle = ERR_PTR(ret);
}
......@@ -1557,8 +1568,18 @@ end:
dma_buf_put(dmabuf);
return handle;
}
struct ion_handle *ion_import_dma_buf(struct ion_client *client, int fd)
{
return __ion_import_dma_buf(client, fd, true);
}
EXPORT_SYMBOL(ion_import_dma_buf);
struct ion_handle *ion_import_dma_buf_nolock(struct ion_client *client, int fd)
{
return __ion_import_dma_buf(client, fd, false);
}
static int ion_sync_for_device(struct ion_client *client, int fd)
{
struct dma_buf *dmabuf;
......@@ -2201,3 +2222,18 @@ void __init ion_reserve(struct ion_platform_data *data)
data->heaps[i].size);
}
}
void lock_client(struct ion_client *client)
{
mutex_lock(&client->lock);
}
void unlock_client(struct ion_client *client)
{
mutex_unlock(&client->lock);
}
struct ion_buffer *get_buffer(struct ion_handle *handle)
{
return handle->buffer;
}
......@@ -2,7 +2,7 @@
* drivers/staging/android/ion/ion_priv.h
*
* Copyright (C) 2011 Google, Inc.
* Copyright (c) 2011-2017, The Linux Foundation. All rights reserved.
* Copyright (c) 2011-2018, The Linux Foundation. All rights reserved.
*
* This software is licensed under the terms of the GNU General Public
* License version 2, as published by the Free Software Foundation, and
......@@ -505,9 +505,34 @@ int ion_walk_heaps(struct ion_client *client, int heap_id,
enum ion_heap_type type, void *data,
int (*f)(struct ion_heap *heap, void *data));
struct ion_handle *ion_handle_get_by_id(struct ion_client *client,
int id);
struct ion_handle *ion_handle_get_by_id_nolock(struct ion_client *client,
int id);
int ion_handle_put(struct ion_handle *handle);
bool ion_handle_validate(struct ion_client *client, struct ion_handle *handle);
void lock_client(struct ion_client *client);
void unlock_client(struct ion_client *client);
struct ion_buffer *get_buffer(struct ion_handle *handle);
/**
* This function is same as ion_free() except it won't use client->lock.
*/
void ion_free_nolock(struct ion_client *client, struct ion_handle *handle);
/**
* This function is same as ion_phys() except it won't use client->lock.
*/
int ion_phys_nolock(struct ion_client *client, struct ion_handle *handle,
ion_phys_addr_t *addr, size_t *len);
/**
* This function is same as ion_import_dma_buf() except it won't use
* client->lock.
*/
struct ion_handle *ion_import_dma_buf_nolock(struct ion_client *client, int fd);
#endif /* _ION_PRIV_H */
......@@ -153,7 +153,13 @@ EXPORT_SYMBOL(msm_ion_client_create);
int msm_ion_do_cache_op(struct ion_client *client, struct ion_handle *handle,
void *vaddr, unsigned long len, unsigned int cmd)
{
return ion_do_cache_op(client, handle, vaddr, 0, len, cmd);
int ret;
lock_client(client);
ret = ion_do_cache_op(client, handle, vaddr, 0, len, cmd);
unlock_client(client);
return ret;
}
EXPORT_SYMBOL(msm_ion_do_cache_op);
......@@ -162,7 +168,13 @@ int msm_ion_do_cache_offset_op(
void *vaddr, unsigned int offset, unsigned long len,
unsigned int cmd)
{
return ion_do_cache_op(client, handle, vaddr, offset, len, cmd);
int ret;
lock_client(client);
ret = ion_do_cache_op(client, handle, vaddr, offset, len, cmd);
unlock_client(client);
return ret;
}
EXPORT_SYMBOL(msm_ion_do_cache_offset_op);
......@@ -179,7 +191,7 @@ static int ion_no_pages_cache_ops(struct ion_client *client,
ion_phys_addr_t buff_phys_start = 0;
size_t buf_length = 0;
ret = ion_phys(client, handle, &buff_phys_start, &buf_length);
ret = ion_phys_nolock(client, handle, &buff_phys_start, &buf_length);
if (ret)
return -EINVAL;
......@@ -293,9 +305,10 @@ static int ion_pages_cache_ops(struct ion_client *client,
int i;
unsigned int len = 0;
void (*op)(const void *, const void *);
struct ion_buffer *buffer;
table = ion_sg_table(client, handle);
buffer = get_buffer(handle);
table = buffer->sg_table;
if (IS_ERR_OR_NULL(table))
return PTR_ERR(table);
......@@ -344,10 +357,18 @@ int ion_do_cache_op(struct ion_client *client, struct ion_handle *handle,
unsigned long flags;
struct sg_table *table;
struct page *page;
struct ion_buffer *buffer;
ret = ion_handle_get_flags(client, handle, &flags);
if (ret)
if (!ion_handle_validate(client, handle)) {
pr_err("%s: invalid handle passed to %s.\n",
__func__, __func__);
return -EINVAL;
}
buffer = get_buffer(handle);
mutex_lock(&buffer->lock);
flags = buffer->flags;
mutex_unlock(&buffer->lock);
if (!ION_IS_CACHED(flags))
return 0;
......@@ -355,7 +376,7 @@ int ion_do_cache_op(struct ion_client *client, struct ion_handle *handle,
if (get_secure_vmid(flags) > 0)
return 0;
table = ion_sg_table(client, handle);
table = buffer->sg_table;
if (IS_ERR_OR_NULL(table))
return PTR_ERR(table);
......@@ -737,19 +758,23 @@ long msm_ion_custom_ioctl(struct ion_client *client,
int ret;
struct mm_struct *mm = current->active_mm;
lock_client(client);
if (data.flush_data.handle > 0) {
handle = ion_handle_get_by_id(client,
(int)data.flush_data.handle);
handle = ion_handle_get_by_id_nolock(
client, (int)data.flush_data.handle);
if (IS_ERR(handle)) {
pr_info("%s: Could not find handle: %d\n",
__func__, (int)data.flush_data.handle);
unlock_client(client);
return PTR_ERR(handle);
}
} else {
handle = ion_import_dma_buf(client, data.flush_data.fd);
handle = ion_import_dma_buf_nolock(client,
data.flush_data.fd);
if (IS_ERR(handle)) {
pr_info("%s: Could not import handle: %pK\n",
__func__, handle);
unlock_client(client);
return -EINVAL;
}
}
......@@ -772,8 +797,9 @@ long msm_ion_custom_ioctl(struct ion_client *client,
}
up_read(&mm->mmap_sem);
ion_free(client, handle);
ion_free_nolock(client, handle);
unlock_client(client);
if (ret < 0)
return ret;
break;
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment