On 2020-06-22 17:55:59, Casey Schaufler wrote: > On 6/22/2020 5:32 PM, Tyler Hicks wrote: > > Ask the LSM to free its audit rule rather than directly calling kfree(). > > Both AppArmor and SELinux do additional work in their audit_rule_free() > > hooks. Fix memory leaks by allowing the LSMs to perform necessary work. > > > > Fixes: b16942455193 ("ima: use the lsm policy update notifier") > > Signed-off-by: Tyler Hicks <tyhicks@xxxxxxxxxxxxxxxxxxx> > > Cc: Janne Karhunen <janne.karhunen@xxxxxxxxx> > > --- > > security/integrity/ima/ima.h | 6 ++++++ > > security/integrity/ima/ima_policy.c | 2 +- > > 2 files changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h > > index df93ac258e01..de05d7f1d3ec 100644 > > --- a/security/integrity/ima/ima.h > > +++ b/security/integrity/ima/ima.h > > @@ -404,6 +404,7 @@ static inline void ima_free_modsig(struct modsig *modsig) > > #ifdef CONFIG_IMA_LSM_RULES > > > > #define security_filter_rule_init security_audit_rule_init > > +#define security_filter_rule_free security_audit_rule_free > > #define security_filter_rule_match security_audit_rule_match > > In context this seems perfectly reasonable. If, however, you're > working with the LSM infrastructure this set of #defines is maddening. > The existing ones have been driving my nuts for the past few years, > so I'd like to discourage adding another. Since the security_filter_rule > functions are IMA specific they shouldn't be prefixed security_. I know > that it seems to be code churn/bikesheading, but we please change these: > > static inline int ima_filter_rule_init(.....) > { > return security_audit_rule_init(.....); > } > > and so forth. I understand if you don't want to make the change. > I have plenty of other things driving me crazy just now, so this > doesn't seem likely to push me over the edge. I'd be happy to take a stab at that as a follow-up or a 13/12 patch. I'd like to leave this one as-is for stable kernel reasons since it is straightforward and simple. Tyler > > > > > #else > > @@ -414,6 +415,11 @@ static inline int security_filter_rule_init(u32 field, u32 op, char *rulestr, > > return -EINVAL; > > } > > > > +static inline void security_filter_rule_free(void *lsmrule) > > +{ > > + return -EINVAL; > > +} > > + > > static inline int security_filter_rule_match(u32 secid, u32 field, u32 op, > > void *lsmrule) > > { > > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c > > index e493063a3c34..236a731492d1 100644 > > --- a/security/integrity/ima/ima_policy.c > > +++ b/security/integrity/ima/ima_policy.c > > @@ -258,7 +258,7 @@ static void ima_lsm_free_rule(struct ima_rule_entry *entry) > > int i; > > > > for (i = 0; i < MAX_LSM_RULES; i++) { > > - kfree(entry->lsm[i].rule); > > + security_filter_rule_free(entry->lsm[i].rule); > > kfree(entry->lsm[i].args_p); > > } > > kfree(entry);