--- /dev/null
+From 32e9f56a96d8d0f23cb2aeb2a3cd18d40393e787 Mon Sep 17 00:00:00 2001
+From: Todd Kjos <tkjos@google.com>
+Date: Fri, 15 Oct 2021 16:38:11 -0700
+Subject: binder: don't detect sender/target during buffer cleanup
+
+From: Todd Kjos <tkjos@google.com>
+
+commit 32e9f56a96d8d0f23cb2aeb2a3cd18d40393e787 upstream.
+
+When freeing txn buffers, binder_transaction_buffer_release()
+attempts to detect whether the current context is the target by
+comparing current->group_leader to proc->tsk. This is an unreliable
+test. Instead explicitly pass an 'is_failure' boolean.
+
+Detecting the sender was being used as a way to tell if the
+transaction failed to be sent. When cleaning up after
+failing to send a transaction, there is no need to close
+the fds associated with a BINDER_TYPE_FDA object. Now
+'is_failure' can be used to accurately detect this case.
+
+Fixes: 44d8047f1d87 ("binder: use standard functions to allocate fds")
+Cc: stable <stable@vger.kernel.org>
+Acked-by: Christian Brauner <christian.brauner@ubuntu.com>
+Signed-off-by: Todd Kjos <tkjos@google.com>
+Link: https://lore.kernel.org/r/20211015233811.3532235-1-tkjos@google.com
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+---
+ drivers/android/binder.c | 14 +++++++-------
+ 1 file changed, 7 insertions(+), 7 deletions(-)
+
+--- a/drivers/android/binder.c
++++ b/drivers/android/binder.c
+@@ -1870,7 +1870,7 @@ static void binder_transaction_buffer_re
+ binder_dec_node(buffer->target_node, 1, 0);
+
+ off_start_offset = ALIGN(buffer->data_size, sizeof(void *));
+- off_end_offset = is_failure ? failed_at :
++ off_end_offset = is_failure && failed_at ? failed_at :
+ off_start_offset + buffer->offsets_size;
+ for (buffer_offset = off_start_offset; buffer_offset < off_end_offset;
+ buffer_offset += sizeof(binder_size_t)) {
+@@ -1956,9 +1956,8 @@ static void binder_transaction_buffer_re
+ binder_size_t fd_buf_size;
+ binder_size_t num_valid;
+
+- if (proc->tsk != current->group_leader) {
++ if (is_failure) {
+ /*
+- * Nothing to do if running in sender context
+ * The fd fixups have not been applied so no
+ * fds need to be closed.
+ */
+@@ -3176,6 +3175,7 @@ err_invalid_target_handle:
+ * binder_free_buf() - free the specified buffer
+ * @proc: binder proc that owns buffer
+ * @buffer: buffer to be freed
++ * @is_failure: failed to send transaction
+ *
+ * If buffer for an async transaction, enqueue the next async
+ * transaction from the node.
+@@ -3185,7 +3185,7 @@ err_invalid_target_handle:
+ static void
+ binder_free_buf(struct binder_proc *proc,
+ struct binder_thread *thread,
+- struct binder_buffer *buffer)
++ struct binder_buffer *buffer, bool is_failure)
+ {
+ binder_inner_proc_lock(proc);
+ if (buffer->transaction) {
+@@ -3213,7 +3213,7 @@ binder_free_buf(struct binder_proc *proc
+ binder_node_inner_unlock(buf_node);
+ }
+ trace_binder_transaction_buffer_release(buffer);
+- binder_transaction_buffer_release(proc, thread, buffer, 0, false);
++ binder_transaction_buffer_release(proc, thread, buffer, 0, is_failure);
+ binder_alloc_free_buf(&proc->alloc, buffer);
+ }
+
+@@ -3415,7 +3415,7 @@ static int binder_thread_write(struct bi
+ proc->pid, thread->pid, (u64)data_ptr,
+ buffer->debug_id,
+ buffer->transaction ? "active" : "finished");
+- binder_free_buf(proc, thread, buffer);
++ binder_free_buf(proc, thread, buffer, false);
+ break;
+ }
+
+@@ -4108,7 +4108,7 @@ retry:
+ buffer->transaction = NULL;
+ binder_cleanup_transaction(t, "fd fixups failed",
+ BR_FAILED_REPLY);
+- binder_free_buf(proc, thread, buffer);
++ binder_free_buf(proc, thread, buffer, true);
+ binder_debug(BINDER_DEBUG_FAILED_TRANSACTION,
+ "%d:%d %stransaction %d fd fixups failed %d/%d, line %d\n",
+ proc->pid, thread->pid,
--- /dev/null
+From 52f88693378a58094c538662ba652aff0253c4fe Mon Sep 17 00:00:00 2001
+From: Todd Kjos <tkjos@google.com>
+Date: Tue, 12 Oct 2021 09:56:13 -0700
+Subject: binder: use cred instead of task for selinux checks
+
+From: Todd Kjos <tkjos@google.com>
+
+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 <jannh@google.com>
+Signed-off-by: Todd Kjos <tkjos@google.com>
+Acked-by: Casey Schaufler <casey@schaufler-ca.com>
+Signed-off-by: Paul Moore <paul@paul-moore.com>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+---
+ drivers/android/binder.c | 12 +++++-----
+ 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 +++++++++++-------------------------------
+ 6 files changed, 54 insertions(+), 76 deletions(-)
+
+--- a/drivers/android/binder.c
++++ b/drivers/android/binder.c
+@@ -2056,7 +2056,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;
+ }
+@@ -2102,7 +2102,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;
+ }
+@@ -2190,7 +2190,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;
+@@ -2595,8 +2595,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__;
+@@ -4565,7 +4565,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_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)
+--- 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
+--- 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(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
+@@ -747,25 +747,25 @@ static int lsm_superblock_alloc(struct s
+
+ /* 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
+@@ -255,29 +255,6 @@ static inline u32 task_sid_obj(const str
+ 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
+
+ /* 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(st
+ 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;
--- /dev/null
+From 29bc22ac5e5bc63275e850f0c8fc549e3d0e306b Mon Sep 17 00:00:00 2001
+From: Todd Kjos <tkjos@google.com>
+Date: Tue, 12 Oct 2021 09:56:12 -0700
+Subject: binder: use euid from cred instead of using task
+
+From: Todd Kjos <tkjos@google.com>
+
+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 <tkjos@google.com>
+Suggested-by: Stephen Smalley <stephen.smalley.work@gmail.com>
+Suggested-by: Jann Horn <jannh@google.com>
+Acked-by: Casey Schaufler <casey@schaufler-ca.com>
+Signed-off-by: Paul Moore <paul@paul-moore.com>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+---
+ drivers/android/binder.c | 4 +++-
+ drivers/android/binder_internal.h | 4 ++++
+ 2 files changed, 7 insertions(+), 1 deletion(-)
+
+--- a/drivers/android/binder.c
++++ b/drivers/android/binder.c
+@@ -2711,7 +2711,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;
+@@ -4353,6 +4353,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);
+ }
+@@ -5055,6 +5056,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);
+ init_waitqueue_head(&proc->freeze_wait);
+ proc->default_priority = task_nice(current);
+--- 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;
projects / thirdparty / kernel / stable-queue.git / commitdiff
