[Cc'ing Janne Karhunen]
On Fri, 2020-02-14 at 15:41 -0800, Casey Schaufler wrote:
<snip>
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index 453427048999..624ed1a34842 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -75,7 +75,7 @@ struct ima_rule_entry {
> bool (*fowner_op)(kuid_t, kuid_t); /* uid_eq(), uid_gt(), uid_lt() */
> int pcr;
> struct {
> - void *rule; /* LSM file metadata specific */
> + void *rules[LSMBLOB_ENTRIES];
> void *args_p; /* audit value */
> int type; /* audit type */
> } lsm[MAX_LSM_RULES];
> @@ -84,6 +84,16 @@ struct ima_rule_entry {
> struct ima_template_desc *template;
> };
>
> +static inline bool ima_lsm_isset(void *rules[])
> +{
> + int i;
> +
> + for (i = 0; i < LSMBLOB_ENTRIES; i++)
> + if (rules[i])
> + return true;
> + return false;
> +}
> +
Even though ima_lsm_isset() is static, it should really be commented.
> /*
> * Without LSM specific knowledge, the default policy can only be
> * written in terms of .action, .func, .mask, .fsmagic, .uid, and .fowner
> @@ -258,9 +268,11 @@ __setup("ima_appraise_tcb", default_appraise_policy_setup);
> static void ima_lsm_free_rule(struct ima_rule_entry *entry)
> {
> int i;
> + int r;
>
> for (i = 0; i < MAX_LSM_RULES; i++) {
> - kfree(entry->lsm[i].rule);
> + for (r = 0; r < LSMBLOB_ENTRIES; r++)
> + kfree(entry->lsm[i].rules[r]);
> kfree(entry->lsm[i].args_p);
> }
> kfree(entry);
> @@ -295,8 +307,8 @@ static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry)
> security_filter_rule_init(nentry->lsm[i].type,
> Audit_equal,
> nentry->lsm[i].args_p,
> - &nentry->lsm[i].rule);
> - if (!nentry->lsm[i].rule)
> + nentry->lsm[i].rules);
> + if (!ima_lsm_isset(nentry->lsm[i].rules))
> pr_warn("rule for LSM \'%s\' is undefined\n",
> (char *)entry->lsm[i].args_p);
Janne, the generic LSM message looks fine, but should there also be an
LSM specific warning the first time it changes?
Mimi
