On Mon, Jan 14, 2019 at 09:10:21AM -0800, Todd Kjos wrote:
> To allow servers to verify client identity, allow a node
> flag to be set that causes the sender's security context
> to be delivered with the transaction. The BR_TRANSACTION
> command is extended in BR_TRANSACTION_SEC_CTX to
> contain a pointer to the security context string.
>
> Signed-off-by: Todd Kjos <tkjos@xxxxxxxxxx>
> ---
> v2: fix 32-bit build warning
> v3: fix smatch warning on unitialized struct element
>
> drivers/android/binder.c | 106 ++++++++++++++++++++++------
> include/uapi/linux/android/binder.h | 19 +++++
> 2 files changed, 102 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index cdfc87629efb8..5f6ef5e63b91e 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -329,6 +329,8 @@ struct binder_error {
> * (invariant after initialized)
> * @min_priority: minimum scheduling priority
> * (invariant after initialized)
> + * @txn_security_ctx: require sender's security context
> + * (invariant after initialized)
> * @async_todo: list of async work items
> * (protected by @proc->inner_lock)
> *
> @@ -365,6 +367,7 @@ struct binder_node {
> * invariant after initialization
> */
> u8 accept_fds:1;
> + u8 txn_security_ctx:1;
> u8 min_priority;
> };
> bool has_async_transaction;
> @@ -615,6 +618,7 @@ struct binder_transaction {
> long saved_priority;
> kuid_t sender_euid;
> struct list_head fd_fixups;
> + binder_uintptr_t security_ctx;
> /**
> * @lock: protects @from, @to_proc, and @to_thread
> *
> @@ -1152,6 +1156,7 @@ static struct binder_node *binder_init_node_ilocked(
> node->work.type = BINDER_WORK_NODE;
> node->min_priority = flags & FLAT_BINDER_FLAG_PRIORITY_MASK;
> node->accept_fds = !!(flags & FLAT_BINDER_FLAG_ACCEPTS_FDS);
> + node->txn_security_ctx = !!(flags & FLAT_BINDER_FLAG_TXN_SECURITY_CTX);
> spin_lock_init(&node->lock);
> INIT_LIST_HEAD(&node->work.entry);
> INIT_LIST_HEAD(&node->async_todo);
> @@ -2778,6 +2783,8 @@ static void binder_transaction(struct binder_proc *proc,
> binder_size_t last_fixup_min_off = 0;
> struct binder_context *context = proc->context;
> int t_debug_id = atomic_inc_return(&binder_last_id);
> + char *secctx = NULL;
> + u32 secctx_sz = 0;
>
> e = binder_transaction_log_add(&binder_transaction_log);
> e->debug_id = t_debug_id;
> @@ -3020,6 +3027,20 @@ static void binder_transaction(struct binder_proc *proc,
> t->flags = tr->flags;
> t->priority = task_nice(current);
>
> + if (target_node && target_node->txn_security_ctx) {
> + u32 secid;
> +
> + security_task_getsecid(proc->tsk, &secid);
> + ret = security_secid_to_secctx(secid, &secctx, &secctx_sz);
> + if (ret) {
> + return_error = BR_FAILED_REPLY;
> + return_error_param = ret;
> + return_error_line = __LINE__;
> + goto err_get_secctx_failed;
> + }
> + extra_buffers_size += ALIGN(secctx_sz, sizeof(u64));
> + }
> +
> trace_binder_transaction(reply, t, target_node);
>
> t->buffer = binder_alloc_new_buf(&target_proc->alloc, tr->data_size,
> @@ -3036,6 +3057,19 @@ static void binder_transaction(struct binder_proc *proc,
> t->buffer = NULL;
> goto err_binder_alloc_buf_failed;
> }
> + if (secctx) {
> + size_t buf_offset = ALIGN(tr->data_size, sizeof(void *)) +
> + ALIGN(tr->offsets_size, sizeof(void *)) +
> + ALIGN(extra_buffers_size, sizeof(void *)) -
> + ALIGN(secctx_sz, sizeof(u64));
> + char *kptr = t->buffer->data + buf_offset;
> +
> + t->security_ctx = (uintptr_t)kptr +
> + binder_alloc_get_user_buffer_offset(&target_proc->alloc);
> + memcpy(kptr, secctx, secctx_sz);
Just for my clarification, instead of storing the string in the transaction
buffer, would it not be better to store the security context id in
t->security_ctx, and then do the conversion to string later, during
binder_thread_read? Then some space will also be saved in the transaction
buffer?
thanks,
- Joel
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
