Messages in this thread | | | From | Todd Kjos <> | Date | Tue, 17 May 2022 12:18:34 -0700 | Subject | Re: [PATCH] binder: fix potential UAF of target_{proc,thread} |
| |
On Tue, May 17, 2022 at 11:58 AM Carlos Llamas <cmllamas@google.com> wrote: > > Commit 9474be34a727 ("binder: add failed transaction logging info") > dereferences target_{proc,thread} after they have been potentially > freed by binder_proc_dec_tmpref() and binder_thread_dec_tmpref(). > > This patch delays the release of the two references after their last > usage. Fixes the following two errors reported by smatch: > > drivers/android/binder.c:3562 binder_transaction() error: dereferencing freed memory 'target_proc' > drivers/android/binder.c:3563 binder_transaction() error: dereferencing freed memory 'target_thread' > > Fixes: 9474be34a727 ("binder: add failed transaction logging info") > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Signed-off-by: Carlos Llamas <cmllamas@google.com>
Acked-by: Todd Kjos <tkjos@google.com>
> --- > drivers/android/binder.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/android/binder.c b/drivers/android/binder.c > index d9253b2a7bd9..83facfa1a5c3 100644 > --- a/drivers/android/binder.c > +++ b/drivers/android/binder.c > @@ -3546,10 +3546,6 @@ static void binder_transaction(struct binder_proc *proc, > err_empty_call_stack: > err_dead_binder: > err_invalid_target_handle: > - if (target_thread) > - binder_thread_dec_tmpref(target_thread); > - if (target_proc) > - binder_proc_dec_tmpref(target_proc); > if (target_node) { > binder_dec_node(target_node, 1, 0); > binder_dec_node_tmpref(target_node); > @@ -3565,6 +3561,11 @@ static void binder_transaction(struct binder_proc *proc, > (u64)tr->data_size, (u64)tr->offsets_size, > return_error_line); > > + if (target_thread) > + binder_thread_dec_tmpref(target_thread); > + if (target_proc) > + binder_proc_dec_tmpref(target_proc); > + > { > struct binder_transaction_log_entry *fe; > > -- > 2.36.0.550.gb090851708-goog >
| |