Re: [PATCH v3 1/3] binder: BINDER_FREEZE ioctl
On Mon, Mar 15, 2021 at 06:16:28PM -0700, Li Li wrote: > From: Marco Ballesio > > Frozen tasks can't process binder transactions, so a way is required to > inform transmitting ends of communication failures due to the frozen > state of their receiving counterparts. Additionally, races are possible > between transitions to frozen state and binder transactions enqueued to > a specific process. > > Implement BINDER_FREEZE ioctl for user space to inform the binder driver > about the intention to freeze or unfreeze a process. When the ioctl is > called, block the caller until any pending binder transactions toward > the target process are flushed. Return an error to transactions to > processes marked as frozen. > > Signed-off-by: Marco Ballesio > Co-developed-by: Todd Kjos > Signed-off-by: Todd Kjos > Signed-off-by: Li Li > --- > drivers/android/binder.c| 139 ++-- > drivers/android/binder_internal.h | 12 +++ > include/uapi/linux/android/binder.h | 13 +++ > 3 files changed, 154 insertions(+), 10 deletions(-) > > diff --git a/drivers/android/binder.c b/drivers/android/binder.c > index c119736ca56a..b93ca53bb90f 100644 > --- a/drivers/android/binder.c > +++ b/drivers/android/binder.c > @@ -1506,6 +1506,12 @@ static void binder_free_transaction(struct > binder_transaction *t) > > if (target_proc) { > binder_inner_proc_lock(target_proc); > + target_proc->outstanding_txns--; > + if (target_proc->outstanding_txns < 0) > + pr_warn("%s: Unexpected outstanding_txns %d\n", > + __func__, target_proc->outstanding_txns); > + if (!target_proc->outstanding_txns && target_proc->is_frozen) > + wake_up_interruptible_all(_proc->freeze_wait); > if (t->buffer) > t->buffer->transaction = NULL; > binder_inner_proc_unlock(target_proc); > @@ -2331,10 +2337,11 @@ static int binder_fixup_parent(struct > binder_transaction *t, > * If the @thread parameter is not NULL, the transaction is always queued > * to the waitlist of that specific thread. > * > - * Return: true if the transactions was successfully queued > - * false if the target process or thread is dead > + * Return: 0 if the transaction was successfully queued > + * BR_DEAD_REPLY if the target process or thread is dead > + * BR_FROZEN_REPLY if the target process or thread is frozen > */ > -static bool binder_proc_transaction(struct binder_transaction *t, > +static int binder_proc_transaction(struct binder_transaction *t, > struct binder_proc *proc, > struct binder_thread *thread) > { > @@ -2354,10 +2361,11 @@ static bool binder_proc_transaction(struct > binder_transaction *t, > > binder_inner_proc_lock(proc); > > - if (proc->is_dead || (thread && thread->is_dead)) { > + if ((proc->is_frozen && !oneway) || proc->is_dead || > + (thread && thread->is_dead)) { > binder_inner_proc_unlock(proc); > binder_node_unlock(node); > - return false; > + return proc->is_frozen ? BR_FROZEN_REPLY : BR_DEAD_REPLY; > } > > if (!thread && !pending_async) > @@ -2373,10 +2381,11 @@ static bool binder_proc_transaction(struct > binder_transaction *t, > if (!pending_async) > binder_wakeup_thread_ilocked(proc, thread, !oneway /* sync */); > > + proc->outstanding_txns++; > binder_inner_proc_unlock(proc); > binder_node_unlock(node); > > - return true; > + return 0; > } > > /** > @@ -3013,13 +3022,16 @@ static void binder_transaction(struct binder_proc > *proc, > if (reply) { > binder_enqueue_thread_work(thread, tcomplete); > binder_inner_proc_lock(target_proc); > - if (target_thread->is_dead) { > + if (target_thread->is_dead || target_proc->is_frozen) { > + return_error = target_thread->is_dead ? > + BR_DEAD_REPLY : BR_FROZEN_REPLY; > binder_inner_proc_unlock(target_proc); > goto err_dead_proc_or_thread; > } > BUG_ON(t->buffer->async_transaction != 0); > binder_pop_transaction_ilocked(target_thread, in_reply_to); > binder_enqueue_thread_work_ilocked(target_thread, >work); > + target_proc->outstanding_txns++; > binder_inner_proc_unlock(target_proc); > wake_up_interruptible_sync(_thread->wait); > binder_free_transaction(in_reply_to); > @@ -3038,7 +3050,9 @@ static void binder_transaction(struct binder_proc *proc, > t->from_parent = thread->transaction_stack; > thread->transaction_stack = t; >
Re: [PATCH v3 1/3] binder: BINDER_FREEZE ioctl
On Mon, Mar 15, 2021 at 6:16 PM Li Li wrote: > > From: Marco Ballesio > > Frozen tasks can't process binder transactions, so a way is required to > inform transmitting ends of communication failures due to the frozen > state of their receiving counterparts. Additionally, races are possible > between transitions to frozen state and binder transactions enqueued to > a specific process. > > Implement BINDER_FREEZE ioctl for user space to inform the binder driver > about the intention to freeze or unfreeze a process. When the ioctl is > called, block the caller until any pending binder transactions toward > the target process are flushed. Return an error to transactions to > processes marked as frozen. > > Signed-off-by: Marco Ballesio > Co-developed-by: Todd Kjos > Signed-off-by: Todd Kjos > Signed-off-by: Li Li For the series, you can add Acked-by: Todd Kjos > --- > drivers/android/binder.c| 139 ++-- > drivers/android/binder_internal.h | 12 +++ > include/uapi/linux/android/binder.h | 13 +++ > 3 files changed, 154 insertions(+), 10 deletions(-) > > diff --git a/drivers/android/binder.c b/drivers/android/binder.c > index c119736ca56a..b93ca53bb90f 100644 > --- a/drivers/android/binder.c > +++ b/drivers/android/binder.c > @@ -1506,6 +1506,12 @@ static void binder_free_transaction(struct > binder_transaction *t) > > if (target_proc) { > binder_inner_proc_lock(target_proc); > + target_proc->outstanding_txns--; > + if (target_proc->outstanding_txns < 0) > + pr_warn("%s: Unexpected outstanding_txns %d\n", > + __func__, target_proc->outstanding_txns); > + if (!target_proc->outstanding_txns && target_proc->is_frozen) > + wake_up_interruptible_all(_proc->freeze_wait); > if (t->buffer) > t->buffer->transaction = NULL; > binder_inner_proc_unlock(target_proc); > @@ -2331,10 +2337,11 @@ static int binder_fixup_parent(struct > binder_transaction *t, > * If the @thread parameter is not NULL, the transaction is always queued > * to the waitlist of that specific thread. > * > - * Return: true if the transactions was successfully queued > - * false if the target process or thread is dead > + * Return: 0 if the transaction was successfully queued > + * BR_DEAD_REPLY if the target process or thread is dead > + * BR_FROZEN_REPLY if the target process or thread is frozen > */ > -static bool binder_proc_transaction(struct binder_transaction *t, > +static int binder_proc_transaction(struct binder_transaction *t, > struct binder_proc *proc, > struct binder_thread *thread) > { > @@ -2354,10 +2361,11 @@ static bool binder_proc_transaction(struct > binder_transaction *t, > > binder_inner_proc_lock(proc); > > - if (proc->is_dead || (thread && thread->is_dead)) { > + if ((proc->is_frozen && !oneway) || proc->is_dead || > + (thread && thread->is_dead)) { > binder_inner_proc_unlock(proc); > binder_node_unlock(node); > - return false; > + return proc->is_frozen ? BR_FROZEN_REPLY : BR_DEAD_REPLY; > } > > if (!thread && !pending_async) > @@ -2373,10 +2381,11 @@ static bool binder_proc_transaction(struct > binder_transaction *t, > if (!pending_async) > binder_wakeup_thread_ilocked(proc, thread, !oneway /* sync > */); > > + proc->outstanding_txns++; > binder_inner_proc_unlock(proc); > binder_node_unlock(node); > > - return true; > + return 0; > } > > /** > @@ -3013,13 +3022,16 @@ static void binder_transaction(struct binder_proc > *proc, > if (reply) { > binder_enqueue_thread_work(thread, tcomplete); > binder_inner_proc_lock(target_proc); > - if (target_thread->is_dead) { > + if (target_thread->is_dead || target_proc->is_frozen) { > + return_error = target_thread->is_dead ? > + BR_DEAD_REPLY : BR_FROZEN_REPLY; > binder_inner_proc_unlock(target_proc); > goto err_dead_proc_or_thread; > } > BUG_ON(t->buffer->async_transaction != 0); > binder_pop_transaction_ilocked(target_thread, in_reply_to); > binder_enqueue_thread_work_ilocked(target_thread, >work); > + target_proc->outstanding_txns++; > binder_inner_proc_unlock(target_proc); > wake_up_interruptible_sync(_thread->wait); > binder_free_transaction(in_reply_to); > @@ -3038,7 +3050,9 @@ static void binder_transaction(struct binder_proc *proc, >
[PATCH v3 1/3] binder: BINDER_FREEZE ioctl
From: Marco Ballesio Frozen tasks can't process binder transactions, so a way is required to inform transmitting ends of communication failures due to the frozen state of their receiving counterparts. Additionally, races are possible between transitions to frozen state and binder transactions enqueued to a specific process. Implement BINDER_FREEZE ioctl for user space to inform the binder driver about the intention to freeze or unfreeze a process. When the ioctl is called, block the caller until any pending binder transactions toward the target process are flushed. Return an error to transactions to processes marked as frozen. Signed-off-by: Marco Ballesio Co-developed-by: Todd Kjos Signed-off-by: Todd Kjos Signed-off-by: Li Li --- drivers/android/binder.c| 139 ++-- drivers/android/binder_internal.h | 12 +++ include/uapi/linux/android/binder.h | 13 +++ 3 files changed, 154 insertions(+), 10 deletions(-) diff --git a/drivers/android/binder.c b/drivers/android/binder.c index c119736ca56a..b93ca53bb90f 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -1506,6 +1506,12 @@ static void binder_free_transaction(struct binder_transaction *t) if (target_proc) { binder_inner_proc_lock(target_proc); + target_proc->outstanding_txns--; + if (target_proc->outstanding_txns < 0) + pr_warn("%s: Unexpected outstanding_txns %d\n", + __func__, target_proc->outstanding_txns); + if (!target_proc->outstanding_txns && target_proc->is_frozen) + wake_up_interruptible_all(_proc->freeze_wait); if (t->buffer) t->buffer->transaction = NULL; binder_inner_proc_unlock(target_proc); @@ -2331,10 +2337,11 @@ static int binder_fixup_parent(struct binder_transaction *t, * If the @thread parameter is not NULL, the transaction is always queued * to the waitlist of that specific thread. * - * Return: true if the transactions was successfully queued - * false if the target process or thread is dead + * Return: 0 if the transaction was successfully queued + * BR_DEAD_REPLY if the target process or thread is dead + * BR_FROZEN_REPLY if the target process or thread is frozen */ -static bool binder_proc_transaction(struct binder_transaction *t, +static int binder_proc_transaction(struct binder_transaction *t, struct binder_proc *proc, struct binder_thread *thread) { @@ -2354,10 +2361,11 @@ static bool binder_proc_transaction(struct binder_transaction *t, binder_inner_proc_lock(proc); - if (proc->is_dead || (thread && thread->is_dead)) { + if ((proc->is_frozen && !oneway) || proc->is_dead || + (thread && thread->is_dead)) { binder_inner_proc_unlock(proc); binder_node_unlock(node); - return false; + return proc->is_frozen ? BR_FROZEN_REPLY : BR_DEAD_REPLY; } if (!thread && !pending_async) @@ -2373,10 +2381,11 @@ static bool binder_proc_transaction(struct binder_transaction *t, if (!pending_async) binder_wakeup_thread_ilocked(proc, thread, !oneway /* sync */); + proc->outstanding_txns++; binder_inner_proc_unlock(proc); binder_node_unlock(node); - return true; + return 0; } /** @@ -3013,13 +3022,16 @@ static void binder_transaction(struct binder_proc *proc, if (reply) { binder_enqueue_thread_work(thread, tcomplete); binder_inner_proc_lock(target_proc); - if (target_thread->is_dead) { + if (target_thread->is_dead || target_proc->is_frozen) { + return_error = target_thread->is_dead ? + BR_DEAD_REPLY : BR_FROZEN_REPLY; binder_inner_proc_unlock(target_proc); goto err_dead_proc_or_thread; } BUG_ON(t->buffer->async_transaction != 0); binder_pop_transaction_ilocked(target_thread, in_reply_to); binder_enqueue_thread_work_ilocked(target_thread, >work); + target_proc->outstanding_txns++; binder_inner_proc_unlock(target_proc); wake_up_interruptible_sync(_thread->wait); binder_free_transaction(in_reply_to); @@ -3038,7 +3050,9 @@ static void binder_transaction(struct binder_proc *proc, t->from_parent = thread->transaction_stack; thread->transaction_stack = t; binder_inner_proc_unlock(proc); - if (!binder_proc_transaction(t, target_proc, target_thread)) { + return_error = binder_proc_transaction(t, +