| Message ID | 20211012165614.2873369-1-tkjos@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | binder: use cred instead of task for security context | expand |
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@schaufler-ca.com>
On Tue, Oct 12, 2021 at 12:56 PM Todd Kjos <tkjos@google.com> 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. Hi Todd, I just merged all three patches into selinux/next, thanks for your help patience on this patchset. Ultimately I merged these patches into selinux/next as opposed to selinux/stable-5.15 because I felt that a couple of weeks in -next before going to Linus would be a good thing. I'm also not certain how widespread binder is outside of Android so I figured the practical difference between next and stable-5.15 is likely very small. Regardless, all of your Fixes and stable tags remain in the patches so as soon as they go up to Linus during the next merge window the stable folks will be notified.
On Thu, Oct 14, 2021 at 2:34 PM Paul Moore <paul@paul-moore.com> wrote: > > On Tue, Oct 12, 2021 at 12:56 PM Todd Kjos <tkjos@google.com> 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. > > Hi Todd, > > I just merged all three patches into selinux/next, thanks for your > help patience on this patchset. Ultimately I merged these patches > into selinux/next as opposed to selinux/stable-5.15 because I felt > that a couple of weeks in -next before going to Linus would be a good > thing. I'm also not certain how widespread binder is outside of > Android so I figured the practical difference between next and > stable-5.15 is likely very small. Regardless, all of your Fixes and > stable tags remain in the patches so as soon as they go up to Linus > during the next merge window the stable folks will be notified. Thanks Paul. This all sounds fine. > > -- > paul moore > www.paul-moore.com