| Message ID | 20200709061911.954326-2-tyhicks@linux.microsoft.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ima: Fix rule parsing bugs and extend KEXEC_CMDLINE rule support | expand |
On 7/9/20 2:19 AM, Tyler Hicks wrote:
> Ask the LSM to free its audit rule rather than directly calling kfree().
Is it to be called audit rule or filter rule ? Likewise in subject line.
Thanks & Regards,
- Nayna
On 2020-07-17 15:20:22, Nayna wrote: > > On 7/9/20 2:19 AM, Tyler Hicks wrote: > > Ask the LSM to free its audit rule rather than directly calling kfree(). > > Is it to be called audit rule or filter rule ? Likewise in subject line. The security hooks call this "audit rule" but Mimi explained the reasoning for IMA referring to this as an "audit filter" here: https://lore.kernel.org/lkml/1593466203.5085.62.camel@linux.ibm.com/ I would be fine with her renaming/rewording this patch, accordingly, in next-integrity-testing. Tyler > > Thanks & Regards, > > - Nayna
On Fri, 2020-07-17 at 14:24 -0500, Tyler Hicks wrote: > On 2020-07-17 15:20:22, Nayna wrote: > > > > On 7/9/20 2:19 AM, Tyler Hicks wrote: > > > Ask the LSM to free its audit rule rather than directly calling kfree(). > > > > Is it to be called audit rule or filter rule ? Likewise in subject line. > gt > The security hooks call this "audit rule" but Mimi explained the > reasoning for IMA referring to this as an "audit filter" here: > > https://lore.kernel.org/lkml/1593466203.5085.62.camel@linux.ibm.com/ > > I would be fine with her renaming/rewording this patch, accordingly, in > next-integrity-testing. Both here and "ima: AppArmor satisfies the audit rule requirements", the subject is AppArmor/LSM, which do refer to the rules as "audit" rules. In the "ima: Rename internal audit rule functions" case, the rule rename is internal to IMA. Here it makes sense to replace "audit" with "filter". Tyler, I've gone ahead and made the change. Mimi
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 4515975cc540..59ec28f5c117 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -420,6 +420,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 #else @@ -430,6 +431,10 @@ static inline int security_filter_rule_init(u32 field, u32 op, char *rulestr, return -EINVAL; } +static inline void security_filter_rule_free(void *lsmrule) +{ +} + 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 66aa3e17a888..d7c268c2b0ce 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);
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@linux.microsoft.com> Cc: Janne Karhunen <janne.karhunen@gmail.com> Cc: Casey Schaufler <casey@schaufler-ca.com> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com> --- * v3 - No change * v2 - Fixed build warning by dropping the 'return -EINVAL' from the stubbed out security_filter_rule_free() since it has a void return type - Added Mimi's Reviewed-by - Developed a follow-on patch to rename security_filter_rule_*() functions, to address Casey's request, but I'll submit it independently of this patch series since it is somewhat unrelated security/integrity/ima/ima.h | 5 +++++ security/integrity/ima/ima_policy.c | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-)