]> git.ipfire.org Git - thirdparty/kernel/stable-queue.git/commitdiff
5.4-stable patches
authorGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Sat, 13 Nov 2021 11:22:59 +0000 (12:22 +0100)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Sat, 13 Nov 2021 11:22:59 +0000 (12:22 +0100)
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

queue-5.4/binder-use-cred-instead-of-task-for-getsecid.patch [new file with mode: 0644]
queue-5.4/binder-use-cred-instead-of-task-for-selinux-checks.patch [new file with mode: 0644]
queue-5.4/binder-use-euid-from-cred-instead-of-using-task.patch [new file with mode: 0644]
queue-5.4/series

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 (file)
index 0000000..a9580f8
--- /dev/null
@@ -0,0 +1,55 @@
+From foo@baz Sat Nov 13 12:18:28 PM CET 2021
+From: Todd Kjos <tkjos@google.com>
+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 <tkjos@google.com>, kernel test robot <lkp@intel.com>, Casey Schaufler <casey@schaufler-ca.com>
+Message-ID: <20211110230025.3272776-3-tkjos@google.com>
+
+From: Todd Kjos <tkjos@google.com>
+
+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 <tkjos@google.com>
+Suggested-by: Stephen Smalley <stephen.smalley.work@gmail.com>
+Reported-by: kernel test robot <lkp@intel.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 |    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 (file)
index 0000000..bea5fd9
--- /dev/null
@@ -0,0 +1,298 @@
+From foo@baz Sat Nov 13 12:18:28 PM CET 2021
+From: Todd Kjos <tkjos@google.com>
+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 <tkjos@google.com>, Casey Schaufler <casey@schaufler-ca.com>
+Message-ID: <20211110230025.3272776-2-tkjos@google.com>
+
+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_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 (file)
index 0000000..7a22df1
--- /dev/null
@@ -0,0 +1,79 @@
+From foo@baz Sat Nov 13 12:18:28 PM CET 2021
+From: Todd Kjos <tkjos@google.com>
+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 <tkjos@google.com>, Casey Schaufler <casey@schaufler-ca.com>
+Message-ID: <20211110230025.3272776-1-tkjos@google.com>
+
+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 |    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 */
index ab7138b0e5c82a93269311ebfea0f0e9ba499105..1ad2b552db86d249591350c718b78c9eaaa2d879 100644 (file)
@@ -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