On 4/20/2022 12:23 PM, Mimi Zohar wrote:
Hi Casey,
Below are a few initial comments/questions from a high level...
On Tue, 2022-04-19 at 09:50 -0700, Casey Schaufler wrote:
On 4/18/2022 7:59 AM, Casey Schaufler wrote:
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index eea6e92500b8..97470354c8ae 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -89,6 +89,7 @@ struct ima_rule_entry {
bool (*fgroup_op)(kgid_t cred_gid, kgid_t rule_gid); /* gid_eq(), gid_gt(), gid_lt() */
int pcr;
unsigned int allowed_algos; /* bitfield of allowed hash algorithms */
+ int which; /* which LSM rule applies to */
If "which" was defined in the lsm[] structure, it would be clear
reading the code that "which" refers to an LSM (e.g. entry-
lsm[i].which). Perhaps rename "which" to "which_lsm", "lsm_slot", or
"rules_lsm".
Both fine suggestions. I will incorporate them.
struct {
void *rule; /* LSM file metadata specific */
char *args_p; /* audit value */
@@ -285,6 +286,20 @@ static int __init default_appraise_policy_setup(char *str)
}
__setup("ima_appraise_tcb", default_appraise_policy_setup);
+static int ima_rules_lsm __ro_after_init;
+
+static int __init ima_rules_lsm_init(char *str)
+{
+ ima_rules_lsm = lsm_name_to_slot(str);
+ if (ima_rules_lsm < 0) {
+ ima_rules_lsm = 0;
+ pr_err("rule lsm \"%s\" not registered", str);
+ }
Specific IMA policy rules could be independent of the default one being
initialized here. Probably "ima_rules_lsm" should be renamed
"default_rules_lsm" or "default_ima_rules_lsm".
Sure. No problem to change.
The pr_err() message
should indicate setting the default rule LSM failed with an indication
of which LSM is set as the default.
Assuming 0 is guaranteed to be a valid LSM,
Unfortunately, it's possible for there to be no LSMs,
in which case 0 won't match any LSM when the hooks are
being invoked.
then something like:
"default rule lsm \"%s\" not registered, using \"%s"\", str,
lsm_slot_to_name(0));
+
+ return 1;
+}
+__setup("ima_rules_lsm=", ima_rules_lsm_init);
+
static struct ima_rule_opt_list *ima_alloc_rule_opt_list(const substring_t *src)
{
struct ima_rule_opt_list *opt_list;
@@ -356,7 +371,7 @@ static void ima_lsm_free_rule(struct ima_rule_entry *entry)
int i;
for (i = 0; i < MAX_LSM_RULES; i++) {
- ima_filter_rule_free(entry->lsm[i].rule);
+ ima_filter_rule_free(entry->lsm[i].rule, entry->which);
kfree(entry->lsm[i].args_p);
}
}
ima_rules_lsm is initialized to 0, If it isn't guranteed to be a valid
LSM, then ima_rules_lsm_init() needs to be called from ima_init.c:
ima_init(), so that it can be reset to an invalid value. Then
ima_filter_rule_init()/free() could check it.
If there is no LSM in slot 0 that implies there are no LSMs
suppling the hooks. Since the list of hooks to invoke will be
empty it doesn't matter what value is in default_rules_lsm.
thanks,
Mimi
