On Fri, Oct 1, 2021 at 10:38 AM Paul Moore paul@paul-moore.com wrote:
On Thu, Sep 30, 2021 at 10:45 PM Todd Kjos tkjos@google.com wrote:
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.") Signed-off-by: Todd Kjos tkjos@google.com Cc: stable@vger.kernel.org # 5.14 (need backport for earlier stables)
drivers/android/binder.c | 14 +++++---- drivers/android/binder_internal.h | 3 ++ include/linux/lsm_hook_defs.h | 14 ++++----- include/linux/security.h | 28 +++++++++--------- security/security.c | 14 ++++----- security/selinux/hooks.c | 48 +++++++++---------------------- 6 files changed, 52 insertions(+), 69 deletions(-)
Thanks Todd, I'm happy to see someone with a better understanding of binder than me pitch in to clean this up :) A couple of quick comments/questions below ...
Ooops, I was a little over zealous when trimming my response and I accidentally cut off my comment that the associated comment blocks in include/linux/lsm_hooks.h should also be updated to reflect the binder LSM hook changes.