ANDROID: ion: Protect kref from userspace manipulation
This separates the kref for ion handles into two components. Userspace requests through the ioctl will hold at most one reference to the internally used kref. All additional requests will increment a separate counter, and the original reference is only put once that counter hits 0. This protects the kernel from a poorly behaving userspace. Bug: 34276203 Change-Id: Ibc36bc4405788ed0fea7337b541cad3be2b934c0 Signed-off-by: Daniel Rosenberg <drosen@google.com> Signed-off-by: Corinna Vinschen <xda@vinschen.de>
This commit is contained in:
parent
ed0ba3a012
commit
3d8a47d738
|
@ -113,6 +113,7 @@ struct ion_client {
|
|||
*/
|
||||
struct ion_handle {
|
||||
struct kref ref;
|
||||
unsigned int user_ref_count;
|
||||
struct ion_client *client;
|
||||
struct ion_buffer *buffer;
|
||||
struct rb_node node;
|
||||
|
@ -438,6 +439,50 @@ int ion_handle_put(struct ion_handle *handle)
|
|||
return ret;
|
||||
}
|
||||
|
||||
/* Must hold the client lock */
|
||||
static void user_ion_handle_get(struct ion_handle *handle)
|
||||
{
|
||||
if (handle->user_ref_count++ == 0) {
|
||||
kref_get(&handle->ref);
|
||||
}
|
||||
}
|
||||
|
||||
/* Must hold the client lock */
|
||||
static struct ion_handle* user_ion_handle_get_check_overflow(struct ion_handle *handle)
|
||||
{
|
||||
if (handle->user_ref_count + 1 == 0)
|
||||
return ERR_PTR(-EOVERFLOW);
|
||||
user_ion_handle_get(handle);
|
||||
return handle;
|
||||
}
|
||||
|
||||
/* passes a kref to the user ref count.
|
||||
* We know we're holding a kref to the object before and
|
||||
* after this call, so no need to reverify handle. */
|
||||
static struct ion_handle* pass_to_user(struct ion_handle *handle)
|
||||
{
|
||||
struct ion_client *client = handle->client;
|
||||
struct ion_handle *ret;
|
||||
|
||||
mutex_lock(&client->lock);
|
||||
ret = user_ion_handle_get_check_overflow(handle);
|
||||
ion_handle_put_nolock(handle);
|
||||
mutex_unlock(&client->lock);
|
||||
return ret;
|
||||
}
|
||||
|
||||
/* Must hold the client lock */
|
||||
static int user_ion_handle_put_nolock(struct ion_handle *handle)
|
||||
{
|
||||
int ret;
|
||||
|
||||
if (--handle->user_ref_count == 0) {
|
||||
ret = ion_handle_put_nolock(handle);
|
||||
}
|
||||
|
||||
return ret;
|
||||
}
|
||||
|
||||
static struct ion_handle *ion_handle_lookup(struct ion_client *client,
|
||||
struct ion_buffer *buffer)
|
||||
{
|
||||
|
@ -649,6 +694,24 @@ static void ion_free_nolock(struct ion_client *client, struct ion_handle *handle
|
|||
ion_handle_put_nolock(handle);
|
||||
}
|
||||
|
||||
static void user_ion_free_nolock(struct ion_client *client, struct ion_handle *handle)
|
||||
{
|
||||
bool valid_handle;
|
||||
|
||||
BUG_ON(client != handle->client);
|
||||
|
||||
valid_handle = ion_handle_validate(client, handle);
|
||||
if (!valid_handle) {
|
||||
WARN(1, "%s: invalid handle passed to free.\n", __func__);
|
||||
return;
|
||||
}
|
||||
if (!handle->user_ref_count > 0) {
|
||||
WARN(1, "%s: User does not have access!\n", __func__);
|
||||
return;
|
||||
}
|
||||
user_ion_handle_put_nolock(handle);
|
||||
}
|
||||
|
||||
void ion_free(struct ion_client *client, struct ion_handle *handle)
|
||||
{
|
||||
BUG_ON(client != handle->client);
|
||||
|
@ -1515,7 +1578,7 @@ static long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
|
|||
data.allocation.flags, true);
|
||||
if (IS_ERR(handle))
|
||||
return PTR_ERR(handle);
|
||||
|
||||
pass_to_user(handle);
|
||||
data.allocation.handle = handle->id;
|
||||
|
||||
cleanup_handle = handle;
|
||||
|
@ -1531,7 +1594,7 @@ static long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
|
|||
mutex_unlock(&client->lock);
|
||||
return PTR_ERR(handle);
|
||||
}
|
||||
ion_free_nolock(client, handle);
|
||||
user_ion_free_nolock(client, handle);
|
||||
ion_handle_put_nolock(handle);
|
||||
mutex_unlock(&client->lock);
|
||||
break;
|
||||
|
@ -1555,10 +1618,15 @@ static long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
|
|||
struct ion_handle *handle;
|
||||
|
||||
handle = ion_import_dma_buf(client, data.fd.fd);
|
||||
if (IS_ERR(handle))
|
||||
if (IS_ERR(handle)) {
|
||||
ret = PTR_ERR(handle);
|
||||
else
|
||||
data.handle.handle = handle->id;
|
||||
} else {
|
||||
handle = pass_to_user(handle);
|
||||
if (IS_ERR(handle))
|
||||
ret = PTR_ERR(handle);
|
||||
else
|
||||
data.handle.handle = handle->id;
|
||||
}
|
||||
break;
|
||||
}
|
||||
case ION_IOC_SYNC:
|
||||
|
@ -1590,8 +1658,10 @@ 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) {
|
||||
ion_free(client, cleanup_handle);
|
||||
ion_handle_put(cleanup_handle);
|
||||
mutex_lock(&client->lock);
|
||||
user_ion_free_nolock(client, cleanup_handle);
|
||||
ion_handle_put_nolock(cleanup_handle);
|
||||
mutex_unlock(&client->lock);
|
||||
}
|
||||
return -EFAULT;
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue