diff options
| author | Martijn Coenen <maco@android.com> | 2017-08-24 15:23:36 +0200 |
|---|---|---|
| committer | Julian Veit <claymore1298@gmail.com> | 2018-12-14 00:58:09 +0100 |
| commit | f0d28d876a5ce2198941ba908c77b7666dfda3a0 (patch) | |
| tree | c9b94e23d09f1a8a8fcdfce58ce8d71b3e4685d2 | |
| parent | 55a7350092d15dfe6193cf19b2e611dce727b0be (diff) | |
ANDROID: binder: fix transaction leak.
If a call to put_user() fails, we failed to
properly free a transaction and send a failed
reply (if necessary).
Bug: 63117588
Test: binderLibTest
Change-Id: Ia98db8cd82ce354a4cdc8811c969988d585c7e31
Signed-off-by: Martijn Coenen <maco@android.com>
Git-commit: be71309f0f4c10a8cc4b42ad495d649d5581ad45
Git-repo: https://android.googlesource.com/kernel/common/
[ported from upstream and resolved merge conflicts]
Signed-off-by: Srinivasarao P <spathi@codeaurora.org>
| -rw-r--r-- | drivers/android/binder.c | 46 |
1 files changed, 33 insertions, 13 deletions
diff --git a/drivers/android/binder.c b/drivers/android/binder.c index 7cf4625f313..c9dbd5b6e53 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -1398,6 +1398,28 @@ static void binder_send_failed_reply(struct binder_transaction *t, } /** + * binder_cleanup_transaction() - cleans up undelivered transaction + * @t: transaction that needs to be cleaned up + * @reason: reason the transaction wasn't delivered + * @error_code: error to return to caller (if synchronous call) + */ +static void binder_cleanup_transaction(struct binder_transaction *t, + const char *reason, + uint32_t error_code) +{ + if (t->buffer->target_node && !(t->flags & TF_ONE_WAY)) { + binder_send_failed_reply(t, error_code); + } else { + binder_debug(BINDER_DEBUG_DEAD_TRANSACTION, + "undelivered transaction %d, %s\n", + t->debug_id, reason); + t->buffer->transaction = NULL; + kfree(t); + binder_stats_deleted(BINDER_STAT_TRANSACTION); + } +} + +/** * binder_validate_object() - checks for a valid metadata object in a buffer. * @buffer: binder_buffer that we're parsing. * @offset: offset in the buffer at which to validate an object. @@ -3037,11 +3059,17 @@ retry: ALIGN(t->buffer->data_size, sizeof(void *)); - if (put_user_preempt_disabled(cmd, (uint32_t __user *)ptr)) + if (put_user_preempt_disabled(cmd, (uint32_t __user *)ptr)) { + binder_cleanup_transaction(t, "put_user failed", + BR_FAILED_REPLY); return -EFAULT; + } ptr += sizeof(uint32_t); - if (copy_to_user_preempt_disabled(ptr, &tr, sizeof(tr))) + if (copy_to_user_preempt_disabled(ptr, &tr, sizeof(tr))) { + binder_cleanup_transaction(t, "copy_to_user failed", + BR_FAILED_REPLY); return -EFAULT; + } ptr += sizeof(tr); trace_binder_transaction_received(t); @@ -3100,17 +3128,9 @@ static void binder_release_work(struct list_head *list) struct binder_transaction *t; t = container_of(w, struct binder_transaction, work); - if (t->buffer->target_node && - !(t->flags & TF_ONE_WAY)) { - binder_send_failed_reply(t, BR_DEAD_REPLY); - } else { - binder_debug(BINDER_DEBUG_DEAD_TRANSACTION, - "undelivered transaction %d\n", - t->debug_id); - t->buffer->transaction = NULL; - kfree(t); - binder_stats_deleted(BINDER_STAT_TRANSACTION); - } + + binder_cleanup_transaction(t, "process died.", + BR_DEAD_REPLY); } break; case BINDER_WORK_TRANSACTION_COMPLETE: { binder_debug(BINDER_DEBUG_DEAD_TRANSACTION, |