Re: [PATCH v5 0/3] binder: use cred instead of task for security context

[Casey Schaufler]

On 10/12/2021 9:56 AM, Todd Kjos wrote:
> This series fixes the possible use of an incorrect security context
> when checking selinux permissions, getting a security ID, or lookup
> up the euid.
>
> The previous behavior was to save the group_leader 'struct task_struct'
> in binder_open() and using that to obtain security IDs or euids.
>
> This has been shown to be unreliable, so this series instead saves the
> 'struct cred' of the task that called binder_open(). This cred is used
> for these lookups instead of the task.
>
> v1 and v2 of this series were a single patch "binder: use euid from"
> cred instead of using task". During review, Stephen Smalley identified
> two more related issues so the corresponding patches were added to
> the series.
>
> v3:
> - add 2 patches to fix getsecid and euid
>
> v4:
> - fix minor checkpatch issues
> - fix build-break for !CONFIG_SECURITY
>
> v5:
> - reorder/refactor patches as suggested by Stephen Smalley so eiud fix
>   is first and saves the cred during binder_open()
> - set *secid=0 for !CONFIG_SECURITY version of secuirty_cred_getsecid()
>
> Todd Kjos (3):
>   binder: use euid from cred instead of using task
>   binder: use cred instead of task for selinux checks
>   binder: use cred instead of task for getsecid
>
>  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(-)

For the series:
	Acked-by: Casey Schaufler <casey@xxxxxxxxxxxxxxxx>