From patchwork Thu Oct 7 00:46:27 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Todd Kjos X-Patchwork-Id: 12540825 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 696D6C43217 for ; Thu, 7 Oct 2021 00:46:38 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 53037611AE for ; Thu, 7 Oct 2021 00:46:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231183AbhJGAs3 (ORCPT ); Wed, 6 Oct 2021 20:48:29 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39638 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S240072AbhJGAs2 (ORCPT ); Wed, 6 Oct 2021 20:48:28 -0400 Received: from mail-yb1-xb49.google.com (mail-yb1-xb49.google.com [IPv6:2607:f8b0:4864:20::b49]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BAB4FC061753 for ; Wed, 6 Oct 2021 17:46:35 -0700 (PDT) Received: by mail-yb1-xb49.google.com with SMTP id 124-20020a251182000000b005a027223ed9so5666088ybr.13 for ; Wed, 06 Oct 2021 17:46:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=fVYGt4GGFCN2TtSXk5ccVtEifg/XO8gXvHwY8CcBHLQ=; b=pxL12Cg7OTFFtudplacxeq1RHvPOJYCNOfv9frqunX2zscRDBi6bv5cPSbdNoTjIqe EEsLk87nstdOcxFaLWqmZ27aQb6XmpwJAyrGHJR5g/aC9DcFJPAyFmLT98E3HhLusAwY TSD/WkjxnE9CKaWaOUD26qntzfbz9LitjMH1Ysd+ZoNqKvr2WuubTuaSp3XO2lqSgzNA h/ND8F6G3fjf7Ifuqf0OqwPkO2aH1KDUIxx4ImKIOtfqdjQh7q8t3KiegzDEtz8HA4Pe dJxh0Eb2wRdzmrHJBIfc8wlZmzk7cZ6Hbn+E4qFfgE9RJLJZISIKVsO9RP/RpN7CtztV 5rGQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=fVYGt4GGFCN2TtSXk5ccVtEifg/XO8gXvHwY8CcBHLQ=; b=U4FHwL3Gw0SZNcnAKmkTQKFyTPdOCuZPk9KD2Df2ukLan/w+utrvViWsLSvUXLbsVC +lK+niW+JPgO2L8T7TN1TufRETZTdIoXqMwY1FEUosckefRRhMuAQIiEheMGJp3GtrV5 HzO7zpHY6H7rD7CZskULJw8Q8jw3RTs+obPB/EGph4WbjrmQWp4JLnS3bOnAVghB5M85 ZI8bbhMR+sT5u49M+KXdzUwY0ZrT9TBdk9qQxvJhVeQBeaUnCDy0cWfGtPdkmBooOZxf ER5SKdoe2m0KQaI6HIYfHjmEKiqhiryUhAw6dEXoUaCuqRZYq3jQjtcn8gL8Jz1ItIXO Ztcg== X-Gm-Message-State: AOAM531JNpbcNvm8GGx4xHVrSaWvgwsPmCWuGjLy7XooWhlSDg1PH2h5 cd+hAMIxuKz41MHN9CQAirOCZ/61hQ== X-Google-Smtp-Source: ABdhPJzR69SX9tebH8WG8Ow5bPAJpWUGL9u+NllqXCUoBuUgD2wzcXfse5fEDNpFOwPl7xAbQF7YRlI/gg== X-Received: from ava-linux2.mtv.corp.google.com ([2620:15c:211:200:6ff2:347f:ac4a:8a04]) (user=tkjos job=sendgmr) by 2002:a05:6902:102d:: with SMTP id x13mr1491298ybt.369.1633567595007; Wed, 06 Oct 2021 17:46:35 -0700 (PDT) Date: Wed, 6 Oct 2021 17:46:27 -0700 In-Reply-To: <20211007004629.1113572-1-tkjos@google.com> Message-Id: <20211007004629.1113572-2-tkjos@google.com> Mime-Version: 1.0 References: <20211007004629.1113572-1-tkjos@google.com> X-Mailer: git-send-email 2.33.0.800.g4c38ced690-goog Subject: [PATCH v4 1/3] binder: use cred instead of task for selinux checks From: Todd Kjos To: gregkh@linuxfoundation.org, arve@android.com, tkjos@android.com, maco@android.com, christian@brauner.io, jmorris@namei.org, serge@hallyn.com, paul@paul-moore.com, stephen.smalley.work@gmail.com, eparis@parisplace.org, keescook@chromium.org, jannh@google.com, jeffv@google.com, zohar@linux.ibm.com, linux-security-module@vger.kernel.org, selinux@vger.kernel.org, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org Cc: joel@joelfernandes.org, kernel-team@android.com, Todd Kjos , stable@vger.kernel.org Precedence: bulk List-ID: Save the struct cred associated with a binder process at initial open to avoid potential race conditions when converting to a security ID. Since binder was integrated with selinux, it has passed 'struct task_struct' associated with the binder_proc to represent the source and target of transactions. The conversion of task to SID was then done in the hook implementations. It turns out that there are race conditions which can result in an incorrect security context being used. Fix by saving the 'struct cred' during binder_open and pass it to the selinux subsystem. Fixes: 79af73079d75 ("Add security hooks to binder and implement the hooks for SELinux.") Suggested-by: Jann Horn Acked-by: Greg Kroah-Hartman Signed-off-by: Todd Kjos Cc: stable@vger.kernel.org # 5.14 (need backport for earlier stables) --- v2: updated comments as suggested by Paul Moore v3: added 2 patches to fix related issues drivers/android/binder.c | 14 +++++---- drivers/android/binder_internal.h | 4 +++ include/linux/lsm_hook_defs.h | 14 ++++----- include/linux/lsm_hooks.h | 14 ++++----- include/linux/security.h | 28 +++++++++--------- security/security.c | 14 ++++----- security/selinux/hooks.c | 48 +++++++++---------------------- 7 files changed, 60 insertions(+), 76 deletions(-) diff --git a/drivers/android/binder.c b/drivers/android/binder.c index 9edacc8b9768..ca599ebdea4a 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -2056,7 +2056,7 @@ static int binder_translate_binder(struct flat_binder_object *fp, ret = -EINVAL; goto done; } - if (security_binder_transfer_binder(proc->tsk, target_proc->tsk)) { + if (security_binder_transfer_binder(proc->cred, target_proc->cred)) { ret = -EPERM; goto done; } @@ -2102,7 +2102,7 @@ static int binder_translate_handle(struct flat_binder_object *fp, proc->pid, thread->pid, fp->handle); return -EINVAL; } - if (security_binder_transfer_binder(proc->tsk, target_proc->tsk)) { + if (security_binder_transfer_binder(proc->cred, target_proc->cred)) { ret = -EPERM; goto done; } @@ -2190,7 +2190,7 @@ static int binder_translate_fd(u32 fd, binder_size_t fd_offset, ret = -EBADF; goto err_fget; } - ret = security_binder_transfer_file(proc->tsk, target_proc->tsk, file); + ret = security_binder_transfer_file(proc->cred, target_proc->cred, file); if (ret < 0) { ret = -EPERM; goto err_security; @@ -2595,8 +2595,8 @@ static void binder_transaction(struct binder_proc *proc, return_error_line = __LINE__; goto err_invalid_target_handle; } - if (security_binder_transaction(proc->tsk, - target_proc->tsk) < 0) { + if (security_binder_transaction(proc->cred, + target_proc->cred) < 0) { return_error = BR_FAILED_REPLY; return_error_param = -EPERM; return_error_line = __LINE__; @@ -4353,6 +4353,7 @@ static void binder_free_proc(struct binder_proc *proc) } binder_alloc_deferred_release(&proc->alloc); put_task_struct(proc->tsk); + put_cred(proc->cred); binder_stats_deleted(BINDER_STAT_PROC); kfree(proc); } @@ -4564,7 +4565,7 @@ static int binder_ioctl_set_ctx_mgr(struct file *filp, ret = -EBUSY; goto out; } - ret = security_binder_set_context_mgr(proc->tsk); + ret = security_binder_set_context_mgr(proc->cred); if (ret < 0) goto out; if (uid_valid(context->binder_context_mgr_uid)) { @@ -5055,6 +5056,7 @@ static int binder_open(struct inode *nodp, struct file *filp) spin_lock_init(&proc->outer_lock); get_task_struct(current->group_leader); proc->tsk = current->group_leader; + proc->cred = get_cred(filp->f_cred); INIT_LIST_HEAD(&proc->todo); init_waitqueue_head(&proc->freeze_wait); proc->default_priority = task_nice(current); diff --git a/drivers/android/binder_internal.h b/drivers/android/binder_internal.h index 402c4d4362a8..d6b6b8cb7346 100644 --- a/drivers/android/binder_internal.h +++ b/drivers/android/binder_internal.h @@ -364,6 +364,9 @@ struct binder_ref { * (invariant after initialized) * @tsk task_struct for group_leader of process * (invariant after initialized) + * @cred struct cred associated with the `struct file` + * in binder_open() + * (invariant after initialized) * @deferred_work_node: element for binder_deferred_list * (protected by binder_deferred_lock) * @deferred_work: bitmap of deferred work to perform @@ -426,6 +429,7 @@ struct binder_proc { struct list_head waiting_threads; int pid; struct task_struct *tsk; + const struct cred *cred; struct hlist_node deferred_work_node; int deferred_work; int outstanding_txns; diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h index 2adeea44c0d5..61590c1f2d33 100644 --- a/include/linux/lsm_hook_defs.h +++ b/include/linux/lsm_hook_defs.h @@ -26,13 +26,13 @@ * #undef LSM_HOOK * }; */ -LSM_HOOK(int, 0, binder_set_context_mgr, struct task_struct *mgr) -LSM_HOOK(int, 0, binder_transaction, struct task_struct *from, - struct task_struct *to) -LSM_HOOK(int, 0, binder_transfer_binder, struct task_struct *from, - struct task_struct *to) -LSM_HOOK(int, 0, binder_transfer_file, struct task_struct *from, - struct task_struct *to, struct file *file) +LSM_HOOK(int, 0, binder_set_context_mgr, const struct cred *mgr) +LSM_HOOK(int, 0, binder_transaction, const struct cred *from, + const struct cred *to) +LSM_HOOK(int, 0, binder_transfer_binder, const struct cred *from, + const struct cred *to) +LSM_HOOK(int, 0, binder_transfer_file, const struct cred *from, + const struct cred *to, struct file *file) LSM_HOOK(int, 0, ptrace_access_check, struct task_struct *child, unsigned int mode) LSM_HOOK(int, 0, ptrace_traceme, struct task_struct *parent) diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h index 5c4c5c0602cb..59024618554e 100644 --- a/include/linux/lsm_hooks.h +++ b/include/linux/lsm_hooks.h @@ -1313,22 +1313,22 @@ * * @binder_set_context_mgr: * Check whether @mgr is allowed to be the binder context manager. - * @mgr contains the task_struct for the task being registered. + * @mgr contains the struct cred for the current binder process. * Return 0 if permission is granted. * @binder_transaction: * Check whether @from is allowed to invoke a binder transaction call * to @to. - * @from contains the task_struct for the sending task. - * @to contains the task_struct for the receiving task. + * @from contains the struct cred for the sending process. + * @to contains the struct cred for the receiving process. * @binder_transfer_binder: * Check whether @from is allowed to transfer a binder reference to @to. - * @from contains the task_struct for the sending task. - * @to contains the task_struct for the receiving task. + * @from contains the struct cred for the sending process. + * @to contains the struct cred for the receiving process. * @binder_transfer_file: * Check whether @from is allowed to transfer @file to @to. - * @from contains the task_struct for the sending task. + * @from contains the struct cred for the sending process. * @file contains the struct file being transferred. - * @to contains the task_struct for the receiving task. + * @to contains the struct cred for the receiving process. * * @ptrace_access_check: * Check permission before allowing the current process to trace the diff --git a/include/linux/security.h b/include/linux/security.h index 5b7288521300..6344d3362df7 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -258,13 +258,13 @@ extern int security_init(void); extern int early_security_init(void); /* Security operations */ -int security_binder_set_context_mgr(struct task_struct *mgr); -int security_binder_transaction(struct task_struct *from, - struct task_struct *to); -int security_binder_transfer_binder(struct task_struct *from, - struct task_struct *to); -int security_binder_transfer_file(struct task_struct *from, - struct task_struct *to, struct file *file); +int security_binder_set_context_mgr(const struct cred *mgr); +int security_binder_transaction(const struct cred *from, + const struct cred *to); +int security_binder_transfer_binder(const struct cred *from, + const struct cred *to); +int security_binder_transfer_file(const struct cred *from, + const struct cred *to, struct file *file); int security_ptrace_access_check(struct task_struct *child, unsigned int mode); int security_ptrace_traceme(struct task_struct *parent); int security_capget(struct task_struct *target, @@ -508,25 +508,25 @@ static inline int early_security_init(void) return 0; } -static inline int security_binder_set_context_mgr(struct task_struct *mgr) +static inline int security_binder_set_context_mgr(const struct cred *mgr) { return 0; } -static inline int security_binder_transaction(struct task_struct *from, - struct task_struct *to) +static inline int security_binder_transaction(const struct cred *from, + const struct cred *to) { return 0; } -static inline int security_binder_transfer_binder(struct task_struct *from, - struct task_struct *to) +static inline int security_binder_transfer_binder(const struct cred *from, + const struct cred *to) { return 0; } -static inline int security_binder_transfer_file(struct task_struct *from, - struct task_struct *to, +static inline int security_binder_transfer_file(const struct cred *from, + const struct cred *to, struct file *file) { return 0; diff --git a/security/security.c b/security/security.c index 9ffa9e9c5c55..67264cb08fb3 100644 --- a/security/security.c +++ b/security/security.c @@ -747,25 +747,25 @@ static int lsm_superblock_alloc(struct super_block *sb) /* Security operations */ -int security_binder_set_context_mgr(struct task_struct *mgr) +int security_binder_set_context_mgr(const struct cred *mgr) { return call_int_hook(binder_set_context_mgr, 0, mgr); } -int security_binder_transaction(struct task_struct *from, - struct task_struct *to) +int security_binder_transaction(const struct cred *from, + const struct cred *to) { return call_int_hook(binder_transaction, 0, from, to); } -int security_binder_transfer_binder(struct task_struct *from, - struct task_struct *to) +int security_binder_transfer_binder(const struct cred *from, + const struct cred *to) { return call_int_hook(binder_transfer_binder, 0, from, to); } -int security_binder_transfer_file(struct task_struct *from, - struct task_struct *to, struct file *file) +int security_binder_transfer_file(const struct cred *from, + const struct cred *to, struct file *file) { return call_int_hook(binder_transfer_file, 0, from, to, file); } diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index e7ebd45ca345..c8bf3db90c8b 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -255,29 +255,6 @@ static inline u32 task_sid_obj(const struct task_struct *task) return sid; } -/* - * get the security ID of a task for use with binder - */ -static inline u32 task_sid_binder(const struct task_struct *task) -{ - /* - * In many case where this function is used we should be using the - * task's subjective SID, but we can't reliably access the subjective - * creds of a task other than our own so we must use the objective - * creds/SID, which are safe to access. The downside is that if a task - * is temporarily overriding it's creds it will not be reflected here; - * however, it isn't clear that binder would handle that case well - * anyway. - * - * If this ever changes and we can safely reference the subjective - * creds/SID of another task, this function will make it easier to - * identify the various places where we make use of the task SIDs in - * the binder code. It is also likely that we will need to adjust - * the main drivers/android binder code as well. - */ - return task_sid_obj(task); -} - static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dentry); /* @@ -2066,18 +2043,19 @@ static inline u32 open_file_to_av(struct file *file) /* Hook functions begin here. */ -static int selinux_binder_set_context_mgr(struct task_struct *mgr) +static int selinux_binder_set_context_mgr(const struct cred *mgr) { return avc_has_perm(&selinux_state, - current_sid(), task_sid_binder(mgr), SECCLASS_BINDER, + current_sid(), cred_sid(mgr), SECCLASS_BINDER, BINDER__SET_CONTEXT_MGR, NULL); } -static int selinux_binder_transaction(struct task_struct *from, - struct task_struct *to) +static int selinux_binder_transaction(const struct cred *from, + const struct cred *to) { u32 mysid = current_sid(); - u32 fromsid = task_sid_binder(from); + u32 fromsid = cred_sid(from); + u32 tosid = cred_sid(to); int rc; if (mysid != fromsid) { @@ -2088,24 +2066,24 @@ static int selinux_binder_transaction(struct task_struct *from, return rc; } - return avc_has_perm(&selinux_state, fromsid, task_sid_binder(to), + return avc_has_perm(&selinux_state, fromsid, tosid, SECCLASS_BINDER, BINDER__CALL, NULL); } -static int selinux_binder_transfer_binder(struct task_struct *from, - struct task_struct *to) +static int selinux_binder_transfer_binder(const struct cred *from, + const struct cred *to) { return avc_has_perm(&selinux_state, - task_sid_binder(from), task_sid_binder(to), + cred_sid(from), cred_sid(to), SECCLASS_BINDER, BINDER__TRANSFER, NULL); } -static int selinux_binder_transfer_file(struct task_struct *from, - struct task_struct *to, +static int selinux_binder_transfer_file(const struct cred *from, + const struct cred *to, struct file *file) { - u32 sid = task_sid_binder(to); + u32 sid = cred_sid(to); struct file_security_struct *fsec = selinux_file(file); struct dentry *dentry = file->f_path.dentry; struct inode_security_struct *isec; From patchwork Thu Oct 7 00:46:28 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Todd Kjos X-Patchwork-Id: 12540827 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6F7C0C43219 for ; Thu, 7 Oct 2021 00:46:40 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 5C98260FD7 for ; Thu, 7 Oct 2021 00:46:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240129AbhJGAsc (ORCPT ); Wed, 6 Oct 2021 20:48:32 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39662 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S240090AbhJGAsb (ORCPT ); Wed, 6 Oct 2021 20:48:31 -0400 Received: from mail-qv1-xf4a.google.com (mail-qv1-xf4a.google.com [IPv6:2607:f8b0:4864:20::f4a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 496F4C061762 for ; Wed, 6 Oct 2021 17:46:38 -0700 (PDT) Received: by mail-qv1-xf4a.google.com with SMTP id p75-20020a0c90d1000000b0037efc8547d4so4232803qvp.16 for ; Wed, 06 Oct 2021 17:46:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=CH/DW8RotmHfFuCbYoHpP62hkgIiMJ06u9s6VjSYC3s=; b=LYTioxtci52jcphCHe8u1eoYv8kmpvwPXGYh1Uwm8AyktnJEWy1v3mFruvAYtAjn/Q EDHeLfY1iPVnDnivL3Upw6ev+O6/tMPhIBx5fWWGXLpSbTLnd75JRyfeyM9HfKVDrM8S iiYfpH+8BwtyYgPllJRfaYYSFUHw0cdfdS4H2S6iR1BXaG03UGQ0VEX9hxzii5TNIvqt vCbzEgSmb227NEoG+nhMpQhyQX+QrfSPWZu/C4QNW/7v4Ww43bKzkNCkwv+6uAWwZYRi gtswe0KxRCcpP5JmR0encJ5FMEyMWyhl0HJDjzzG7/4/9Klnx3hJL7wNarFLuE9Ma9xl U0Gg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=CH/DW8RotmHfFuCbYoHpP62hkgIiMJ06u9s6VjSYC3s=; b=GR8x5K8HX6taGhIXDA40hSIwvYNptQB2coMVUpUFckxO+7uX10jmGOOUPdxCADSssh 66V8U7r5C52YWmDg/YavlFSglzZUyKLD+yr89Zrrj+lsrRbaWnrBwGkiOtO86510r6Q+ Z990gqSDEo4xIfGF2ma0lNRFp4bPT5teJNcWJ5ZjPWUzGrM+M3VuIhLO5FtwZB7vb6l3 xNm8pnlVD6cAxaCONq1u1B3k4dtmmG+7+cYijdZmJ0UerxyURXfoWhbHmJAW1lNc9Si+ MEVY5do6uYvGAJio3510kA0M2zpqAaFWAEVbi9p+/JFIeqbtJgFITBAcCX+7lhbJEcvZ Lgrw== X-Gm-Message-State: AOAM530Y8U5CmSwrjZ/F1+fBNSJk5NDN2t0B7+Xy6qRZnoV/cYdiF8o2 srgE4sEri10jmO6TdGrqI6QE0dBPUg== X-Google-Smtp-Source: ABdhPJwMqLn/cAh17Y37uR0h1Q2rnNT1/CHPF8cLO2zOiQpjODuYunYRg5mcCdS5tsK+bwE+XPkH/q8wag== X-Received: from ava-linux2.mtv.corp.google.com ([2620:15c:211:200:6ff2:347f:ac4a:8a04]) (user=tkjos job=sendgmr) by 2002:ac8:615c:: with SMTP id d28mr1528098qtm.103.1633567597415; Wed, 06 Oct 2021 17:46:37 -0700 (PDT) Date: Wed, 6 Oct 2021 17:46:28 -0700 In-Reply-To: <20211007004629.1113572-1-tkjos@google.com> Message-Id: <20211007004629.1113572-3-tkjos@google.com> Mime-Version: 1.0 References: <20211007004629.1113572-1-tkjos@google.com> X-Mailer: git-send-email 2.33.0.800.g4c38ced690-goog Subject: [PATCH v4 2/3] binder: use cred instead of task for getsecid From: Todd Kjos To: gregkh@linuxfoundation.org, arve@android.com, tkjos@android.com, maco@android.com, christian@brauner.io, jmorris@namei.org, serge@hallyn.com, paul@paul-moore.com, stephen.smalley.work@gmail.com, eparis@parisplace.org, keescook@chromium.org, jannh@google.com, jeffv@google.com, zohar@linux.ibm.com, linux-security-module@vger.kernel.org, selinux@vger.kernel.org, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org Cc: joel@joelfernandes.org, kernel-team@android.com, Todd Kjos , kernel test robot , stable@vger.kernel.org Precedence: bulk List-ID: Use the 'struct cred' saved at binder_open() to lookup the security ID via security_cred_getsecid(). This ensures that the security context that opened binder is the one used to generate the secctx. Fixes: ec74136ded79 ("binder: create node flag to request sender's security context") Signed-off-by: Todd Kjos Suggested-by: Stephen Smalley Reported-by: kernel test robot Cc: stable@vger.kernel.org # 5.4+ --- v3: added this patch to series v4: fix build-break for !CONFIG_SECURITY drivers/android/binder.c | 11 +---------- include/linux/security.h | 4 ++++ 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/drivers/android/binder.c b/drivers/android/binder.c index ca599ebdea4a..989afd0804ca 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -2722,16 +2722,7 @@ static void binder_transaction(struct binder_proc *proc, u32 secid; size_t added_size; - /* - * Arguably this should be the task's subjective LSM secid but - * we can't reliably access the subjective creds of a task - * other than our own so we must use the objective creds, which - * are safe to access. The downside is that if a task is - * temporarily overriding it's creds it will not be reflected - * here; however, it isn't clear that binder would handle that - * case well anyway. - */ - security_task_getsecid_obj(proc->tsk, &secid); + security_cred_getsecid(proc->cred, &secid); ret = security_secid_to_secctx(secid, &secctx, &secctx_sz); if (ret) { return_error = BR_FAILED_REPLY; diff --git a/include/linux/security.h b/include/linux/security.h index 6344d3362df7..f02cc0211b10 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -1041,6 +1041,10 @@ static inline void security_transfer_creds(struct cred *new, { } +static inline void security_cred_getsecid(const struct cred *c, u32 *secid) +{ +} + static inline int security_kernel_act_as(struct cred *cred, u32 secid) { return 0; From patchwork Thu Oct 7 00:46:29 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Todd Kjos X-Patchwork-Id: 12540829 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C5AC9C433F5 for ; Thu, 7 Oct 2021 00:46:46 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id AEA8460F4A for ; Thu, 7 Oct 2021 00:46:46 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240124AbhJGAsi (ORCPT ); Wed, 6 Oct 2021 20:48:38 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39686 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S240142AbhJGAsd (ORCPT ); Wed, 6 Oct 2021 20:48:33 -0400 Received: from mail-yb1-xb49.google.com (mail-yb1-xb49.google.com [IPv6:2607:f8b0:4864:20::b49]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BE4C4C061764 for ; Wed, 6 Oct 2021 17:46:40 -0700 (PDT) Received: by mail-yb1-xb49.google.com with SMTP id g2-20020a256b02000000b005b71ee6a855so5707036ybc.8 for ; Wed, 06 Oct 2021 17:46:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=zPCs1rEpKuQz4LUSoTxaJ0G3vwf7KAFSlUITLwZs6tU=; b=p7C8NObDL/qgsnHg5/TYMe/e2qJ6I/Lq4YqPZ2gUnySyfykd5F/A2vU/1m3ceyFytV I/7OeC1DNhDBO/Sg1SZmblT8zSDOS20AEaXhLm2H/qlJpIU+UkrwJgwQR6ZhajMJlwdg H0AQ8c2Ji4cC5UGLPFyjbyHiVLS1+KBXJaeDkVYrBsKAL2mDeRIuBNWlxuMFkZA9ZGV8 A7qky4OO4I/PaZtI6fTe4+d1q4NwNuqcs2UJX5Gqm7isuGeOruusB375wUngfU0n4dV1 v6iAo7zDuAInyEyLRn9Rt/0nvTXYRScq+aeug+gsRlashaZxAnGdEcAqgBxeJf60frPU Y76A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=zPCs1rEpKuQz4LUSoTxaJ0G3vwf7KAFSlUITLwZs6tU=; b=YPdf6h9H+9/BRtfEcGYQDmXot8pUzgjqTkWbbVI0oWYKQS8dxDU/ej4BfT7Xp2i1lA 8FJ0P9Zpsxu5W6m5A9iqpjteM88HPsvHqYrry+GFRfU2vqLlUqbgciUWEm1U7bOCR0n0 37nHR9LwljuEurUbJxCMNHr+M+Ud+T4d2DZ1YjdhZFT4L2PqGDOKrs4+5It2WZsoutEv cw01xQaHGiUe0/QX5Vd7fqCDWP4tkhRPYPpS8v6E9xoEtivr9xhRrcHIbeOyJgELEw0f Z/MfWc+b9uYgMhzA0pGn9iBFNYcfnFX3jf0EbW5NV7+NaLYJTybLynvAvf3zZk+sW2J+ N6og== X-Gm-Message-State: AOAM532/48PL2AVIrWfxRWiNcejYhKYEtwg9fRgUQjd7OGeEivDD9r6u 78sP3buhd8NoH6Kpy0lgE9nEQrJ1sg== X-Google-Smtp-Source: ABdhPJyjO6u79sIe1Ctn1/EYwfXuBQtVGMw40kM5yChFcDVNsb4RU39eXJBDBGttGXiLTmWRzysZNXQWxg== X-Received: from ava-linux2.mtv.corp.google.com ([2620:15c:211:200:6ff2:347f:ac4a:8a04]) (user=tkjos job=sendgmr) by 2002:a5b:501:: with SMTP id o1mr1359354ybp.402.1633567600035; Wed, 06 Oct 2021 17:46:40 -0700 (PDT) Date: Wed, 6 Oct 2021 17:46:29 -0700 In-Reply-To: <20211007004629.1113572-1-tkjos@google.com> Message-Id: <20211007004629.1113572-4-tkjos@google.com> Mime-Version: 1.0 References: <20211007004629.1113572-1-tkjos@google.com> X-Mailer: git-send-email 2.33.0.800.g4c38ced690-goog Subject: [PATCH v4 3/3] binder: use euid from cred instead of using task From: Todd Kjos To: gregkh@linuxfoundation.org, arve@android.com, tkjos@android.com, maco@android.com, christian@brauner.io, jmorris@namei.org, serge@hallyn.com, paul@paul-moore.com, stephen.smalley.work@gmail.com, eparis@parisplace.org, keescook@chromium.org, jannh@google.com, jeffv@google.com, zohar@linux.ibm.com, linux-security-module@vger.kernel.org, selinux@vger.kernel.org, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org Cc: joel@joelfernandes.org, kernel-team@android.com, Todd Kjos , stable@vger.kernel.org Precedence: bulk List-ID: Set a transaction's sender_euid from the 'struct cred' saved at binder_open() instead of looking up the euid from the binder proc's 'struct task'. This ensures the euid is associated with the security context that of the task that opened binder. Fixes: 457b9a6f09f0 ("Staging: android: add binder driver") Signed-off-by: Todd Kjos Suggested-by: Stephen Smalley Cc: stable@vger.kernel.org # 4.4+ --- v3: added this patch to series drivers/android/binder.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/android/binder.c b/drivers/android/binder.c index 989afd0804ca..26382e982c5e 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -2711,7 +2711,7 @@ static void binder_transaction(struct binder_proc *proc, t->from = thread; else t->from = NULL; - t->sender_euid = task_euid(proc->tsk); + t->sender_euid = proc->cred->euid; t->to_proc = target_proc; t->to_thread = target_thread; t->code = tr->code;