diff options
| author | Michał Orynicz <michal.orynicz@sonymobile.com> | 2016-09-26 15:46:18 +0200 |
|---|---|---|
| committer | Alain Vongsouvanh <alainv@google.com> | 2016-09-29 17:04:25 +0000 |
| commit | 32ddc87dc4226bd4143f5bda39941a05d77e96b5 (patch) | |
| tree | 74d17d88f62a46712ff46cd8bbdedb3f067a7236 | |
| parent | 24de04c015b36a45299cf7390f5a95393f734909 (diff) | |
| download | bcm-32ddc87dc4226bd4143f5bda39941a05d77e96b5.tar.gz | |
Disallow usage of weak references where strong needed
The usage of weak references instead of strong references
in Binder can potentially lead to a use-after-free vulnerability
in system_server.
Disallow weak references in cases where strong references are needed.
Based on code snippet for ANDROID-30445380
Change-Id: Ifea2e55167b8f6131ca4a601d558558f79919eba
| -rw-r--r-- | drivers/staging/android/binder.c | 26 |
1 files changed, 17 insertions, 9 deletions
diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c index 0552bca32c4..78007f684b0 100644 --- a/drivers/staging/android/binder.c +++ b/drivers/staging/android/binder.c @@ -996,7 +996,7 @@ static int binder_dec_node(struct binder_node *node, int strong, int internal) static struct binder_ref *binder_get_ref(struct binder_proc *proc, - uint32_t desc) + uint32_t desc, bool need_strong_ref) { struct rb_node *n = proc->refs_by_desc.rb_node; struct binder_ref *ref; @@ -1004,12 +1004,16 @@ static struct binder_ref *binder_get_ref(struct binder_proc *proc, while (n) { ref = rb_entry(n, struct binder_ref, rb_node_desc); - if (desc < ref->desc) + if (desc < ref->desc) { n = n->rb_left; - else if (desc > ref->desc) + } else if (desc > ref->desc) { n = n->rb_right; - else + } else if (need_strong_ref && !ref->strong) { + binder_user_error("tried to use weak ref as strong ref\n"); + return NULL; + } else { return ref; + } } return NULL; } @@ -1273,7 +1277,8 @@ static void binder_transaction_buffer_release(struct binder_proc *proc, } break; case BINDER_TYPE_HANDLE: case BINDER_TYPE_WEAK_HANDLE: { - struct binder_ref *ref = binder_get_ref(proc, fp->handle); + struct binder_ref *ref = binder_get_ref(proc, fp->handle, + fp->type == BINDER_TYPE_HANDLE); if (ref == NULL) { pr_err("transaction release %d bad handle %d\n", debug_id, fp->handle); @@ -1366,7 +1371,7 @@ static void binder_transaction(struct binder_proc *proc, } else { if (tr->target.handle) { struct binder_ref *ref; - ref = binder_get_ref(proc, tr->target.handle); + ref = binder_get_ref(proc, tr->target.handle, true); if (ref == NULL) { binder_user_error("%d:%d got transaction to invalid handle\n", proc->pid, thread->pid); @@ -1571,7 +1576,8 @@ static void binder_transaction(struct binder_proc *proc, } break; case BINDER_TYPE_HANDLE: case BINDER_TYPE_WEAK_HANDLE: { - struct binder_ref *ref = binder_get_ref(proc, fp->handle); + struct binder_ref *ref = binder_get_ref(proc, fp->handle, + fp->type == BINDER_TYPE_HANDLE); if (ref == NULL) { binder_user_error("%d:%d got transaction with invalid handle, %d\n", proc->pid, @@ -1778,7 +1784,9 @@ int binder_thread_write(struct binder_proc *proc, struct binder_thread *thread, ref->desc); } } else - ref = binder_get_ref(proc, target); + ref = binder_get_ref(proc, target, + cmd == BC_ACQUIRE || + cmd == BC_RELEASE); if (ref == NULL) { binder_user_error("%d:%d refcount change on invalid ref %d\n", proc->pid, thread->pid, target); @@ -1973,7 +1981,7 @@ int binder_thread_write(struct binder_proc *proc, struct binder_thread *thread, if (get_user(cookie, (binder_uintptr_t __user *)ptr)) return -EFAULT; ptr += sizeof(binder_uintptr_t); - ref = binder_get_ref(proc, target); + ref = binder_get_ref(proc, target, false); if (ref == NULL) { binder_user_error("%d:%d %s invalid ref %d\n", proc->pid, thread->pid, |