ion: Fix use after free during ION_IOC_ALLOC
authorDaniel Rosenberg <drosen@google.com>
Tue, 25 Jan 2022 14:18:06 +0000 (14:18 +0000)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Sat, 29 Jan 2022 09:15:58 +0000 (10:15 +0100)
If a user happens to call ION_IOC_FREE during an ION_IOC_ALLOC
on the just allocated id, and the copy_to_user fails, the cleanup
code will attempt to free an already freed handle.

This adds a wrapper for ion_alloc that adds an ion_handle_get to
avoid this.

Signed-off-by: Daniel Rosenberg <drosen@google.com>
Signed-off-by: Dennis Cagle <d-cagle@codeaurora.org>
Signed-off-by: Patrick Daly <pdaly@codeaurora.org>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/staging/android/ion/ion-ioctl.c
drivers/staging/android/ion/ion.c
drivers/staging/android/ion/ion.h

index e3596855a7031599721845a3323ad4c925cdfec4..f260e0e70488ffdf3c84acab667e13977539fe8c 100644 (file)
@@ -96,10 +96,10 @@ long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
        {
                struct ion_handle *handle;
 
-               handle = ion_alloc(client, data.allocation.len,
-                                               data.allocation.align,
-                                               data.allocation.heap_id_mask,
-                                               data.allocation.flags);
+               handle = __ion_alloc(client, data.allocation.len,
+                                    data.allocation.align,
+                                    data.allocation.heap_id_mask,
+                                    data.allocation.flags, true);
                if (IS_ERR(handle))
                        return PTR_ERR(handle);
 
@@ -174,10 +174,14 @@ 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)
+                       if (cleanup_handle) {
                                ion_free(client, cleanup_handle);
+                               ion_handle_put(cleanup_handle);
+                       }
                        return -EFAULT;
                }
        }
+       if (cleanup_handle)
+               ion_handle_put(cleanup_handle);
        return ret;
 }
index aac9b38b8c25c50de592273150e3d125e806b95a..4f769213be1b745b74adb6763c2bc1ae09901a99 100644 (file)
@@ -401,9 +401,9 @@ static int ion_handle_add(struct ion_client *client, struct ion_handle *handle)
        return 0;
 }
 
-struct ion_handle *ion_alloc(struct ion_client *client, size_t len,
-                            size_t align, unsigned int heap_id_mask,
-                            unsigned int flags)
+struct ion_handle *__ion_alloc(struct ion_client *client, size_t len,
+                              size_t align, unsigned int heap_id_mask,
+                              unsigned int flags, bool grab_handle)
 {
        struct ion_handle *handle;
        struct ion_device *dev = client->dev;
@@ -453,6 +453,8 @@ struct ion_handle *ion_alloc(struct ion_client *client, size_t len,
                return handle;
 
        mutex_lock(&client->lock);
+       if (grab_handle)
+               ion_handle_get(handle);
        ret = ion_handle_add(client, handle);
        mutex_unlock(&client->lock);
        if (ret) {
@@ -462,6 +464,13 @@ struct ion_handle *ion_alloc(struct ion_client *client, size_t len,
 
        return handle;
 }
+
+struct ion_handle *ion_alloc(struct ion_client *client, size_t len,
+                            size_t align, unsigned int heap_id_mask,
+                            unsigned int flags)
+{
+       return __ion_alloc(client, len, align, heap_id_mask, flags, false);
+}
 EXPORT_SYMBOL(ion_alloc);
 
 void ion_free_nolock(struct ion_client *client,
index 93dafb4586e43359c050da946f088d95ba35610f..cfa50dfb46edc71cfc38dacafb06f46089bb5220 100644 (file)
@@ -109,6 +109,10 @@ struct ion_handle *ion_alloc(struct ion_client *client, size_t len,
                             size_t align, unsigned int heap_id_mask,
                             unsigned int flags);
 
+struct ion_handle *__ion_alloc(struct ion_client *client, size_t len,
+                              size_t align, unsigned int heap_id_mask,
+                              unsigned int flags, bool grab_handle);
+
 /**
  * ion_free - free a handle
  * @client:    the client