| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <Y37WKunJ+dyH+5rm@google.com>
Date: Thu, 24 Nov 2022 02:25:46 +0000
From: Carlos Llamas <cmllamas@...gle.com>
To: Li Li <dualli@...omium.org>
Cc: dualli@...gle.com, gregkh@...uxfoundation.org, arve@...roid.com,
tkjos@...roid.com, maco@...roid.com, joel@...lfernandes.org,
brauner@...nel.org, surenb@...gle.com, arnd@...db.de,
masahiroy@...nel.org, devel@...verdev.osuosl.org,
linux-kernel@...r.kernel.org, hridya@...gle.com,
smoreland@...gle.com, kernel-team@...roid.com
Subject: Re: [PATCH v3 1/1] binder: return pending info for frozen async txns
On Wed, Nov 23, 2022 at 12:16:54PM -0800, Li Li wrote:
> From: Li Li <dualli@...gle.com>
>
> An async transaction to a frozen process will still be successfully
> put in the queue. But this pending async transaction won't be processed
> until the target process is unfrozen at an unspecified time in the
> future. Pass this important information back to the user space caller
> by returning BR_TRANSACTION_PENDING_FROZEN.
>
> Signed-off-by: Li Li <dualli@...gle.com>
> ---
> drivers/android/binder.c | 32 +++++++++++++++++++++++------
> drivers/android/binder_internal.h | 3 ++-
> include/uapi/linux/android/binder.h | 7 ++++++-
> 3 files changed, 34 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index 880224ec6abb..acd53147d5d1 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -2728,7 +2728,10 @@ binder_find_outdated_transaction_ilocked(struct binder_transaction *t,
> *
> * 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
> + * BR_FROZEN_REPLY if the target process or thread is frozen and
> + * the sync transaction was rejected
> + * BR_TRANSACTION_PENDING_FROZEN if the target process is frozen
> + * and the async transaction was successfully queued
> */
> static int binder_proc_transaction(struct binder_transaction *t,
> struct binder_proc *proc,
> @@ -2738,6 +2741,7 @@ static int binder_proc_transaction(struct binder_transaction *t,
> bool oneway = !!(t->flags & TF_ONE_WAY);
> bool pending_async = false;
> struct binder_transaction *t_outdated = NULL;
> + bool frozen = false;
>
> BUG_ON(!node);
> binder_node_lock(node);
> @@ -2751,15 +2755,16 @@ static int binder_proc_transaction(struct binder_transaction *t,
>
> binder_inner_proc_lock(proc);
> if (proc->is_frozen) {
> + frozen = true;
> proc->sync_recv |= !oneway;
> proc->async_recv |= oneway;
> }
>
> - if ((proc->is_frozen && !oneway) || proc->is_dead ||
> + if ((frozen && !oneway) || proc->is_dead ||
> (thread && thread->is_dead)) {
> binder_inner_proc_unlock(proc);
> binder_node_unlock(node);
> - return proc->is_frozen ? BR_FROZEN_REPLY : BR_DEAD_REPLY;
> + return frozen ? BR_FROZEN_REPLY : BR_DEAD_REPLY;
> }
>
> if (!thread && !pending_async)
> @@ -2770,7 +2775,7 @@ static int binder_proc_transaction(struct binder_transaction *t,
> } else if (!pending_async) {
> binder_enqueue_work_ilocked(&t->work, &proc->todo);
> } else {
> - if ((t->flags & TF_UPDATE_TXN) && proc->is_frozen) {
> + if ((t->flags & TF_UPDATE_TXN) && frozen) {
> t_outdated = binder_find_outdated_transaction_ilocked(t,
> &node->async_todo);
> if (t_outdated) {
> @@ -2807,6 +2812,9 @@ static int binder_proc_transaction(struct binder_transaction *t,
> binder_stats_deleted(BINDER_STAT_TRANSACTION);
> }
>
> + if (oneway && frozen)
> + return BR_TRANSACTION_PENDING_FROZEN;
> +
> return 0;
> }
>
> @@ -3607,9 +3615,17 @@ static void binder_transaction(struct binder_proc *proc,
> } else {
> BUG_ON(target_node == NULL);
> BUG_ON(t->buffer->async_transaction != 1);
> - binder_enqueue_thread_work(thread, tcomplete);
> return_error = binder_proc_transaction(t, target_proc, NULL);
> - if (return_error)
> + /*
> + * Let the caller know when async transaction reaches a frozen
> + * process and is put in a pending queue, waiting for the target
> + * process to be unfrozen.
> + */
> + if (return_error == BR_TRANSACTION_PENDING_FROZEN)
> + tcomplete->type = BINDER_WORK_TRANSACTION_PENDING;
> + binder_enqueue_thread_work(thread, tcomplete);
> + if (return_error &&
> + return_error != BR_TRANSACTION_PENDING_FROZEN)
> goto err_dead_proc_or_thread;
> }
> if (target_thread)
> @@ -4440,10 +4456,13 @@ static int binder_thread_read(struct binder_proc *proc,
> binder_stat_br(proc, thread, cmd);
> } break;
> case BINDER_WORK_TRANSACTION_COMPLETE:
> + case BINDER_WORK_TRANSACTION_PENDING:
> case BINDER_WORK_TRANSACTION_ONEWAY_SPAM_SUSPECT: {
> if (proc->oneway_spam_detection_enabled &&
> w->type == BINDER_WORK_TRANSACTION_ONEWAY_SPAM_SUSPECT)
> cmd = BR_ONEWAY_SPAM_SUSPECT;
> + else if (w->type == BINDER_WORK_TRANSACTION_PENDING)
> + cmd = BR_TRANSACTION_PENDING_FROZEN;
> else
> cmd = BR_TRANSACTION_COMPLETE;
> binder_inner_proc_unlock(proc);
> @@ -6170,6 +6189,7 @@ static const char * const binder_return_strings[] = {
> "BR_FAILED_REPLY",
> "BR_FROZEN_REPLY",
> "BR_ONEWAY_SPAM_SUSPECT",
> + "BR_TRANSACTION_PENDING_FROZEN"
> };
>
> static const char * const binder_command_strings[] = {
> diff --git a/drivers/android/binder_internal.h b/drivers/android/binder_internal.h
> index abe19d88c6ec..28ef5b3704b1 100644
> --- a/drivers/android/binder_internal.h
> +++ b/drivers/android/binder_internal.h
> @@ -133,7 +133,7 @@ enum binder_stat_types {
> };
>
> struct binder_stats {
> - atomic_t br[_IOC_NR(BR_ONEWAY_SPAM_SUSPECT) + 1];
> + atomic_t br[_IOC_NR(BR_TRANSACTION_PENDING_FROZEN) + 1];
> atomic_t bc[_IOC_NR(BC_REPLY_SG) + 1];
> atomic_t obj_created[BINDER_STAT_COUNT];
> atomic_t obj_deleted[BINDER_STAT_COUNT];
> @@ -152,6 +152,7 @@ struct binder_work {
> enum binder_work_type {
> BINDER_WORK_TRANSACTION = 1,
> BINDER_WORK_TRANSACTION_COMPLETE,
> + BINDER_WORK_TRANSACTION_PENDING,
> BINDER_WORK_TRANSACTION_ONEWAY_SPAM_SUSPECT,
> BINDER_WORK_RETURN_ERROR,
> BINDER_WORK_NODE,
> diff --git a/include/uapi/linux/android/binder.h b/include/uapi/linux/android/binder.h
> index e72e4de8f452..5f636b5afcd7 100644
> --- a/include/uapi/linux/android/binder.h
> +++ b/include/uapi/linux/android/binder.h
> @@ -450,7 +450,7 @@ enum binder_driver_return_protocol {
>
> BR_FROZEN_REPLY = _IO('r', 18),
> /*
> - * The target of the last transaction (either a bcTRANSACTION or
> + * The target of the last sync transaction (either a bcTRANSACTION or
> * a bcATTEMPT_ACQUIRE) is frozen. No parameters.
> */
>
> @@ -460,6 +460,11 @@ enum binder_driver_return_protocol {
> * asynchronous transaction makes the allocated async buffer size exceed
> * detection threshold. No parameters.
> */
> +
> + BR_TRANSACTION_PENDING_FROZEN = _IO('r', 20),
> + /*
> + * The target of the last async transaction is frozen. No parameters.
> + */
> };
>
> enum binder_driver_command_protocol {
> --
> 2.38.1.584.g0f3c55d4c2-goog
>
Thanks!
Acked-by: Carlos Llamas <cmllamas@...gle.com>
Powered by blists - more mailing lists