| Message ID | 20191216223621.5127-4-casey@schaufler-ca.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v12,01/25] LSM: Infrastructure management of the sock security | expand |
On 12/16/19 5:35 PM, Casey Schaufler wrote: > Change the secid parameter of security_audit_rule_match > to a lsmblob structure pointer. Pass the entry from the > lsmblob structure for the approprite slot to the LSM hook. > > Change the users of security_audit_rule_match to use the > lsmblob instead of a u32. In some cases this requires a > temporary conversion using lsmblob_init() that will go > away when other interfaces get converted. > > Reviewed-by: Kees Cook <keescook@chromium.org> > Reviewed-by: John Johansen <john.johansen@canonical.com> > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > --- > include/linux/security.h | 7 ++++--- > kernel/auditfilter.c | 7 +++++-- > kernel/auditsc.c | 14 ++++++++++---- > security/integrity/ima/ima.h | 4 ++-- > security/integrity/ima/ima_policy.c | 7 +++++-- > security/security.c | 18 +++++++++++++++--- > 6 files changed, 41 insertions(+), 16 deletions(-) > > diff --git a/include/linux/security.h b/include/linux/security.h > index b74dc70088ca..9c6dbe248eaf 100644 > --- a/include/linux/security.h > +++ b/include/linux/security.h > @@ -1837,7 +1837,8 @@ static inline int security_key_getsecurity(struct key *key, char **_buffer) > #ifdef CONFIG_SECURITY > int security_audit_rule_init(u32 field, u32 op, char *rulestr, void **lsmrule); > int security_audit_rule_known(struct audit_krule *krule); > -int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule); > +int security_audit_rule_match(struct lsmblob *blob, u32 field, u32 op, > + void *lsmrule); > void security_audit_rule_free(void *lsmrule); > > #else > @@ -1853,8 +1854,8 @@ static inline int security_audit_rule_known(struct audit_krule *krule) > return 0; > } > > -static inline int security_audit_rule_match(u32 secid, u32 field, u32 op, > - void *lsmrule) > +static inline int security_audit_rule_match(struct lsmblob *blob, u32 field, > + u32 op, void *lsmrule) > { > return 0; > } > diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c > index b0126e9c0743..356db1dd276c 100644 > --- a/kernel/auditfilter.c > +++ b/kernel/auditfilter.c > @@ -1325,6 +1325,7 @@ int audit_filter(int msgtype, unsigned int listtype) > struct audit_field *f = &e->rule.fields[i]; > pid_t pid; > u32 sid; > + struct lsmblob blob; > > switch (f->type) { > case AUDIT_PID: > @@ -1355,8 +1356,10 @@ int audit_filter(int msgtype, unsigned int listtype) > case AUDIT_SUBJ_CLR: > if (f->lsm_rule) { > security_task_getsecid(current, &sid); > - result = security_audit_rule_match(sid, > - f->type, f->op, f->lsm_rule); > + lsmblob_init(&blob, sid); > + result = security_audit_rule_match( > + &blob, f->type, > + f->op, f->lsm_rule); > } > break; > case AUDIT_EXE: > diff --git a/kernel/auditsc.c b/kernel/auditsc.c > index 4effe01ebbe2..7566e5b1c419 100644 > --- a/kernel/auditsc.c > +++ b/kernel/auditsc.c > @@ -445,6 +445,7 @@ static int audit_filter_rules(struct task_struct *tsk, > const struct cred *cred; > int i, need_sid = 1; > u32 sid; > + struct lsmblob blob; > unsigned int sessionid; > > cred = rcu_dereference_check(tsk->cred, tsk == current || task_creation); > @@ -643,7 +644,9 @@ static int audit_filter_rules(struct task_struct *tsk, > security_task_getsecid(tsk, &sid); > need_sid = 0; > } > - result = security_audit_rule_match(sid, f->type, > + lsmblob_init(&blob, sid); > + result = security_audit_rule_match(&blob, > + f->type, > f->op, > f->lsm_rule); > } > @@ -658,15 +661,17 @@ static int audit_filter_rules(struct task_struct *tsk, > if (f->lsm_rule) { > /* Find files that match */ > if (name) { > + lsmblob_init(&blob, name->osid); > result = security_audit_rule_match( > - name->osid, > + &blob, > f->type, > f->op, > f->lsm_rule); > } else if (ctx) { > list_for_each_entry(n, &ctx->names_list, list) { > + lsmblob_init(&blob, n->osid); > if (security_audit_rule_match( > - n->osid, > + &blob, > f->type, > f->op, > f->lsm_rule)) { > @@ -678,7 +683,8 @@ static int audit_filter_rules(struct task_struct *tsk, > /* Find ipc objects that match */ > if (!ctx || ctx->type != AUDIT_IPC) > break; > - if (security_audit_rule_match(ctx->ipc.osid, > + lsmblob_init(&blob, ctx->ipc.osid); > + if (security_audit_rule_match(&blob, > f->type, f->op, > f->lsm_rule)) > ++result; > diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h > index df4ca482fb53..d95b0ece7434 100644 > --- a/security/integrity/ima/ima.h > +++ b/security/integrity/ima/ima.h > @@ -381,8 +381,8 @@ static inline int security_filter_rule_init(u32 field, u32 op, char *rulestr, > return -EINVAL; > } > > -static inline int security_filter_rule_match(u32 secid, u32 field, u32 op, > - void *lsmrule) > +static inline int security_filter_rule_match(struct lsmblob *blob, u32 field, > + u32 op, void *lsmrule) > { > return -EINVAL; > } > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c > index f19a895ad7cd..193ddd55420b 100644 > --- a/security/integrity/ima/ima_policy.c > +++ b/security/integrity/ima/ima_policy.c > @@ -414,6 +414,7 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode, > for (i = 0; i < MAX_LSM_RULES; i++) { > int rc = 0; > u32 osid; > + struct lsmblob blob; > > if (!rule->lsm[i].rule) > continue; > @@ -423,7 +424,8 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode, > case LSM_OBJ_ROLE: > case LSM_OBJ_TYPE: > security_inode_getsecid(inode, &osid); > - rc = security_filter_rule_match(osid, > + lsmblob_init(&blob, osid); > + rc = security_filter_rule_match(&blob, > rule->lsm[i].type, > Audit_equal, > rule->lsm[i].rule); > @@ -431,7 +433,8 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode, > case LSM_SUBJ_USER: > case LSM_SUBJ_ROLE: > case LSM_SUBJ_TYPE: > - rc = security_filter_rule_match(secid, > + lsmblob_init(&blob, secid); > + rc = security_filter_rule_match(&blob, > rule->lsm[i].type, > Audit_equal, > rule->lsm[i].rule); > diff --git a/security/security.c b/security/security.c > index a89634af639a..bfea9739c084 100644 > --- a/security/security.c > +++ b/security/security.c > @@ -439,7 +439,7 @@ static int lsm_append(const char *new, char **result) > /* > * Current index to use while initializing the lsmblob secid list. > */ > -static int lsm_slot __initdata; > +static int lsm_slot __lsm_ro_after_init; > > /** > * security_add_hooks - Add a modules hooks to the hook lists. > @@ -2412,9 +2412,21 @@ void security_audit_rule_free(void *lsmrule) > call_void_hook(audit_rule_free, lsmrule); > } > > -int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule) > +int security_audit_rule_match(struct lsmblob *blob, u32 field, u32 op, > + void *lsmrule) > { > - return call_int_hook(audit_rule_match, 0, secid, field, op, lsmrule); > + struct security_hook_list *hp; > + int rc; > + > + hlist_for_each_entry(hp, &security_hook_heads.audit_rule_match, list) { > + if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot)) > + continue; Do you think we really need to retain these WARN_ON()s? If not, then you could dispense with it now and leave lsm_slot as __initdata? Otherwise, Acked-by: Stephen Smalley <sds@tycho.nsa.gov> > + rc = hp->hook.audit_rule_match(blob->secid[hp->lsmid->slot], > + field, op, lsmrule); > + if (rc != 0) > + return rc; > + } > + return 0; > } > #endif /* CONFIG_AUDIT */ > >
On 12/17/2019 9:34 AM, Stephen Smalley wrote: > On 12/16/19 5:35 PM, Casey Schaufler wrote: >> Change the secid parameter of security_audit_rule_match >> to a lsmblob structure pointer. Pass the entry from the >> lsmblob structure for the approprite slot to the LSM hook. >> >> Change the users of security_audit_rule_match to use the >> lsmblob instead of a u32. In some cases this requires a >> temporary conversion using lsmblob_init() that will go >> away when other interfaces get converted. >> >> Reviewed-by: Kees Cook <keescook@chromium.org> >> Reviewed-by: John Johansen <john.johansen@canonical.com> >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> >> --- >> include/linux/security.h | 7 ++++--- >> kernel/auditfilter.c | 7 +++++-- >> kernel/auditsc.c | 14 ++++++++++---- >> security/integrity/ima/ima.h | 4 ++-- >> security/integrity/ima/ima_policy.c | 7 +++++-- >> security/security.c | 18 +++++++++++++++--- >> 6 files changed, 41 insertions(+), 16 deletions(-) >> >> diff --git a/include/linux/security.h b/include/linux/security.h >> index b74dc70088ca..9c6dbe248eaf 100644 >> --- a/include/linux/security.h >> +++ b/include/linux/security.h >> @@ -1837,7 +1837,8 @@ static inline int security_key_getsecurity(struct key *key, char **_buffer) >> #ifdef CONFIG_SECURITY >> int security_audit_rule_init(u32 field, u32 op, char *rulestr, void **lsmrule); >> int security_audit_rule_known(struct audit_krule *krule); >> -int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule); >> +int security_audit_rule_match(struct lsmblob *blob, u32 field, u32 op, >> + void *lsmrule); >> void security_audit_rule_free(void *lsmrule); >> #else >> @@ -1853,8 +1854,8 @@ static inline int security_audit_rule_known(struct audit_krule *krule) >> return 0; >> } >> -static inline int security_audit_rule_match(u32 secid, u32 field, u32 op, >> - void *lsmrule) >> +static inline int security_audit_rule_match(struct lsmblob *blob, u32 field, >> + u32 op, void *lsmrule) >> { >> return 0; >> } >> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c >> index b0126e9c0743..356db1dd276c 100644 >> --- a/kernel/auditfilter.c >> +++ b/kernel/auditfilter.c >> @@ -1325,6 +1325,7 @@ int audit_filter(int msgtype, unsigned int listtype) >> struct audit_field *f = &e->rule.fields[i]; >> pid_t pid; >> u32 sid; >> + struct lsmblob blob; >> switch (f->type) { >> case AUDIT_PID: >> @@ -1355,8 +1356,10 @@ int audit_filter(int msgtype, unsigned int listtype) >> case AUDIT_SUBJ_CLR: >> if (f->lsm_rule) { >> security_task_getsecid(current, &sid); >> - result = security_audit_rule_match(sid, >> - f->type, f->op, f->lsm_rule); >> + lsmblob_init(&blob, sid); >> + result = security_audit_rule_match( >> + &blob, f->type, >> + f->op, f->lsm_rule); >> } >> break; >> case AUDIT_EXE: >> diff --git a/kernel/auditsc.c b/kernel/auditsc.c >> index 4effe01ebbe2..7566e5b1c419 100644 >> --- a/kernel/auditsc.c >> +++ b/kernel/auditsc.c >> @@ -445,6 +445,7 @@ static int audit_filter_rules(struct task_struct *tsk, >> const struct cred *cred; >> int i, need_sid = 1; >> u32 sid; >> + struct lsmblob blob; >> unsigned int sessionid; >> cred = rcu_dereference_check(tsk->cred, tsk == current || task_creation); >> @@ -643,7 +644,9 @@ static int audit_filter_rules(struct task_struct *tsk, >> security_task_getsecid(tsk, &sid); >> need_sid = 0; >> } >> - result = security_audit_rule_match(sid, f->type, >> + lsmblob_init(&blob, sid); >> + result = security_audit_rule_match(&blob, >> + f->type, >> f->op, >> f->lsm_rule); >> } >> @@ -658,15 +661,17 @@ static int audit_filter_rules(struct task_struct *tsk, >> if (f->lsm_rule) { >> /* Find files that match */ >> if (name) { >> + lsmblob_init(&blob, name->osid); >> result = security_audit_rule_match( >> - name->osid, >> + &blob, >> f->type, >> f->op, >> f->lsm_rule); >> } else if (ctx) { >> list_for_each_entry(n, &ctx->names_list, list) { >> + lsmblob_init(&blob, n->osid); >> if (security_audit_rule_match( >> - n->osid, >> + &blob, >> f->type, >> f->op, >> f->lsm_rule)) { >> @@ -678,7 +683,8 @@ static int audit_filter_rules(struct task_struct *tsk, >> /* Find ipc objects that match */ >> if (!ctx || ctx->type != AUDIT_IPC) >> break; >> - if (security_audit_rule_match(ctx->ipc.osid, >> + lsmblob_init(&blob, ctx->ipc.osid); >> + if (security_audit_rule_match(&blob, >> f->type, f->op, >> f->lsm_rule)) >> ++result; >> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h >> index df4ca482fb53..d95b0ece7434 100644 >> --- a/security/integrity/ima/ima.h >> +++ b/security/integrity/ima/ima.h >> @@ -381,8 +381,8 @@ static inline int security_filter_rule_init(u32 field, u32 op, char *rulestr, >> return -EINVAL; >> } >> -static inline int security_filter_rule_match(u32 secid, u32 field, u32 op, >> - void *lsmrule) >> +static inline int security_filter_rule_match(struct lsmblob *blob, u32 field, >> + u32 op, void *lsmrule) >> { >> return -EINVAL; >> } >> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c >> index f19a895ad7cd..193ddd55420b 100644 >> --- a/security/integrity/ima/ima_policy.c >> +++ b/security/integrity/ima/ima_policy.c >> @@ -414,6 +414,7 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode, >> for (i = 0; i < MAX_LSM_RULES; i++) { >> int rc = 0; >> u32 osid; >> + struct lsmblob blob; >> if (!rule->lsm[i].rule) >> continue; >> @@ -423,7 +424,8 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode, >> case LSM_OBJ_ROLE: >> case LSM_OBJ_TYPE: >> security_inode_getsecid(inode, &osid); >> - rc = security_filter_rule_match(osid, >> + lsmblob_init(&blob, osid); >> + rc = security_filter_rule_match(&blob, >> rule->lsm[i].type, >> Audit_equal, >> rule->lsm[i].rule); >> @@ -431,7 +433,8 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode, >> case LSM_SUBJ_USER: >> case LSM_SUBJ_ROLE: >> case LSM_SUBJ_TYPE: >> - rc = security_filter_rule_match(secid, >> + lsmblob_init(&blob, secid); >> + rc = security_filter_rule_match(&blob, >> rule->lsm[i].type, >> Audit_equal, >> rule->lsm[i].rule); >> diff --git a/security/security.c b/security/security.c >> index a89634af639a..bfea9739c084 100644 >> --- a/security/security.c >> +++ b/security/security.c >> @@ -439,7 +439,7 @@ static int lsm_append(const char *new, char **result) >> /* >> * Current index to use while initializing the lsmblob secid list. >> */ >> -static int lsm_slot __initdata; >> +static int lsm_slot __lsm_ro_after_init; >> /** >> * security_add_hooks - Add a modules hooks to the hook lists. >> @@ -2412,9 +2412,21 @@ void security_audit_rule_free(void *lsmrule) >> call_void_hook(audit_rule_free, lsmrule); >> } >> -int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule) >> +int security_audit_rule_match(struct lsmblob *blob, u32 field, u32 op, >> + void *lsmrule) >> { >> - return call_int_hook(audit_rule_match, 0, secid, field, op, lsmrule); >> + struct security_hook_list *hp; >> + int rc; >> + >> + hlist_for_each_entry(hp, &security_hook_heads.audit_rule_match, list) { >> + if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot)) >> + continue; > > Do you think we really need to retain these WARN_ON()s? Kees was especially keen on having the WARN_ON(). I'd be fine with removing it. > If not, then you could dispense with it now and leave lsm_slot as __initdata? Otherwise, > Acked-by: Stephen Smalley <sds@tycho.nsa.gov> > >> + rc = hp->hook.audit_rule_match(blob->secid[hp->lsmid->slot], >> + field, op, lsmrule); >> + if (rc != 0) >> + return rc; >> + } >> + return 0; >> } >> #endif /* CONFIG_AUDIT */ >> >
On Tue, Dec 17, 2019 at 02:01:19PM -0800, Casey Schaufler wrote: > On 12/17/2019 9:34 AM, Stephen Smalley wrote: > > On 12/16/19 5:35 PM, Casey Schaufler wrote: > >> Change the secid parameter of security_audit_rule_match > >> to a lsmblob structure pointer. Pass the entry from the > >> lsmblob structure for the approprite slot to the LSM hook. > >> > >> Change the users of security_audit_rule_match to use the > >> lsmblob instead of a u32. In some cases this requires a > >> temporary conversion using lsmblob_init() that will go > >> away when other interfaces get converted. > >> > >> Reviewed-by: Kees Cook <keescook@chromium.org> > >> Reviewed-by: John Johansen <john.johansen@canonical.com> > >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > >> --- > >> include/linux/security.h | 7 ++++--- > >> kernel/auditfilter.c | 7 +++++-- > >> kernel/auditsc.c | 14 ++++++++++---- > >> security/integrity/ima/ima.h | 4 ++-- > >> security/integrity/ima/ima_policy.c | 7 +++++-- > >> security/security.c | 18 +++++++++++++++--- > >> 6 files changed, 41 insertions(+), 16 deletions(-) > >> > >> diff --git a/include/linux/security.h b/include/linux/security.h > >> index b74dc70088ca..9c6dbe248eaf 100644 > >> --- a/include/linux/security.h > >> +++ b/include/linux/security.h > >> @@ -1837,7 +1837,8 @@ static inline int security_key_getsecurity(struct key *key, char **_buffer) > >> #ifdef CONFIG_SECURITY > >> int security_audit_rule_init(u32 field, u32 op, char *rulestr, void **lsmrule); > >> int security_audit_rule_known(struct audit_krule *krule); > >> -int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule); > >> +int security_audit_rule_match(struct lsmblob *blob, u32 field, u32 op, > >> + void *lsmrule); > >> void security_audit_rule_free(void *lsmrule); > >> #else > >> @@ -1853,8 +1854,8 @@ static inline int security_audit_rule_known(struct audit_krule *krule) > >> return 0; > >> } > >> -static inline int security_audit_rule_match(u32 secid, u32 field, u32 op, > >> - void *lsmrule) > >> +static inline int security_audit_rule_match(struct lsmblob *blob, u32 field, > >> + u32 op, void *lsmrule) > >> { > >> return 0; > >> } > >> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c > >> index b0126e9c0743..356db1dd276c 100644 > >> --- a/kernel/auditfilter.c > >> +++ b/kernel/auditfilter.c > >> @@ -1325,6 +1325,7 @@ int audit_filter(int msgtype, unsigned int listtype) > >> struct audit_field *f = &e->rule.fields[i]; > >> pid_t pid; > >> u32 sid; > >> + struct lsmblob blob; > >> switch (f->type) { > >> case AUDIT_PID: > >> @@ -1355,8 +1356,10 @@ int audit_filter(int msgtype, unsigned int listtype) > >> case AUDIT_SUBJ_CLR: > >> if (f->lsm_rule) { > >> security_task_getsecid(current, &sid); > >> - result = security_audit_rule_match(sid, > >> - f->type, f->op, f->lsm_rule); > >> + lsmblob_init(&blob, sid); > >> + result = security_audit_rule_match( > >> + &blob, f->type, > >> + f->op, f->lsm_rule); > >> } > >> break; > >> case AUDIT_EXE: > >> diff --git a/kernel/auditsc.c b/kernel/auditsc.c > >> index 4effe01ebbe2..7566e5b1c419 100644 > >> --- a/kernel/auditsc.c > >> +++ b/kernel/auditsc.c > >> @@ -445,6 +445,7 @@ static int audit_filter_rules(struct task_struct *tsk, > >> const struct cred *cred; > >> int i, need_sid = 1; > >> u32 sid; > >> + struct lsmblob blob; > >> unsigned int sessionid; > >> cred = rcu_dereference_check(tsk->cred, tsk == current || task_creation); > >> @@ -643,7 +644,9 @@ static int audit_filter_rules(struct task_struct *tsk, > >> security_task_getsecid(tsk, &sid); > >> need_sid = 0; > >> } > >> - result = security_audit_rule_match(sid, f->type, > >> + lsmblob_init(&blob, sid); > >> + result = security_audit_rule_match(&blob, > >> + f->type, > >> f->op, > >> f->lsm_rule); > >> } > >> @@ -658,15 +661,17 @@ static int audit_filter_rules(struct task_struct *tsk, > >> if (f->lsm_rule) { > >> /* Find files that match */ > >> if (name) { > >> + lsmblob_init(&blob, name->osid); > >> result = security_audit_rule_match( > >> - name->osid, > >> + &blob, > >> f->type, > >> f->op, > >> f->lsm_rule); > >> } else if (ctx) { > >> list_for_each_entry(n, &ctx->names_list, list) { > >> + lsmblob_init(&blob, n->osid); > >> if (security_audit_rule_match( > >> - n->osid, > >> + &blob, > >> f->type, > >> f->op, > >> f->lsm_rule)) { > >> @@ -678,7 +683,8 @@ static int audit_filter_rules(struct task_struct *tsk, > >> /* Find ipc objects that match */ > >> if (!ctx || ctx->type != AUDIT_IPC) > >> break; > >> - if (security_audit_rule_match(ctx->ipc.osid, > >> + lsmblob_init(&blob, ctx->ipc.osid); > >> + if (security_audit_rule_match(&blob, > >> f->type, f->op, > >> f->lsm_rule)) > >> ++result; > >> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h > >> index df4ca482fb53..d95b0ece7434 100644 > >> --- a/security/integrity/ima/ima.h > >> +++ b/security/integrity/ima/ima.h > >> @@ -381,8 +381,8 @@ static inline int security_filter_rule_init(u32 field, u32 op, char *rulestr, > >> return -EINVAL; > >> } > >> -static inline int security_filter_rule_match(u32 secid, u32 field, u32 op, > >> - void *lsmrule) > >> +static inline int security_filter_rule_match(struct lsmblob *blob, u32 field, > >> + u32 op, void *lsmrule) > >> { > >> return -EINVAL; > >> } > >> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c > >> index f19a895ad7cd..193ddd55420b 100644 > >> --- a/security/integrity/ima/ima_policy.c > >> +++ b/security/integrity/ima/ima_policy.c > >> @@ -414,6 +414,7 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode, > >> for (i = 0; i < MAX_LSM_RULES; i++) { > >> int rc = 0; > >> u32 osid; > >> + struct lsmblob blob; > >> if (!rule->lsm[i].rule) > >> continue; > >> @@ -423,7 +424,8 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode, > >> case LSM_OBJ_ROLE: > >> case LSM_OBJ_TYPE: > >> security_inode_getsecid(inode, &osid); > >> - rc = security_filter_rule_match(osid, > >> + lsmblob_init(&blob, osid); > >> + rc = security_filter_rule_match(&blob, > >> rule->lsm[i].type, > >> Audit_equal, > >> rule->lsm[i].rule); > >> @@ -431,7 +433,8 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode, > >> case LSM_SUBJ_USER: > >> case LSM_SUBJ_ROLE: > >> case LSM_SUBJ_TYPE: > >> - rc = security_filter_rule_match(secid, > >> + lsmblob_init(&blob, secid); > >> + rc = security_filter_rule_match(&blob, > >> rule->lsm[i].type, > >> Audit_equal, > >> rule->lsm[i].rule); > >> diff --git a/security/security.c b/security/security.c > >> index a89634af639a..bfea9739c084 100644 > >> --- a/security/security.c > >> +++ b/security/security.c > >> @@ -439,7 +439,7 @@ static int lsm_append(const char *new, char **result) > >> /* > >> * Current index to use while initializing the lsmblob secid list. > >> */ > >> -static int lsm_slot __initdata; > >> +static int lsm_slot __lsm_ro_after_init; > >> /** > >> * security_add_hooks - Add a modules hooks to the hook lists. > >> @@ -2412,9 +2412,21 @@ void security_audit_rule_free(void *lsmrule) > >> call_void_hook(audit_rule_free, lsmrule); > >> } > >> -int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule) > >> +int security_audit_rule_match(struct lsmblob *blob, u32 field, u32 op, > >> + void *lsmrule) > >> { > >> - return call_int_hook(audit_rule_match, 0, secid, field, op, lsmrule); > >> + struct security_hook_list *hp; > >> + int rc; > >> + > >> + hlist_for_each_entry(hp, &security_hook_heads.audit_rule_match, list) { > >> + if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot)) > >> + continue; > > > > Do you think we really need to retain these WARN_ON()s? > > Kees was especially keen on having the WARN_ON(). > I'd be fine with removing it. It should really really never happen, so I like the WARN_ON staying. -Kees > > > > If not, then you could dispense with it now and leave lsm_slot as __initdata? Otherwise, > > Acked-by: Stephen Smalley <sds@tycho.nsa.gov> > > > >> + rc = hp->hook.audit_rule_match(blob->secid[hp->lsmid->slot], > >> + field, op, lsmrule); > >> + if (rc != 0) > >> + return rc; > >> + } > >> + return 0; > >> } > >> #endif /* CONFIG_AUDIT */ > >> > >
On 12/17/2019 3:47 PM, Kees Cook wrote: > On Tue, Dec 17, 2019 at 02:01:19PM -0800, Casey Schaufler wrote: >> On 12/17/2019 9:34 AM, Stephen Smalley wrote: >>> On 12/16/19 5:35 PM, Casey Schaufler wrote: >>>> Change the secid parameter of security_audit_rule_match >>>> to a lsmblob structure pointer. Pass the entry from the >>>> lsmblob structure for the approprite slot to the LSM hook. >>>> >>>> Change the users of security_audit_rule_match to use the >>>> lsmblob instead of a u32. In some cases this requires a >>>> temporary conversion using lsmblob_init() that will go >>>> away when other interfaces get converted. >>>> >>>> Reviewed-by: Kees Cook <keescook@chromium.org> >>>> Reviewed-by: John Johansen <john.johansen@canonical.com> >>>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> >>>> --- >>>> include/linux/security.h | 7 ++++--- >>>> kernel/auditfilter.c | 7 +++++-- >>>> kernel/auditsc.c | 14 ++++++++++---- >>>> security/integrity/ima/ima.h | 4 ++-- >>>> security/integrity/ima/ima_policy.c | 7 +++++-- >>>> security/security.c | 18 +++++++++++++++--- >>>> 6 files changed, 41 insertions(+), 16 deletions(-) >>>> >>>> diff --git a/include/linux/security.h b/include/linux/security.h >>>> index b74dc70088ca..9c6dbe248eaf 100644 >>>> --- a/include/linux/security.h >>>> +++ b/include/linux/security.h >>>> @@ -1837,7 +1837,8 @@ static inline int security_key_getsecurity(struct key *key, char **_buffer) >>>> #ifdef CONFIG_SECURITY >>>> int security_audit_rule_init(u32 field, u32 op, char *rulestr, void **lsmrule); >>>> int security_audit_rule_known(struct audit_krule *krule); >>>> -int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule); >>>> +int security_audit_rule_match(struct lsmblob *blob, u32 field, u32 op, >>>> + void *lsmrule); >>>> void security_audit_rule_free(void *lsmrule); >>>> #else >>>> @@ -1853,8 +1854,8 @@ static inline int security_audit_rule_known(struct audit_krule *krule) >>>> return 0; >>>> } >>>> -static inline int security_audit_rule_match(u32 secid, u32 field, u32 op, >>>> - void *lsmrule) >>>> +static inline int security_audit_rule_match(struct lsmblob *blob, u32 field, >>>> + u32 op, void *lsmrule) >>>> { >>>> return 0; >>>> } >>>> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c >>>> index b0126e9c0743..356db1dd276c 100644 >>>> --- a/kernel/auditfilter.c >>>> +++ b/kernel/auditfilter.c >>>> @@ -1325,6 +1325,7 @@ int audit_filter(int msgtype, unsigned int listtype) >>>> struct audit_field *f = &e->rule.fields[i]; >>>> pid_t pid; >>>> u32 sid; >>>> + struct lsmblob blob; >>>> switch (f->type) { >>>> case AUDIT_PID: >>>> @@ -1355,8 +1356,10 @@ int audit_filter(int msgtype, unsigned int listtype) >>>> case AUDIT_SUBJ_CLR: >>>> if (f->lsm_rule) { >>>> security_task_getsecid(current, &sid); >>>> - result = security_audit_rule_match(sid, >>>> - f->type, f->op, f->lsm_rule); >>>> + lsmblob_init(&blob, sid); >>>> + result = security_audit_rule_match( >>>> + &blob, f->type, >>>> + f->op, f->lsm_rule); >>>> } >>>> break; >>>> case AUDIT_EXE: >>>> diff --git a/kernel/auditsc.c b/kernel/auditsc.c >>>> index 4effe01ebbe2..7566e5b1c419 100644 >>>> --- a/kernel/auditsc.c >>>> +++ b/kernel/auditsc.c >>>> @@ -445,6 +445,7 @@ static int audit_filter_rules(struct task_struct *tsk, >>>> const struct cred *cred; >>>> int i, need_sid = 1; >>>> u32 sid; >>>> + struct lsmblob blob; >>>> unsigned int sessionid; >>>> cred = rcu_dereference_check(tsk->cred, tsk == current || task_creation); >>>> @@ -643,7 +644,9 @@ static int audit_filter_rules(struct task_struct *tsk, >>>> security_task_getsecid(tsk, &sid); >>>> need_sid = 0; >>>> } >>>> - result = security_audit_rule_match(sid, f->type, >>>> + lsmblob_init(&blob, sid); >>>> + result = security_audit_rule_match(&blob, >>>> + f->type, >>>> f->op, >>>> f->lsm_rule); >>>> } >>>> @@ -658,15 +661,17 @@ static int audit_filter_rules(struct task_struct *tsk, >>>> if (f->lsm_rule) { >>>> /* Find files that match */ >>>> if (name) { >>>> + lsmblob_init(&blob, name->osid); >>>> result = security_audit_rule_match( >>>> - name->osid, >>>> + &blob, >>>> f->type, >>>> f->op, >>>> f->lsm_rule); >>>> } else if (ctx) { >>>> list_for_each_entry(n, &ctx->names_list, list) { >>>> + lsmblob_init(&blob, n->osid); >>>> if (security_audit_rule_match( >>>> - n->osid, >>>> + &blob, >>>> f->type, >>>> f->op, >>>> f->lsm_rule)) { >>>> @@ -678,7 +683,8 @@ static int audit_filter_rules(struct task_struct *tsk, >>>> /* Find ipc objects that match */ >>>> if (!ctx || ctx->type != AUDIT_IPC) >>>> break; >>>> - if (security_audit_rule_match(ctx->ipc.osid, >>>> + lsmblob_init(&blob, ctx->ipc.osid); >>>> + if (security_audit_rule_match(&blob, >>>> f->type, f->op, >>>> f->lsm_rule)) >>>> ++result; >>>> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h >>>> index df4ca482fb53..d95b0ece7434 100644 >>>> --- a/security/integrity/ima/ima.h >>>> +++ b/security/integrity/ima/ima.h >>>> @@ -381,8 +381,8 @@ static inline int security_filter_rule_init(u32 field, u32 op, char *rulestr, >>>> return -EINVAL; >>>> } >>>> -static inline int security_filter_rule_match(u32 secid, u32 field, u32 op, >>>> - void *lsmrule) >>>> +static inline int security_filter_rule_match(struct lsmblob *blob, u32 field, >>>> + u32 op, void *lsmrule) >>>> { >>>> return -EINVAL; >>>> } >>>> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c >>>> index f19a895ad7cd..193ddd55420b 100644 >>>> --- a/security/integrity/ima/ima_policy.c >>>> +++ b/security/integrity/ima/ima_policy.c >>>> @@ -414,6 +414,7 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode, >>>> for (i = 0; i < MAX_LSM_RULES; i++) { >>>> int rc = 0; >>>> u32 osid; >>>> + struct lsmblob blob; >>>> if (!rule->lsm[i].rule) >>>> continue; >>>> @@ -423,7 +424,8 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode, >>>> case LSM_OBJ_ROLE: >>>> case LSM_OBJ_TYPE: >>>> security_inode_getsecid(inode, &osid); >>>> - rc = security_filter_rule_match(osid, >>>> + lsmblob_init(&blob, osid); >>>> + rc = security_filter_rule_match(&blob, >>>> rule->lsm[i].type, >>>> Audit_equal, >>>> rule->lsm[i].rule); >>>> @@ -431,7 +433,8 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode, >>>> case LSM_SUBJ_USER: >>>> case LSM_SUBJ_ROLE: >>>> case LSM_SUBJ_TYPE: >>>> - rc = security_filter_rule_match(secid, >>>> + lsmblob_init(&blob, secid); >>>> + rc = security_filter_rule_match(&blob, >>>> rule->lsm[i].type, >>>> Audit_equal, >>>> rule->lsm[i].rule); >>>> diff --git a/security/security.c b/security/security.c >>>> index a89634af639a..bfea9739c084 100644 >>>> --- a/security/security.c >>>> +++ b/security/security.c >>>> @@ -439,7 +439,7 @@ static int lsm_append(const char *new, char **result) >>>> /* >>>> * Current index to use while initializing the lsmblob secid list. >>>> */ >>>> -static int lsm_slot __initdata; >>>> +static int lsm_slot __lsm_ro_after_init; >>>> /** >>>> * security_add_hooks - Add a modules hooks to the hook lists. >>>> @@ -2412,9 +2412,21 @@ void security_audit_rule_free(void *lsmrule) >>>> call_void_hook(audit_rule_free, lsmrule); >>>> } >>>> -int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule) >>>> +int security_audit_rule_match(struct lsmblob *blob, u32 field, u32 op, >>>> + void *lsmrule) >>>> { >>>> - return call_int_hook(audit_rule_match, 0, secid, field, op, lsmrule); >>>> + struct security_hook_list *hp; >>>> + int rc; >>>> + >>>> + hlist_for_each_entry(hp, &security_hook_heads.audit_rule_match, list) { >>>> + if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot)) >>>> + continue; >>> Do you think we really need to retain these WARN_ON()s? >> Kees was especially keen on having the WARN_ON(). >> I'd be fine with removing it. > It should really really never happen, so I like the WARN_ON staying. > > -Kees Given that Mr. Hardening likes it the way it is, I'm inclined to leave it as is. Would that prevent an Ack? > >> >>> If not, then you could dispense with it now and leave lsm_slot as __initdata? Otherwise, >>> Acked-by: Stephen Smalley <sds@tycho.nsa.gov> >>> >>>> + rc = hp->hook.audit_rule_match(blob->secid[hp->lsmid->slot], >>>> + field, op, lsmrule); >>>> + if (rc != 0) >>>> + return rc; >>>> + } >>>> + return 0; >>>> } >>>> #endif /* CONFIG_AUDIT */ >>>>
On 12/17/19 7:28 PM, Casey Schaufler wrote: > On 12/17/2019 3:47 PM, Kees Cook wrote: >> On Tue, Dec 17, 2019 at 02:01:19PM -0800, Casey Schaufler wrote: >>> On 12/17/2019 9:34 AM, Stephen Smalley wrote: >>>> On 12/16/19 5:35 PM, Casey Schaufler wrote: >>>>> Change the secid parameter of security_audit_rule_match >>>>> to a lsmblob structure pointer. Pass the entry from the >>>>> lsmblob structure for the approprite slot to the LSM hook. >>>>> >>>>> Change the users of security_audit_rule_match to use the >>>>> lsmblob instead of a u32. In some cases this requires a >>>>> temporary conversion using lsmblob_init() that will go >>>>> away when other interfaces get converted. >>>>> >>>>> Reviewed-by: Kees Cook <keescook@chromium.org> >>>>> Reviewed-by: John Johansen <john.johansen@canonical.com> >>>>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> >>>>> --- >>>>> include/linux/security.h | 7 ++++--- >>>>> kernel/auditfilter.c | 7 +++++-- >>>>> kernel/auditsc.c | 14 ++++++++++---- >>>>> security/integrity/ima/ima.h | 4 ++-- >>>>> security/integrity/ima/ima_policy.c | 7 +++++-- >>>>> security/security.c | 18 +++++++++++++++--- >>>>> 6 files changed, 41 insertions(+), 16 deletions(-) >>>>> >>>>> diff --git a/include/linux/security.h b/include/linux/security.h >>>>> index b74dc70088ca..9c6dbe248eaf 100644 >>>>> --- a/include/linux/security.h >>>>> +++ b/include/linux/security.h >>>>> @@ -1837,7 +1837,8 @@ static inline int security_key_getsecurity(struct key *key, char **_buffer) >>>>> #ifdef CONFIG_SECURITY >>>>> int security_audit_rule_init(u32 field, u32 op, char *rulestr, void **lsmrule); >>>>> int security_audit_rule_known(struct audit_krule *krule); >>>>> -int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule); >>>>> +int security_audit_rule_match(struct lsmblob *blob, u32 field, u32 op, >>>>> + void *lsmrule); >>>>> void security_audit_rule_free(void *lsmrule); >>>>> #else >>>>> @@ -1853,8 +1854,8 @@ static inline int security_audit_rule_known(struct audit_krule *krule) >>>>> return 0; >>>>> } >>>>> -static inline int security_audit_rule_match(u32 secid, u32 field, u32 op, >>>>> - void *lsmrule) >>>>> +static inline int security_audit_rule_match(struct lsmblob *blob, u32 field, >>>>> + u32 op, void *lsmrule) >>>>> { >>>>> return 0; >>>>> } >>>>> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c >>>>> index b0126e9c0743..356db1dd276c 100644 >>>>> --- a/kernel/auditfilter.c >>>>> +++ b/kernel/auditfilter.c >>>>> @@ -1325,6 +1325,7 @@ int audit_filter(int msgtype, unsigned int listtype) >>>>> struct audit_field *f = &e->rule.fields[i]; >>>>> pid_t pid; >>>>> u32 sid; >>>>> + struct lsmblob blob; >>>>> switch (f->type) { >>>>> case AUDIT_PID: >>>>> @@ -1355,8 +1356,10 @@ int audit_filter(int msgtype, unsigned int listtype) >>>>> case AUDIT_SUBJ_CLR: >>>>> if (f->lsm_rule) { >>>>> security_task_getsecid(current, &sid); >>>>> - result = security_audit_rule_match(sid, >>>>> - f->type, f->op, f->lsm_rule); >>>>> + lsmblob_init(&blob, sid); >>>>> + result = security_audit_rule_match( >>>>> + &blob, f->type, >>>>> + f->op, f->lsm_rule); >>>>> } >>>>> break; >>>>> case AUDIT_EXE: >>>>> diff --git a/kernel/auditsc.c b/kernel/auditsc.c >>>>> index 4effe01ebbe2..7566e5b1c419 100644 >>>>> --- a/kernel/auditsc.c >>>>> +++ b/kernel/auditsc.c >>>>> @@ -445,6 +445,7 @@ static int audit_filter_rules(struct task_struct *tsk, >>>>> const struct cred *cred; >>>>> int i, need_sid = 1; >>>>> u32 sid; >>>>> + struct lsmblob blob; >>>>> unsigned int sessionid; >>>>> cred = rcu_dereference_check(tsk->cred, tsk == current || task_creation); >>>>> @@ -643,7 +644,9 @@ static int audit_filter_rules(struct task_struct *tsk, >>>>> security_task_getsecid(tsk, &sid); >>>>> need_sid = 0; >>>>> } >>>>> - result = security_audit_rule_match(sid, f->type, >>>>> + lsmblob_init(&blob, sid); >>>>> + result = security_audit_rule_match(&blob, >>>>> + f->type, >>>>> f->op, >>>>> f->lsm_rule); >>>>> } >>>>> @@ -658,15 +661,17 @@ static int audit_filter_rules(struct task_struct *tsk, >>>>> if (f->lsm_rule) { >>>>> /* Find files that match */ >>>>> if (name) { >>>>> + lsmblob_init(&blob, name->osid); >>>>> result = security_audit_rule_match( >>>>> - name->osid, >>>>> + &blob, >>>>> f->type, >>>>> f->op, >>>>> f->lsm_rule); >>>>> } else if (ctx) { >>>>> list_for_each_entry(n, &ctx->names_list, list) { >>>>> + lsmblob_init(&blob, n->osid); >>>>> if (security_audit_rule_match( >>>>> - n->osid, >>>>> + &blob, >>>>> f->type, >>>>> f->op, >>>>> f->lsm_rule)) { >>>>> @@ -678,7 +683,8 @@ static int audit_filter_rules(struct task_struct *tsk, >>>>> /* Find ipc objects that match */ >>>>> if (!ctx || ctx->type != AUDIT_IPC) >>>>> break; >>>>> - if (security_audit_rule_match(ctx->ipc.osid, >>>>> + lsmblob_init(&blob, ctx->ipc.osid); >>>>> + if (security_audit_rule_match(&blob, >>>>> f->type, f->op, >>>>> f->lsm_rule)) >>>>> ++result; >>>>> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h >>>>> index df4ca482fb53..d95b0ece7434 100644 >>>>> --- a/security/integrity/ima/ima.h >>>>> +++ b/security/integrity/ima/ima.h >>>>> @@ -381,8 +381,8 @@ static inline int security_filter_rule_init(u32 field, u32 op, char *rulestr, >>>>> return -EINVAL; >>>>> } >>>>> -static inline int security_filter_rule_match(u32 secid, u32 field, u32 op, >>>>> - void *lsmrule) >>>>> +static inline int security_filter_rule_match(struct lsmblob *blob, u32 field, >>>>> + u32 op, void *lsmrule) >>>>> { >>>>> return -EINVAL; >>>>> } >>>>> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c >>>>> index f19a895ad7cd..193ddd55420b 100644 >>>>> --- a/security/integrity/ima/ima_policy.c >>>>> +++ b/security/integrity/ima/ima_policy.c >>>>> @@ -414,6 +414,7 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode, >>>>> for (i = 0; i < MAX_LSM_RULES; i++) { >>>>> int rc = 0; >>>>> u32 osid; >>>>> + struct lsmblob blob; >>>>> if (!rule->lsm[i].rule) >>>>> continue; >>>>> @@ -423,7 +424,8 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode, >>>>> case LSM_OBJ_ROLE: >>>>> case LSM_OBJ_TYPE: >>>>> security_inode_getsecid(inode, &osid); >>>>> - rc = security_filter_rule_match(osid, >>>>> + lsmblob_init(&blob, osid); >>>>> + rc = security_filter_rule_match(&blob, >>>>> rule->lsm[i].type, >>>>> Audit_equal, >>>>> rule->lsm[i].rule); >>>>> @@ -431,7 +433,8 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode, >>>>> case LSM_SUBJ_USER: >>>>> case LSM_SUBJ_ROLE: >>>>> case LSM_SUBJ_TYPE: >>>>> - rc = security_filter_rule_match(secid, >>>>> + lsmblob_init(&blob, secid); >>>>> + rc = security_filter_rule_match(&blob, >>>>> rule->lsm[i].type, >>>>> Audit_equal, >>>>> rule->lsm[i].rule); >>>>> diff --git a/security/security.c b/security/security.c >>>>> index a89634af639a..bfea9739c084 100644 >>>>> --- a/security/security.c >>>>> +++ b/security/security.c >>>>> @@ -439,7 +439,7 @@ static int lsm_append(const char *new, char **result) >>>>> /* >>>>> * Current index to use while initializing the lsmblob secid list. >>>>> */ >>>>> -static int lsm_slot __initdata; >>>>> +static int lsm_slot __lsm_ro_after_init; >>>>> /** >>>>> * security_add_hooks - Add a modules hooks to the hook lists. >>>>> @@ -2412,9 +2412,21 @@ void security_audit_rule_free(void *lsmrule) >>>>> call_void_hook(audit_rule_free, lsmrule); >>>>> } >>>>> -int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule) >>>>> +int security_audit_rule_match(struct lsmblob *blob, u32 field, u32 op, >>>>> + void *lsmrule) >>>>> { >>>>> - return call_int_hook(audit_rule_match, 0, secid, field, op, lsmrule); >>>>> + struct security_hook_list *hp; >>>>> + int rc; >>>>> + >>>>> + hlist_for_each_entry(hp, &security_hook_heads.audit_rule_match, list) { >>>>> + if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot)) >>>>> + continue; >>>> Do you think we really need to retain these WARN_ON()s? >>> Kees was especially keen on having the WARN_ON(). >>> I'd be fine with removing it. >> It should really really never happen, so I like the WARN_ON staying. >> >> -Kees > > Given that Mr. Hardening likes it the way it is, I'm inclined to leave > it as is. Would that prevent an Ack? No, I already acked it in my reply, just thought I'd ask about the WARN_ON. > > >> >>> >>>> If not, then you could dispense with it now and leave lsm_slot as __initdata? Otherwise, >>>> Acked-by: Stephen Smalley <sds@tycho.nsa.gov> >>>> >>>>> + rc = hp->hook.audit_rule_match(blob->secid[hp->lsmid->slot], >>>>> + field, op, lsmrule); >>>>> + if (rc != 0) >>>>> + return rc; >>>>> + } >>>>> + return 0; >>>>> } >>>>> #endif /* CONFIG_AUDIT */ >>>>>
diff --git a/include/linux/security.h b/include/linux/security.h index b74dc70088ca..9c6dbe248eaf 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -1837,7 +1837,8 @@ static inline int security_key_getsecurity(struct key *key, char **_buffer) #ifdef CONFIG_SECURITY int security_audit_rule_init(u32 field, u32 op, char *rulestr, void **lsmrule); int security_audit_rule_known(struct audit_krule *krule); -int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule); +int security_audit_rule_match(struct lsmblob *blob, u32 field, u32 op, + void *lsmrule); void security_audit_rule_free(void *lsmrule); #else @@ -1853,8 +1854,8 @@ static inline int security_audit_rule_known(struct audit_krule *krule) return 0; } -static inline int security_audit_rule_match(u32 secid, u32 field, u32 op, - void *lsmrule) +static inline int security_audit_rule_match(struct lsmblob *blob, u32 field, + u32 op, void *lsmrule) { return 0; } diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c index b0126e9c0743..356db1dd276c 100644 --- a/kernel/auditfilter.c +++ b/kernel/auditfilter.c @@ -1325,6 +1325,7 @@ int audit_filter(int msgtype, unsigned int listtype) struct audit_field *f = &e->rule.fields[i]; pid_t pid; u32 sid; + struct lsmblob blob; switch (f->type) { case AUDIT_PID: @@ -1355,8 +1356,10 @@ int audit_filter(int msgtype, unsigned int listtype) case AUDIT_SUBJ_CLR: if (f->lsm_rule) { security_task_getsecid(current, &sid); - result = security_audit_rule_match(sid, - f->type, f->op, f->lsm_rule); + lsmblob_init(&blob, sid); + result = security_audit_rule_match( + &blob, f->type, + f->op, f->lsm_rule); } break; case AUDIT_EXE: diff --git a/kernel/auditsc.c b/kernel/auditsc.c index 4effe01ebbe2..7566e5b1c419 100644 --- a/kernel/auditsc.c +++ b/kernel/auditsc.c @@ -445,6 +445,7 @@ static int audit_filter_rules(struct task_struct *tsk, const struct cred *cred; int i, need_sid = 1; u32 sid; + struct lsmblob blob; unsigned int sessionid; cred = rcu_dereference_check(tsk->cred, tsk == current || task_creation); @@ -643,7 +644,9 @@ static int audit_filter_rules(struct task_struct *tsk, security_task_getsecid(tsk, &sid); need_sid = 0; } - result = security_audit_rule_match(sid, f->type, + lsmblob_init(&blob, sid); + result = security_audit_rule_match(&blob, + f->type, f->op, f->lsm_rule); } @@ -658,15 +661,17 @@ static int audit_filter_rules(struct task_struct *tsk, if (f->lsm_rule) { /* Find files that match */ if (name) { + lsmblob_init(&blob, name->osid); result = security_audit_rule_match( - name->osid, + &blob, f->type, f->op, f->lsm_rule); } else if (ctx) { list_for_each_entry(n, &ctx->names_list, list) { + lsmblob_init(&blob, n->osid); if (security_audit_rule_match( - n->osid, + &blob, f->type, f->op, f->lsm_rule)) { @@ -678,7 +683,8 @@ static int audit_filter_rules(struct task_struct *tsk, /* Find ipc objects that match */ if (!ctx || ctx->type != AUDIT_IPC) break; - if (security_audit_rule_match(ctx->ipc.osid, + lsmblob_init(&blob, ctx->ipc.osid); + if (security_audit_rule_match(&blob, f->type, f->op, f->lsm_rule)) ++result; diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index df4ca482fb53..d95b0ece7434 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -381,8 +381,8 @@ static inline int security_filter_rule_init(u32 field, u32 op, char *rulestr, return -EINVAL; } -static inline int security_filter_rule_match(u32 secid, u32 field, u32 op, - void *lsmrule) +static inline int security_filter_rule_match(struct lsmblob *blob, u32 field, + u32 op, void *lsmrule) { return -EINVAL; } diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index f19a895ad7cd..193ddd55420b 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -414,6 +414,7 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode, for (i = 0; i < MAX_LSM_RULES; i++) { int rc = 0; u32 osid; + struct lsmblob blob; if (!rule->lsm[i].rule) continue; @@ -423,7 +424,8 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode, case LSM_OBJ_ROLE: case LSM_OBJ_TYPE: security_inode_getsecid(inode, &osid); - rc = security_filter_rule_match(osid, + lsmblob_init(&blob, osid); + rc = security_filter_rule_match(&blob, rule->lsm[i].type, Audit_equal, rule->lsm[i].rule); @@ -431,7 +433,8 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode, case LSM_SUBJ_USER: case LSM_SUBJ_ROLE: case LSM_SUBJ_TYPE: - rc = security_filter_rule_match(secid, + lsmblob_init(&blob, secid); + rc = security_filter_rule_match(&blob, rule->lsm[i].type, Audit_equal, rule->lsm[i].rule); diff --git a/security/security.c b/security/security.c index a89634af639a..bfea9739c084 100644 --- a/security/security.c +++ b/security/security.c @@ -439,7 +439,7 @@ static int lsm_append(const char *new, char **result) /* * Current index to use while initializing the lsmblob secid list. */ -static int lsm_slot __initdata; +static int lsm_slot __lsm_ro_after_init; /** * security_add_hooks - Add a modules hooks to the hook lists. @@ -2412,9 +2412,21 @@ void security_audit_rule_free(void *lsmrule) call_void_hook(audit_rule_free, lsmrule); } -int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule) +int security_audit_rule_match(struct lsmblob *blob, u32 field, u32 op, + void *lsmrule) { - return call_int_hook(audit_rule_match, 0, secid, field, op, lsmrule); + struct security_hook_list *hp; + int rc; + + hlist_for_each_entry(hp, &security_hook_heads.audit_rule_match, list) { + if (WARN_ON(hp->lsmid->slot < 0 || hp->lsmid->slot >= lsm_slot)) + continue; + rc = hp->hook.audit_rule_match(blob->secid[hp->lsmid->slot], + field, op, lsmrule); + if (rc != 0) + return rc; + } + return 0; } #endif /* CONFIG_AUDIT */