From c32ed785e40a9689fc942f47ca07492c5b103d35 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Sat, 13 Nov 2021 12:22:59 +0100 Subject: [PATCH] 5.4-stable patches added patches: binder-use-cred-instead-of-task-for-getsecid.patch binder-use-cred-instead-of-task-for-selinux-checks.patch binder-use-euid-from-cred-instead-of-using-task.patch --- ...se-cred-instead-of-task-for-getsecid.patch | 55 ++++ ...d-instead-of-task-for-selinux-checks.patch | 298 ++++++++++++++++++ ...euid-from-cred-instead-of-using-task.patch | 79 +++++ queue-5.4/series | 3 + 4 files changed, 435 insertions(+) create mode 100644 queue-5.4/binder-use-cred-instead-of-task-for-getsecid.patch create mode 100644 queue-5.4/binder-use-cred-instead-of-task-for-selinux-checks.patch create mode 100644 queue-5.4/binder-use-euid-from-cred-instead-of-using-task.patch diff --git a/queue-5.4/binder-use-cred-instead-of-task-for-getsecid.patch b/queue-5.4/binder-use-cred-instead-of-task-for-getsecid.patch new file mode 100644 index 00000000000..a9580f885f4 --- /dev/null +++ b/queue-5.4/binder-use-cred-instead-of-task-for-getsecid.patch @@ -0,0 +1,55 @@ +From foo@baz Sat Nov 13 12:18:28 PM CET 2021 +From: Todd Kjos +Date: Wed, 10 Nov 2021 15:00:25 -0800 +Subject: binder: use cred instead of task for getsecid +To: stable@vger.kernel.org, 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 +Cc: joel@joelfernandes.org, kernel-team@android.com, Todd Kjos , kernel test robot , Casey Schaufler +Message-ID: <20211110230025.3272776-3-tkjos@google.com> + +From: Todd Kjos + +commit 4d5b5539742d2554591751b4248b0204d20dcc9d upstream. + +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. + +Cc: stable@vger.kernel.org # 5.4+ +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 +Acked-by: Casey Schaufler +Signed-off-by: Paul Moore +Signed-off-by: Greg Kroah-Hartman +--- + drivers/android/binder.c | 2 +- + include/linux/security.h | 5 +++++ + 2 files changed, 6 insertions(+), 1 deletion(-) + +--- a/drivers/android/binder.c ++++ b/drivers/android/binder.c +@@ -3106,7 +3106,7 @@ static void binder_transaction(struct bi + u32 secid; + size_t added_size; + +- security_task_getsecid(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; +--- a/include/linux/security.h ++++ b/include/linux/security.h +@@ -985,6 +985,11 @@ static inline void security_transfer_cre + { + } + ++static inline void security_cred_getsecid(const struct cred *c, u32 *secid) ++{ ++ *secid = 0; ++} ++ + static inline int security_kernel_act_as(struct cred *cred, u32 secid) + { + return 0; diff --git a/queue-5.4/binder-use-cred-instead-of-task-for-selinux-checks.patch b/queue-5.4/binder-use-cred-instead-of-task-for-selinux-checks.patch new file mode 100644 index 00000000000..bea5fd9bf37 --- /dev/null +++ b/queue-5.4/binder-use-cred-instead-of-task-for-selinux-checks.patch @@ -0,0 +1,298 @@ +From foo@baz Sat Nov 13 12:18:28 PM CET 2021 +From: Todd Kjos +Date: Wed, 10 Nov 2021 15:00:24 -0800 +Subject: binder: use cred instead of task for selinux checks +To: stable@vger.kernel.org, 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 +Cc: joel@joelfernandes.org, kernel-team@android.com, Todd Kjos , Casey Schaufler +Message-ID: <20211110230025.3272776-2-tkjos@google.com> + +From: Todd Kjos + +commit 52f88693378a58094c538662ba652aff0253c4fe upstream. + +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 using the 'struct cred' saved during binder_open and pass +it to the selinux subsystem. + +Cc: stable@vger.kernel.org # 5.14 (need backport for earlier stables) +Fixes: 79af73079d75 ("Add security hooks to binder and implement the hooks for SELinux.") +Suggested-by: Jann Horn +Signed-off-by: Todd Kjos +Acked-by: Casey Schaufler +Signed-off-by: Paul Moore +Signed-off-by: Greg Kroah-Hartman +--- + drivers/android/binder.c | 12 ++++++------ + include/linux/lsm_hooks.h | 28 ++++++++++++++-------------- + include/linux/security.h | 28 ++++++++++++++-------------- + security/security.c | 14 +++++++------- + security/selinux/hooks.c | 36 +++++++++++++++--------------------- + 5 files changed, 56 insertions(+), 62 deletions(-) + +--- a/drivers/android/binder.c ++++ b/drivers/android/binder.c +@@ -2446,7 +2446,7 @@ static int binder_translate_binder(struc + 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; + } +@@ -2492,7 +2492,7 @@ static int binder_translate_handle(struc + 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; + } +@@ -2580,7 +2580,7 @@ static int binder_translate_fd(u32 fd, b + 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; +@@ -2979,8 +2979,8 @@ static void binder_transaction(struct bi + 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__; +@@ -4922,7 +4922,7 @@ static int binder_ioctl_set_ctx_mgr(stru + 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)) { +--- a/include/linux/lsm_hooks.h ++++ b/include/linux/lsm_hooks.h +@@ -1241,22 +1241,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 +@@ -1456,13 +1456,13 @@ + * @what: kernel feature being accessed + */ + union security_list_options { +- int (*binder_set_context_mgr)(struct task_struct *mgr); +- int (*binder_transaction)(struct task_struct *from, +- struct task_struct *to); +- int (*binder_transfer_binder)(struct task_struct *from, +- struct task_struct *to); +- int (*binder_transfer_file)(struct task_struct *from, +- struct task_struct *to, ++ int (*binder_set_context_mgr)(const struct cred *mgr); ++ int (*binder_transaction)(const struct cred *from, ++ const struct cred *to); ++ int (*binder_transfer_binder)(const struct cred *from, ++ const struct cred *to); ++ int (*binder_transfer_file)(const struct cred *from, ++ const struct cred *to, + struct file *file); + + int (*ptrace_access_check)(struct task_struct *child, +--- a/include/linux/security.h ++++ b/include/linux/security.h +@@ -249,13 +249,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, +@@ -481,25 +481,25 @@ static inline int early_security_init(vo + 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; +--- a/security/security.c ++++ b/security/security.c +@@ -670,25 +670,25 @@ static void __init lsm_early_task(struct + + /* 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); + } +--- a/security/selinux/hooks.c ++++ b/security/selinux/hooks.c +@@ -2050,22 +2050,19 @@ static inline u32 open_file_to_av(struct + + /* 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) + { +- u32 mysid = current_sid(); +- u32 mgrsid = task_sid(mgr); +- + return avc_has_perm(&selinux_state, +- mysid, mgrsid, 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(from); +- u32 tosid = task_sid(to); ++ u32 fromsid = cred_sid(from); ++ u32 tosid = cred_sid(to); + int rc; + + if (mysid != fromsid) { +@@ -2076,27 +2073,24 @@ static int selinux_binder_transaction(st + return rc; + } + +- return avc_has_perm(&selinux_state, +- fromsid, tosid, SECCLASS_BINDER, BINDER__CALL, +- NULL); ++ 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) + { +- u32 fromsid = task_sid(from); +- u32 tosid = task_sid(to); +- + return avc_has_perm(&selinux_state, +- fromsid, tosid, SECCLASS_BINDER, BINDER__TRANSFER, ++ 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(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; diff --git a/queue-5.4/binder-use-euid-from-cred-instead-of-using-task.patch b/queue-5.4/binder-use-euid-from-cred-instead-of-using-task.patch new file mode 100644 index 00000000000..7a22df1c61c --- /dev/null +++ b/queue-5.4/binder-use-euid-from-cred-instead-of-using-task.patch @@ -0,0 +1,79 @@ +From foo@baz Sat Nov 13 12:18:28 PM CET 2021 +From: Todd Kjos +Date: Wed, 10 Nov 2021 15:00:23 -0800 +Subject: binder: use euid from cred instead of using task +To: stable@vger.kernel.org, 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 +Cc: joel@joelfernandes.org, kernel-team@android.com, Todd Kjos , Casey Schaufler +Message-ID: <20211110230025.3272776-1-tkjos@google.com> + +From: Todd Kjos + +commit 29bc22ac5e5bc63275e850f0c8fc549e3d0e306b upstream. + +Save the 'struct cred' associated with a binder process +at initial open to avoid potential race conditions +when converting to an euid. + +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. + +Cc: stable@vger.kernel.org # 4.4+ +Fixes: 457b9a6f09f0 ("Staging: android: add binder driver") +Signed-off-by: Todd Kjos +Suggested-by: Stephen Smalley +Suggested-by: Jann Horn +Acked-by: Casey Schaufler +Signed-off-by: Paul Moore +Signed-off-by: Greg Kroah-Hartman +--- + drivers/android/binder.c | 8 +++++++- + 1 file changed, 7 insertions(+), 1 deletion(-) + +--- a/drivers/android/binder.c ++++ b/drivers/android/binder.c +@@ -424,6 +424,9 @@ enum binder_deferred_state { + * (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 +@@ -469,6 +472,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; + bool is_dead; +@@ -3091,7 +3095,7 @@ static void binder_transaction(struct bi + 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; +@@ -4707,6 +4711,7 @@ static void binder_free_proc(struct bind + } + binder_alloc_deferred_release(&proc->alloc); + put_task_struct(proc->tsk); ++ put_cred(proc->cred); + binder_stats_deleted(BINDER_STAT_PROC); + kfree(proc); + } +@@ -5234,6 +5239,7 @@ static int binder_open(struct inode *nod + 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); + proc->default_priority = task_nice(current); + /* binderfs stashes devices in i_private */ diff --git a/queue-5.4/series b/queue-5.4/series index ab7138b0e5c..1ad2b552db8 100644 --- a/queue-5.4/series +++ b/queue-5.4/series @@ -1,2 +1,5 @@ xhci-fix-usb-3.1-enumeration-issues-by-increasing-roothub-power-on-good-delay.patch usb-xhci-enable-runtime-pm-by-default-on-amd-yellow-carp-platform.patch +binder-use-euid-from-cred-instead-of-using-task.patch +binder-use-cred-instead-of-task-for-selinux-checks.patch +binder-use-cred-instead-of-task-for-getsecid.patch -- 2.47.3