| Message ID | 5360cd42-5827-58af-515c-6e1ded1d9154@schaufler-ca.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | LSM: Module stacking for SARA and Landlock | expand |
On Fri, Sep 21, 2018 at 5:17 PM, Casey Schaufler <casey@schaufler-ca.com> wrote: > The SELinux specific credential poisioning only makes sense > if SELinux is managing the credentials. As the intent of this > patch set is to move the blob management out of the modules > and into the infrastructure, the SELinux specific code has > to go. The poisioning could be introduced into the infrastructure > at some later date. > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> Reviewed-by: Kees Cook <keescook@chromium.org> -Kees > --- > kernel/cred.c | 13 ------------- > security/selinux/hooks.c | 6 ------ > 2 files changed, 19 deletions(-) > > diff --git a/kernel/cred.c b/kernel/cred.c > index ecf03657e71c..fa2061ee4955 100644 > --- a/kernel/cred.c > +++ b/kernel/cred.c > @@ -704,19 +704,6 @@ bool creds_are_invalid(const struct cred *cred) > { > if (cred->magic != CRED_MAGIC) > return true; > -#ifdef CONFIG_SECURITY_SELINUX > - /* > - * cred->security == NULL if security_cred_alloc_blank() or > - * security_prepare_creds() returned an error. > - */ > - if (selinux_is_enabled() && cred->security) { > - if ((unsigned long) cred->security < PAGE_SIZE) > - return true; > - if ((*(u32 *)cred->security & 0xffffff00) == > - (POISON_FREE << 24 | POISON_FREE << 16 | POISON_FREE << 8)) > - return true; > - } > -#endif > return false; > } > EXPORT_SYMBOL(creds_are_invalid); > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 9d6cdd21acb6..80614ca25a2b 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -3920,12 +3920,6 @@ static void selinux_cred_free(struct cred *cred) > { > struct task_security_struct *tsec = selinux_cred(cred); > > - /* > - * cred->security == NULL if security_cred_alloc_blank() or > - * security_prepare_creds() returned an error. > - */ > - BUG_ON(cred->security && (unsigned long) cred->security < PAGE_SIZE); > - cred->security = (void *) 0x7UL; > kfree(tsec); > } > > -- > 2.17.1 > >
On Fri, 21 Sep 2018, Casey Schaufler wrote: > The SELinux specific credential poisioning only makes sense > if SELinux is managing the credentials. As the intent of this > patch set is to move the blob management out of the modules > and into the infrastructure, the SELinux specific code has > to go. The poisioning could be introduced into the infrastructure > at some later date. If it's useful, it should be incorporated into core LSM, otherwise that's a regression for SELinux.
On 9/27/2018 3:13 PM, James Morris wrote: > On Fri, 21 Sep 2018, Casey Schaufler wrote: > >> The SELinux specific credential poisioning only makes sense >> if SELinux is managing the credentials. As the intent of this >> patch set is to move the blob management out of the modules >> and into the infrastructure, the SELinux specific code has >> to go. The poisioning could be introduced into the infrastructure >> at some later date. > If it's useful, it should be incorporated into core LSM, otherwise that's > a regression for SELinux When I discussed this code with David Howells he indicated that it was primarily used for debugging the original shared credential implementation and that is was not especially valuable any longer. If someone thinks it is valuable we should consider doing it in the infrastructure for all the blobs, not just the credential.
diff --git a/kernel/cred.c b/kernel/cred.c index ecf03657e71c..fa2061ee4955 100644 --- a/kernel/cred.c +++ b/kernel/cred.c @@ -704,19 +704,6 @@ bool creds_are_invalid(const struct cred *cred) { if (cred->magic != CRED_MAGIC) return true; -#ifdef CONFIG_SECURITY_SELINUX - /* - * cred->security == NULL if security_cred_alloc_blank() or - * security_prepare_creds() returned an error. - */ - if (selinux_is_enabled() && cred->security) { - if ((unsigned long) cred->security < PAGE_SIZE) - return true; - if ((*(u32 *)cred->security & 0xffffff00) == - (POISON_FREE << 24 | POISON_FREE << 16 | POISON_FREE << 8)) - return true; - } -#endif return false; } EXPORT_SYMBOL(creds_are_invalid); diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 9d6cdd21acb6..80614ca25a2b 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -3920,12 +3920,6 @@ static void selinux_cred_free(struct cred *cred) { struct task_security_struct *tsec = selinux_cred(cred); - /* - * cred->security == NULL if security_cred_alloc_blank() or - * security_prepare_creds() returned an error. - */ - BUG_ON(cred->security && (unsigned long) cred->security < PAGE_SIZE); - cred->security = (void *) 0x7UL; kfree(tsec); }
The SELinux specific credential poisioning only makes sense if SELinux is managing the credentials. As the intent of this patch set is to move the blob management out of the modules and into the infrastructure, the SELinux specific code has to go. The poisioning could be introduced into the infrastructure at some later date. Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> --- kernel/cred.c | 13 ------------- security/selinux/hooks.c | 6 ------ 2 files changed, 19 deletions(-)