From 169adc2b6b3c5e86391921117b4ab3aaeb3c6ee1 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Thu, 10 Feb 2022 11:11:26 +0900 Subject: android/binder.h: add linux/android/binder(fs).h to UAPI compile-test coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit linux/android/binder.h and linux/android/binderfs.h are currently excluded from the UAPI compile-test because of the errors like follows: HDRTEST usr/include/linux/android/binder.h In file included from : ./usr/include/linux/android/binder.h:291:9: error: unknown type name ‘pid_t’ 291 | pid_t sender_pid; | ^~~~~ ./usr/include/linux/android/binder.h:292:9: error: unknown type name ‘uid_t’ 292 | uid_t sender_euid; | ^~~~~ The errors can be fixed by replacing {pid,uid}_t with __kernel_{pid,uid}_t. Then, remove the no-header-test entries from user/include/Makefile. Signed-off-by: Masahiro Yamada Reviewed-by: Arnd Bergmann Signed-off-by: Arnd Bergmann --- include/uapi/linux/android/binder.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'include/uapi/linux/android') diff --git a/include/uapi/linux/android/binder.h b/include/uapi/linux/android/binder.h index 3246f2c74696..11157fae8a8e 100644 --- a/include/uapi/linux/android/binder.h +++ b/include/uapi/linux/android/binder.h @@ -288,8 +288,8 @@ struct binder_transaction_data { /* General information about the transaction. */ __u32 flags; - pid_t sender_pid; - uid_t sender_euid; + __kernel_pid_t sender_pid; + __kernel_uid_t sender_euid; binder_size_t data_size; /* number of bytes of data */ binder_size_t offsets_size; /* number of bytes of offsets */ -- cgit From bd32889e841c12533d09a1bd02bba932baa9ed8f Mon Sep 17 00:00:00 2001 From: Carlos Llamas Date: Fri, 29 Apr 2022 23:56:41 +0000 Subject: binder: add BINDER_GET_EXTENDED_ERROR ioctl Provide a userspace mechanism to pull precise error information upon failed operations. Extending the current error codes returned by the interfaces allows userspace to better determine the course of action. This could be for instance, retrying a failed transaction at a later point and thus offloading the error handling from the driver. Acked-by: Christian Brauner (Microsoft) Acked-by: Todd Kjos Signed-off-by: Carlos Llamas Link: https://lore.kernel.org/r/20220429235644.697372-3-cmllamas@google.com Signed-off-by: Greg Kroah-Hartman --- drivers/android/binder.c | 60 +++++++++++++++++++++++++++++++++++++ drivers/android/binder_internal.h | 3 ++ include/uapi/linux/android/binder.h | 16 ++++++++++ 3 files changed, 79 insertions(+) (limited to 'include/uapi/linux/android') diff --git a/drivers/android/binder.c b/drivers/android/binder.c index f0690d46caa1..4c2caf38e056 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -147,6 +147,13 @@ module_param_call(stop_on_user_error, binder_set_stop_on_user_error, binder_stop_on_user_error = 2; \ } while (0) +#define binder_set_extended_error(ee, _id, _command, _param) \ + do { \ + (ee)->id = _id; \ + (ee)->command = _command; \ + (ee)->param = _param; \ + } while (0) + #define to_flat_binder_object(hdr) \ container_of(hdr, struct flat_binder_object, hdr) @@ -2708,6 +2715,24 @@ static struct binder_node *binder_get_node_refs_for_txn( return target_node; } +static void binder_set_txn_from_error(struct binder_transaction *t, int id, + uint32_t command, int32_t param) +{ + struct binder_thread *from = binder_get_txn_from_and_acq_inner(t); + + if (!from) { + /* annotation for sparse */ + __release(&from->proc->inner_lock); + return; + } + + /* don't override existing errors */ + if (from->ee.command == BR_OK) + binder_set_extended_error(&from->ee, id, command, param); + binder_inner_proc_unlock(from->proc); + binder_thread_dec_tmpref(from); +} + static void binder_transaction(struct binder_proc *proc, struct binder_thread *thread, struct binder_transaction_data *tr, int reply, @@ -2753,6 +2778,10 @@ static void binder_transaction(struct binder_proc *proc, e->offsets_size = tr->offsets_size; strscpy(e->context_name, proc->context->name, BINDERFS_MAX_NAME); + binder_inner_proc_lock(proc); + binder_set_extended_error(&thread->ee, t_debug_id, BR_OK, 0); + binder_inner_proc_unlock(proc); + if (reply) { binder_inner_proc_lock(proc); in_reply_to = thread->transaction_stack; @@ -3498,10 +3527,16 @@ err_invalid_target_handle: BUG_ON(thread->return_error.cmd != BR_OK); if (in_reply_to) { + binder_set_txn_from_error(in_reply_to, t_debug_id, + return_error, return_error_param); thread->return_error.cmd = BR_TRANSACTION_COMPLETE; binder_enqueue_thread_work(thread, &thread->return_error.work); binder_send_failed_reply(in_reply_to, return_error); } else { + binder_inner_proc_lock(proc); + binder_set_extended_error(&thread->ee, t_debug_id, + return_error, return_error_param); + binder_inner_proc_unlock(proc); thread->return_error.cmd = return_error; binder_enqueue_thread_work(thread, &thread->return_error.work); } @@ -4628,6 +4663,7 @@ static struct binder_thread *binder_get_thread_ilocked( thread->return_error.cmd = BR_OK; thread->reply_error.work.type = BINDER_WORK_RETURN_ERROR; thread->reply_error.cmd = BR_OK; + thread->ee.command = BR_OK; INIT_LIST_HEAD(&new_thread->waiting_thread_node); return thread; } @@ -5066,6 +5102,25 @@ static int binder_ioctl_get_freezer_info( return 0; } +static int binder_ioctl_get_extended_error(struct binder_thread *thread, + void __user *ubuf) +{ + struct binder_extended_error *ee = &thread->ee; + + binder_inner_proc_lock(thread->proc); + if (copy_to_user(ubuf, ee, sizeof(*ee))) { + binder_inner_proc_unlock(thread->proc); + return -EFAULT; + } + + ee->id = 0; + ee->command = BR_OK; + ee->param = 0; + binder_inner_proc_unlock(thread->proc); + + return 0; +} + static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) { int ret; @@ -5274,6 +5329,11 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) binder_inner_proc_unlock(proc); break; } + case BINDER_GET_EXTENDED_ERROR: + ret = binder_ioctl_get_extended_error(thread, ubuf); + if (ret < 0) + goto err; + break; default: ret = -EINVAL; goto err; diff --git a/drivers/android/binder_internal.h b/drivers/android/binder_internal.h index cf70a104594d..8dc0bccf8513 100644 --- a/drivers/android/binder_internal.h +++ b/drivers/android/binder_internal.h @@ -480,6 +480,8 @@ struct binder_proc { * (only accessed by this thread) * @reply_error: transaction errors reported by target thread * (protected by @proc->inner_lock) + * @ee: extended error information from this thread + * (protected by @proc->inner_lock) * @wait: wait queue for thread work * @stats: per-thread statistics * (atomics, no lock needed) @@ -504,6 +506,7 @@ struct binder_thread { bool process_todo; struct binder_error return_error; struct binder_error reply_error; + struct binder_extended_error ee; wait_queue_head_t wait; struct binder_stats stats; atomic_t tmp_ref; diff --git a/include/uapi/linux/android/binder.h b/include/uapi/linux/android/binder.h index 11157fae8a8e..e6ee8cae303b 100644 --- a/include/uapi/linux/android/binder.h +++ b/include/uapi/linux/android/binder.h @@ -236,6 +236,21 @@ struct binder_frozen_status_info { __u32 async_recv; }; +/* struct binder_extened_error - extended error information + * @id: identifier for the failed operation + * @command: command as defined by binder_driver_return_protocol + * @param: parameter holding a negative errno value + * + * Used with BINDER_GET_EXTENDED_ERROR. This extends the error information + * returned by the driver upon a failed operation. Userspace can pull this + * data to properly handle specific error scenarios. + */ +struct binder_extended_error { + __u32 id; + __u32 command; + __s32 param; +}; + #define BINDER_WRITE_READ _IOWR('b', 1, struct binder_write_read) #define BINDER_SET_IDLE_TIMEOUT _IOW('b', 3, __s64) #define BINDER_SET_MAX_THREADS _IOW('b', 5, __u32) @@ -249,6 +264,7 @@ struct binder_frozen_status_info { #define BINDER_FREEZE _IOW('b', 14, struct binder_freeze_info) #define BINDER_GET_FROZEN_INFO _IOWR('b', 15, struct binder_frozen_status_info) #define BINDER_ENABLE_ONEWAY_SPAM_DETECTION _IOW('b', 16, __u32) +#define BINDER_GET_EXTENDED_ERROR _IOWR('b', 17, struct binder_extended_error) /* * NOTE: Two special error codes you should check for when calling -- cgit From 8cc5b032240ae5220b62c689c20459d3e1825b2d Mon Sep 17 00:00:00 2001 From: Carlos Llamas Date: Wed, 1 Jun 2022 01:00:17 +0000 Subject: binder: fix sender_euid type in uapi header The {pid,uid}_t fields of struct binder_transaction were recently replaced to use kernel types in commit 169adc2b6b3c ("android/binder.h: add linux/android/binder(fs).h to UAPI compile-test coverage"). However, using __kernel_uid_t here breaks backwards compatibility in architectures using 16-bits for this type, since glibc and some others still expect a 32-bit uid_t. Instead, let's use __kernel_uid32_t which avoids this compatibility problem. Fixes: 169adc2b6b3c ("android/binder.h: add linux/android/binder(fs).h to UAPI compile-test coverage") Reported-by: Christopher Ferris Signed-off-by: Carlos Llamas Acked-by: Todd Kjos Signed-off-by: Arnd Bergmann --- include/uapi/linux/android/binder.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include/uapi/linux/android') diff --git a/include/uapi/linux/android/binder.h b/include/uapi/linux/android/binder.h index 11157fae8a8e..688bcdaeed53 100644 --- a/include/uapi/linux/android/binder.h +++ b/include/uapi/linux/android/binder.h @@ -289,7 +289,7 @@ struct binder_transaction_data { /* General information about the transaction. */ __u32 flags; __kernel_pid_t sender_pid; - __kernel_uid_t sender_euid; + __kernel_uid32_t sender_euid; binder_size_t data_size; /* number of bytes of data */ binder_size_t offsets_size; /* number of bytes of offsets */ -- cgit From 9864bb4801331daa48514face9d0f4861e4d485b Mon Sep 17 00:00:00 2001 From: Li Li Date: Thu, 26 May 2022 15:00:18 -0700 Subject: Binder: add TF_UPDATE_TXN to replace outdated txn When the target process is busy, incoming oneway transactions are queued in the async_todo list. If the clients continue sending extra oneway transactions while the target process is frozen, this queue can become too large to accommodate new transactions. That's why binder driver introduced ONEWAY_SPAM_DETECTION to detect this situation. It's helpful to debug the async binder buffer exhausting issue, but the issue itself isn't solved directly. In real cases applications are designed to send oneway transactions repeatedly, delivering updated inforamtion to the target process. Typical examples are Wi-Fi signal strength and some real time sensor data. Even if the apps might only care about the lastet information, all outdated oneway transactions are still accumulated there until the frozen process is thawed later. For this kind of situations, there's no existing method to skip those outdated transactions and deliver the latest one only. This patch introduces a new transaction flag TF_UPDATE_TXN. To use it, use apps can set this new flag along with TF_ONE_WAY. When such an oneway transaction is to be queued into the async_todo list of a frozen process, binder driver will check if any previous pending transactions can be superseded by comparing their code, flags and target node. If such an outdated pending transaction is found, the latest transaction will supersede that outdated one. This effectively prevents the async binder buffer running out and saves unnecessary binder read workloads. Acked-by: Todd Kjos Signed-off-by: Li Li Link: https://lore.kernel.org/r/20220526220018.3334775-2-dualli@chromium.org Signed-off-by: Greg Kroah-Hartman --- drivers/android/binder.c | 85 +++++++++++++++++++++++++++++++++++-- drivers/android/binder_trace.h | 4 ++ include/uapi/linux/android/binder.h | 1 + 3 files changed, 87 insertions(+), 3 deletions(-) (limited to 'include/uapi/linux/android') diff --git a/drivers/android/binder.c b/drivers/android/binder.c index 362c0deb65f1..d4f84f25c30b 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -2626,6 +2626,56 @@ static int binder_fixup_parent(struct list_head *pf_head, return binder_add_fixup(pf_head, buffer_offset, bp->buffer, 0); } +/** + * binder_can_update_transaction() - Can a txn be superseded by an updated one? + * @t1: the pending async txn in the frozen process + * @t2: the new async txn to supersede the outdated pending one + * + * Return: true if t2 can supersede t1 + * false if t2 can not supersede t1 + */ +static bool binder_can_update_transaction(struct binder_transaction *t1, + struct binder_transaction *t2) +{ + if ((t1->flags & t2->flags & (TF_ONE_WAY | TF_UPDATE_TXN)) != + (TF_ONE_WAY | TF_UPDATE_TXN) || !t1->to_proc || !t2->to_proc) + return false; + if (t1->to_proc->tsk == t2->to_proc->tsk && t1->code == t2->code && + t1->flags == t2->flags && t1->buffer->pid == t2->buffer->pid && + t1->buffer->target_node->ptr == t2->buffer->target_node->ptr && + t1->buffer->target_node->cookie == t2->buffer->target_node->cookie) + return true; + return false; +} + +/** + * binder_find_outdated_transaction_ilocked() - Find the outdated transaction + * @t: new async transaction + * @target_list: list to find outdated transaction + * + * Return: the outdated transaction if found + * NULL if no outdated transacton can be found + * + * Requires the proc->inner_lock to be held. + */ +static struct binder_transaction * +binder_find_outdated_transaction_ilocked(struct binder_transaction *t, + struct list_head *target_list) +{ + struct binder_work *w; + + list_for_each_entry(w, target_list, entry) { + struct binder_transaction *t_queued; + + if (w->type != BINDER_WORK_TRANSACTION) + continue; + t_queued = container_of(w, struct binder_transaction, work); + if (binder_can_update_transaction(t_queued, t)) + return t_queued; + } + return NULL; +} + /** * binder_proc_transaction() - sends a transaction to a process and wakes it up * @t: transaction to send @@ -2651,6 +2701,7 @@ static int binder_proc_transaction(struct binder_transaction *t, struct binder_node *node = t->buffer->target_node; bool oneway = !!(t->flags & TF_ONE_WAY); bool pending_async = false; + struct binder_transaction *t_outdated = NULL; BUG_ON(!node); binder_node_lock(node); @@ -2678,12 +2729,24 @@ static int binder_proc_transaction(struct binder_transaction *t, if (!thread && !pending_async) thread = binder_select_thread_ilocked(proc); - if (thread) + if (thread) { binder_enqueue_thread_work_ilocked(thread, &t->work); - else if (!pending_async) + } else if (!pending_async) { binder_enqueue_work_ilocked(&t->work, &proc->todo); - else + } else { + if ((t->flags & TF_UPDATE_TXN) && proc->is_frozen) { + t_outdated = binder_find_outdated_transaction_ilocked(t, + &node->async_todo); + if (t_outdated) { + binder_debug(BINDER_DEBUG_TRANSACTION, + "txn %d supersedes %d\n", + t->debug_id, t_outdated->debug_id); + list_del_init(&t_outdated->work.entry); + proc->outstanding_txns--; + } + } binder_enqueue_work_ilocked(&t->work, &node->async_todo); + } if (!pending_async) binder_wakeup_thread_ilocked(proc, thread, !oneway /* sync */); @@ -2692,6 +2755,22 @@ static int binder_proc_transaction(struct binder_transaction *t, binder_inner_proc_unlock(proc); binder_node_unlock(node); + /* + * To reduce potential contention, free the outdated transaction and + * buffer after releasing the locks. + */ + if (t_outdated) { + struct binder_buffer *buffer = t_outdated->buffer; + + t_outdated->buffer = NULL; + buffer->transaction = NULL; + trace_binder_transaction_update_buffer_release(buffer); + binder_transaction_buffer_release(proc, NULL, buffer, 0, 0); + binder_alloc_free_buf(&proc->alloc, buffer); + kfree(t_outdated); + binder_stats_deleted(BINDER_STAT_TRANSACTION); + } + return 0; } diff --git a/drivers/android/binder_trace.h b/drivers/android/binder_trace.h index 8eeccdc64724..8cc07e6a4273 100644 --- a/drivers/android/binder_trace.h +++ b/drivers/android/binder_trace.h @@ -311,6 +311,10 @@ DEFINE_EVENT(binder_buffer_class, binder_transaction_failed_buffer_release, TP_PROTO(struct binder_buffer *buffer), TP_ARGS(buffer)); +DEFINE_EVENT(binder_buffer_class, binder_transaction_update_buffer_release, + TP_PROTO(struct binder_buffer *buffer), + TP_ARGS(buffer)); + TRACE_EVENT(binder_update_page_range, TP_PROTO(struct binder_alloc *alloc, bool allocate, void __user *start, void __user *end), diff --git a/include/uapi/linux/android/binder.h b/include/uapi/linux/android/binder.h index 986333cf5bbe..e72e4de8f452 100644 --- a/include/uapi/linux/android/binder.h +++ b/include/uapi/linux/android/binder.h @@ -287,6 +287,7 @@ enum transaction_flags { TF_STATUS_CODE = 0x08, /* contents are a 32-bit status code */ TF_ACCEPT_FDS = 0x10, /* allow replies with file descriptors */ TF_CLEAR_BUF = 0x20, /* clear buffer on txn complete */ + TF_UPDATE_TXN = 0x40, /* update the outdated pending async txn */ }; struct binder_transaction_data { -- cgit