Re: [PATCH v2] binder: use cred instead of task for selinux checks

On Tue, Oct 5, 2021 at 9:31 AM Greg KH [off-list ref] wrote:

On Fri, Oct 01, 2021 at 10:55:21AM -0700, Todd Kjos wrote:

quoted

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 <redacted>
Cc: stable@vger.kernel.org # 5.14+ (need backport for earlier stables)
---
v2: updated comments as suggested by Paul Moore

 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(-)

Ideally I could get an ack from the security developers before taking
this in my tree...

This should probably go in via one of the security trees, e.g. SELinux
or LSM, rather than the binder/driver tree.

-- 
paul moore
www.paul-moore.com