From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S938941AbcJXOCs (ORCPT ); Mon, 24 Oct 2016 10:02:48 -0400 Received: from mail-qk0-f171.google.com ([209.85.220.171]:35345 "EHLO mail-qk0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935741AbcJXOCp (ORCPT ); Mon, 24 Oct 2016 10:02:45 -0400 MIME-Version: 1.0 In-Reply-To: <1477315238-104062-2-git-send-email-maco@android.com> References: <1477315238-104062-1-git-send-email-maco@android.com> <1477315238-104062-2-git-send-email-maco@android.com> From: Martijn Coenen Date: Mon, 24 Oct 2016 16:02:43 +0200 Message-ID: Subject: Re: [PATCH 01/10] ANDROID: binder: Add strong ref checks To: gregkh@linuxfoundation.org Cc: =?UTF-8?B?QXJ2ZSBIasO4bm5ldsOlZw==?= , linux-kernel@vger.kernel.org Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id u9OE2sbq014497 On Mon, Oct 24, 2016 at 3:20 PM, Martijn Coenen wrote: > From: Arve Hjønnevåg > > Prevent using a binder_ref with only weak references where a strong > reference is required. > > Signed-off-by: Arve Hjønnevåg Signed-off-by: Martijn Coenen > --- > drivers/android/binder.c | 30 +++++++++++++++++++++--------- > 1 file changed, 21 insertions(+), 9 deletions(-) > > diff --git a/drivers/android/binder.c b/drivers/android/binder.c > index 562af94..3681759 100644 > --- a/drivers/android/binder.c > +++ b/drivers/android/binder.c > @@ -1002,7 +1002,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) > + u32 desc, bool need_strong_ref) > { > struct rb_node *n = proc->refs_by_desc.rb_node; > struct binder_ref *ref; > @@ -1010,12 +1010,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; > } > @@ -1285,7 +1289,10 @@ 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; > + > + ref = binder_get_ref(proc, fp->handle, > + fp->type == BINDER_TYPE_HANDLE); > > if (ref == NULL) { > pr_err("transaction release %d bad handle %d\n", > @@ -1380,7 +1387,7 @@ static void binder_transaction(struct binder_proc *proc, > 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); > @@ -1589,7 +1596,10 @@ 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; > + > + 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", > @@ -1800,7 +1810,9 @@ static int binder_thread_write(struct binder_proc *proc, > 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); > @@ -1996,7 +2008,7 @@ static int binder_thread_write(struct binder_proc *proc, > 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, > -- > 2.8.0.rc3.226.g39d4020 >