From: Lee Jones <lee.jones@linaro.org>
To: lee.jones@linaro.org
Cc: stable@vger.kernel.org, Daniel Rosenberg <drosen@google.com>,
Dennis Cagle <d-cagle@codeaurora.org>,
Patrick Daly <pdaly@codeaurora.org>
Subject: [PATCH v2 4.9 1/3] ion: Fix use after free during ION_IOC_ALLOC
Date: Tue, 25 Jan 2022 14:18:06 +0000 [thread overview]
Message-ID: <20220125141808.1172511-1-lee.jones@linaro.org> (raw)
From: Daniel Rosenberg <drosen@google.com>
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>
---
NB: These are Android patches that were not sent to Mainline.
Only v4.9 is affected by these issues due to refactoring.
drivers/staging/android/ion/ion-ioctl.c | 14 +++++++++-----
drivers/staging/android/ion/ion.c | 15 ++++++++++++---
drivers/staging/android/ion/ion.h | 4 ++++
3 files changed, 25 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/android/ion/ion-ioctl.c b/drivers/staging/android/ion/ion-ioctl.c
index e3596855a7031..f260e0e70488f 100644
--- a/drivers/staging/android/ion/ion-ioctl.c
+++ b/drivers/staging/android/ion/ion-ioctl.c
@@ -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;
}
diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index aac9b38b8c25c..4f769213be1b7 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -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,
diff --git a/drivers/staging/android/ion/ion.h b/drivers/staging/android/ion/ion.h
index 93dafb4586e43..cfa50dfb46edc 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -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
--
2.35.0.rc0.227.g00780c9af4-goog
next reply other threads:[~2022-01-25 14:25 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-25 14:18 Lee Jones [this message]
2022-01-25 14:18 ` [PATCH v2 4.9 2/3] ion: Protect kref from userspace manipulation Lee Jones
2022-01-25 14:18 ` [PATCH v2 4.9 3/3] ion: Do not 'put' ION handle until after its final use Lee Jones
2022-01-27 15:38 ` [PATCH v2 4.9 1/3] ion: Fix use after free during ION_IOC_ALLOC Greg KH
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220125141808.1172511-1-lee.jones@linaro.org \
--to=lee.jones@linaro.org \
--cc=d-cagle@codeaurora.org \
--cc=drosen@google.com \
--cc=pdaly@codeaurora.org \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).