On 3/6/2020 2:01 PM, Paul Moore wrote: > On Fri, Feb 21, 2020 at 7:04 PM Casey Schaufler <[email protected]> > 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 <[email protected]> >> Reviewed-by: John Johansen <[email protected]> >> Acked-by: Stephen Smalley <[email protected]> >> Signed-off-by: Casey Schaufler <[email protected]> >> --- >> include/linux/security.h | 7 ++++--- >> kernel/auditfilter.c | 6 ++++-- >> kernel/auditsc.c | 14 ++++++++++---- >> security/integrity/ima/ima.h | 4 ++-- >> security/integrity/ima/ima_policy.c | 7 +++++-- >> security/security.c | 8 +++++--- >> 6 files changed, 30 insertions(+), 16 deletions(-) > ... > >> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c >> index 3a44abf4fced..509eb21eff7f 100644 >> --- a/kernel/auditfilter.c >> +++ b/kernel/auditfilter.c >> @@ -1327,6 +1327,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: >> @@ -1357,8 +1358,9 @@ int audit_filter(int msgtype, unsigned int listtype) >> case AUDIT_SUBJ_CLR: >> if (f->lsm_isset) { >> security_task_getsecid(current, >> &sid); >> - result = >> security_audit_rule_match(sid, >> - f->type, f->op, >> + lsmblob_init(&blob, sid); >> + result = security_audit_rule_match( >> + &blob, f->type, f->op, >> f->lsm_rules); > Unless I'm mistaken this patch is almost exclusively the following pattern: > > lsmblob_init(blob, sid); > security_audit_rule_match(blob, ...); > > ... which means we are assigning every array member in @blob the same > value of "sid" and then sending that into the LSM where each LSM is > going to then have to index into that array, to all get the same > value, and then do their match. I'm assuming this will make more > sense as I progress through the rest of the patchset, but right now it > seems like we could get by just fine with a u32 here.
When I do the next version I'll put considerably more effort into explaining the scaffolding strategy. I'm definitely in that area where the advantage of keeping patches small and the advantage of making a change obvious are at odds. This will apply to the next few patches in the series as well. -- Linux-audit mailing list [email protected] https://www.redhat.com/mailman/listinfo/linux-audit
