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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 5 Oct 2021 16:05:17 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Paul Moore <paul@...l-moore.com>
Cc:     Todd Kjos <tkjos@...gle.com>, arve@...roid.com, tkjos@...roid.com,
        maco@...roid.com, christian@...uner.io,
        James Morris <jmorris@...ei.org>,
        Serge Hallyn <serge@...lyn.com>,
        Stephen Smalley <stephen.smalley.work@...il.com>,
        Eric Paris <eparis@...isplace.org>, keescook@...omium.org,
        jannh@...gle.com, Jeffrey Vander Stoep <jeffv@...gle.com>,
        zohar@...ux.ibm.com, linux-security-module@...r.kernel.org,
        selinux@...r.kernel.org, devel@...verdev.osuosl.org,
        linux-kernel@...r.kernel.org, joel@...lfernandes.org,
        kernel-team@...roid.com, stable@...r.kernel.org
Subject: Re: [PATCH v2] binder: use cred instead of task for selinux checks

On Tue, Oct 05, 2021 at 09:53:31AM -0400, Paul Moore wrote:
> On Tue, Oct 5, 2021 at 9:31 AM Greg KH <gregkh@...uxfoundation.org> wrote:
> > On Fri, Oct 01, 2021 at 10:55:21AM -0700, Todd Kjos 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@...gle.com>
> > > Cc: stable@...r.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.

Fine with me, take it away!

Acked-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>