| Message ID | 20240919155740.29539-2-paul@paul-moore.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | selinux,smack: properly reference the LSM blob in security_watch_key() | expand |
On 9/19/2024 8:57 AM, Paul Moore wrote: > Unfortunately when we migrated the lifecycle management of the key LSM > blob to the LSM framework we forgot to convert the security_watch_key() > callbacks for SELinux and Smack. This patch corrects this by making use > of the selinux_key() and smack_key() helper functions respectively. > > This patch also removes some input checking in the Smack callback as it > is no longer needed. > > Reported-by: syzbot+044fdf24e96093584232@syzkaller.appspotmail.com > Fixes: 5f8d28f6d7d5 ("lsm: infrastructure management of the key security blob") > Signed-off-by: Paul Moore <paul@paul-moore.com> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com> > --- > security/selinux/hooks.c | 2 +- > security/smack/smack_lsm.c | 13 +++---------- > 2 files changed, 4 insertions(+), 11 deletions(-) > > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c > index 81fbfa5b80d4..67baa487cf7a 100644 > --- a/security/selinux/hooks.c > +++ b/security/selinux/hooks.c > @@ -6720,7 +6720,7 @@ static int selinux_key_getsecurity(struct key *key, char **_buffer) > #ifdef CONFIG_KEY_NOTIFICATIONS > static int selinux_watch_key(struct key *key) > { > - struct key_security_struct *ksec = key->security; > + struct key_security_struct *ksec = selinux_key(key); > u32 sid = current_sid(); > > return avc_has_perm(sid, ksec->sid, SECCLASS_KEY, KEY__VIEW, NULL); > diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c > index da0c2bffbd08..563fb404f659 100644 > --- a/security/smack/smack_lsm.c > +++ b/security/smack/smack_lsm.c > @@ -4629,16 +4629,9 @@ static int smack_watch_key(struct key *key) > { > struct smk_audit_info ad; > struct smack_known *tkp = smk_of_current(); > + struct smack_known **blob = smack_key(key); > int rc; > > - if (key == NULL) > - return -EINVAL; > - /* > - * If the key hasn't been initialized give it access so that > - * it may do so. > - */ > - if (key->security == NULL) > - return 0; > /* > * This should not occur > */ > @@ -4653,8 +4646,8 @@ static int smack_watch_key(struct key *key) > ad.a.u.key_struct.key = key->serial; > ad.a.u.key_struct.key_desc = key->description; > #endif > - rc = smk_access(tkp, key->security, MAY_READ, &ad); > - rc = smk_bu_note("key watch", tkp, key->security, MAY_READ, rc); > + rc = smk_access(tkp, *blob, MAY_READ, &ad); > + rc = smk_bu_note("key watch", tkp, *blob, MAY_READ, rc); > return rc; > } > #endif /* CONFIG_KEY_NOTIFICATIONS */
On Thu, Sep 19, 2024 at 12:34 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > On 9/19/2024 8:57 AM, Paul Moore wrote: > > Unfortunately when we migrated the lifecycle management of the key LSM > > blob to the LSM framework we forgot to convert the security_watch_key() > > callbacks for SELinux and Smack. This patch corrects this by making use > > of the selinux_key() and smack_key() helper functions respectively. > > > > This patch also removes some input checking in the Smack callback as it > > is no longer needed. > > > > Reported-by: syzbot+044fdf24e96093584232@syzkaller.appspotmail.com > > Fixes: 5f8d28f6d7d5 ("lsm: infrastructure management of the key security blob") > > Signed-off-by: Paul Moore <paul@paul-moore.com> > > Reviewed-by: Casey Schaufler <casey@schaufler-ca.com> Thanks for the quick review, it looks like syzbot was happy with the patch too so I've merged this into lsm/stable-6.12 and I'll send it up to Linus soon.
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 81fbfa5b80d4..67baa487cf7a 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -6720,7 +6720,7 @@ static int selinux_key_getsecurity(struct key *key, char **_buffer) #ifdef CONFIG_KEY_NOTIFICATIONS static int selinux_watch_key(struct key *key) { - struct key_security_struct *ksec = key->security; + struct key_security_struct *ksec = selinux_key(key); u32 sid = current_sid(); return avc_has_perm(sid, ksec->sid, SECCLASS_KEY, KEY__VIEW, NULL); diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index da0c2bffbd08..563fb404f659 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -4629,16 +4629,9 @@ static int smack_watch_key(struct key *key) { struct smk_audit_info ad; struct smack_known *tkp = smk_of_current(); + struct smack_known **blob = smack_key(key); int rc; - if (key == NULL) - return -EINVAL; - /* - * If the key hasn't been initialized give it access so that - * it may do so. - */ - if (key->security == NULL) - return 0; /* * This should not occur */ @@ -4653,8 +4646,8 @@ static int smack_watch_key(struct key *key) ad.a.u.key_struct.key = key->serial; ad.a.u.key_struct.key_desc = key->description; #endif - rc = smk_access(tkp, key->security, MAY_READ, &ad); - rc = smk_bu_note("key watch", tkp, key->security, MAY_READ, rc); + rc = smk_access(tkp, *blob, MAY_READ, &ad); + rc = smk_bu_note("key watch", tkp, *blob, MAY_READ, rc); return rc; } #endif /* CONFIG_KEY_NOTIFICATIONS */
Unfortunately when we migrated the lifecycle management of the key LSM blob to the LSM framework we forgot to convert the security_watch_key() callbacks for SELinux and Smack. This patch corrects this by making use of the selinux_key() and smack_key() helper functions respectively. This patch also removes some input checking in the Smack callback as it is no longer needed. Reported-by: syzbot+044fdf24e96093584232@syzkaller.appspotmail.com Fixes: 5f8d28f6d7d5 ("lsm: infrastructure management of the key security blob") Signed-off-by: Paul Moore <paul@paul-moore.com> --- security/selinux/hooks.c | 2 +- security/smack/smack_lsm.c | 13 +++---------- 2 files changed, 4 insertions(+), 11 deletions(-)