On Tue, 2022-02-01 at 15:37 -0500, Stefan Berger wrote:
Limit the number of policy rules one can set in non-init_ima_ns to aThis paragraph describes 'what' you're doing, not 'why'. Please prefix
hardcoded 1024 rules. If the user attempts to exceed this limit by
setting too many additional rules, emit an audit message with the cause
'too-many-rules' and simply ignore the newly added rules.
this paragraph with a short, probably one sentence, reason for the
change.
Switch the accounting for the memory allocated for IMA policy rules toDoes this change affect the existing IMA policy rules for init_ima_ns?
GFP_KERNEL_ACCOUNT so that cgroups kernel memory accounting takes effect.
Signed-off-by: Stefan Berger <stefanb@xxxxxxxxxxxxx>Perhaps "EDQUOT" would be more appropriate.
---
security/integrity/ima/ima_fs.c | 20 ++++++++++++++------
security/integrity/ima/ima_policy.c | 23 ++++++++++++++++++-----
2 files changed, 32 insertions(+), 11 deletions(-)
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 58d80884880f..cd102bbd4677 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -410,24 +410,32 @@ static int ima_release_policy(struct inode *inode, struct file *file)
{
struct ima_namespace *ns = &init_ima_ns;
const char *cause = ns->valid_policy ? "completed" : "failed";
+ int err = 0;
if ((file->f_flags & O_ACCMODE) == O_RDONLY)
return seq_release(inode, file);
- if (ns->valid_policy && ima_check_policy(ns) < 0) {
- cause = "failed";
- ns->valid_policy = 0;
+ if (ns->valid_policy) {
+ err = ima_check_policy(ns);
+ if (err < 0) {
+ if (err == -ENOSPC)
+ cause = "too-many-rules";The 'err' value is already properly set. No need for the minus sign.
+ else
+ cause = "failed";
+ ns->valid_policy = 0;
+ }
}
pr_info("policy update %s\n", cause);
- integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL,
- "policy_update", cause, !ns->valid_policy, 0);
+ integrity_audit_message(AUDIT_INTEGRITY_STATUS, NULL, NULL,
+ "policy_update", cause, !ns->valid_policy, 0,
+ -err);
if (!ns->valid_policy) {Using list_for_each_entry() is fine here, because multiple policy
ima_delete_rules(ns);
ns->valid_policy = 1;
clear_bit(IMA_FS_BUSY, &ns->ima_fs_flags);
- return 0;
+ return err;
}
ima_update_policy(ns);
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index acb4c36e539f..3f84d04f101d 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -305,7 +305,8 @@ static struct ima_rule_opt_list *ima_alloc_rule_opt_list(const substring_t *src)
return ERR_PTR(-EINVAL);
}
- opt_list = kzalloc(struct_size(opt_list, items, count), GFP_KERNEL);
+ opt_list = kzalloc(struct_size(opt_list, items, count),
+ GFP_KERNEL_ACCOUNT);
if (!opt_list) {
kfree(src_copy);
return ERR_PTR(-ENOMEM);
@@ -379,7 +380,7 @@ static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_namespace *ns,
* Immutable elements are copied over as pointers and data; only
* lsm rules can change
*/
- nentry = kmemdup(entry, sizeof(*nentry), GFP_KERNEL);
+ nentry = kmemdup(entry, sizeof(*nentry), GFP_KERNEL_ACCOUNT);
if (!nentry)
return NULL;
@@ -826,7 +827,7 @@ static void add_rules(struct ima_namespace *ns,
if (policy_rule & IMA_CUSTOM_POLICY) {
entry = kmemdup(&entries[i], sizeof(*entry),
- GFP_KERNEL);
+ GFP_KERNEL_ACCOUNT);
if (!entry)
continue;
@@ -863,7 +864,7 @@ static int __init ima_init_arch_policy(struct ima_namespace *ns)
ns->arch_policy_entry = kcalloc(arch_entries + 1,
sizeof(*ns->arch_policy_entry),
- GFP_KERNEL);
+ GFP_KERNEL_ACCOUNT);
if (!ns->arch_policy_entry)
return 0;
@@ -975,8 +976,20 @@ void __init ima_init_policy(struct ima_namespace *ns)
/* Make sure we have a valid policy, at least containing some rules. */
int ima_check_policy(struct ima_namespace *ns)
{
+ struct ima_rule_entry *entry;
+ size_t len1 = 0;
+ size_t len2 = 0;
+
if (list_empty(&ns->ima_temp_rules))
return -EINVAL;
+ if (ns != &init_ima_ns) {
+ list_for_each_entry(entry, &ns->ima_temp_rules, list)
+ len1++;
+ list_for_each_entry(entry, &ns->ima_policy_rules, list)
updates at the same time are blocked. Please add a comment.
+ len2++;One variable should be enough.
+ if (len1 + len2 > 1024)
+ return -ENOSPC;
+ }
return 0;
}
@@ -1848,7 +1861,7 @@ ssize_t ima_parse_add_rule(struct ima_namespace *ns, char *rule)
if (*p == '#' || *p == '\0')
return len;
- entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+ entry = kzalloc(sizeof(*entry), GFP_KERNEL_ACCOUNT);
if (!entry) {
integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL,
NULL, op, "-ENOMEM", -ENOMEM, audit_info);