* [PATCH v2 1/4] ima: report policy load status
2014-10-03 11:40 [PATCH v2 0/4] ima: few policy loading improvements Dmitry Kasatkin
@ 2014-10-03 11:40 ` Dmitry Kasatkin
2014-10-03 11:40 ` [PATCH v2 2/4] ima: no need to allocate entry for comment Dmitry Kasatkin
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Dmitry Kasatkin @ 2014-10-03 11:40 UTC (permalink / raw)
To: zohar, linux-ima-devel, linux-security-module
Cc: linux-kernel, dmitry.kasatkin, Dmitry Kasatkin
Audit messages are rate limited and often policy update info
is not visible. Report policy loading status also using pr_info.
Changes in v2:
* reporting moved to ima_release_policy to notice parsing errors
* reporting both completed and failed status
Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
security/integrity/ima/ima_fs.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index da92fcc..16d8527 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -311,6 +311,8 @@ static int ima_open_policy(struct inode *inode, struct file *filp)
*/
static int ima_release_policy(struct inode *inode, struct file *file)
{
+ pr_info("IMA: policy update %s\n",
+ valid_policy ? "completed" : "failed");
if (!valid_policy) {
ima_delete_rules();
valid_policy = 1;
--
1.9.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 2/4] ima: no need to allocate entry for comment
2014-10-03 11:40 [PATCH v2 0/4] ima: few policy loading improvements Dmitry Kasatkin
2014-10-03 11:40 ` [PATCH v2 1/4] ima: report policy load status Dmitry Kasatkin
@ 2014-10-03 11:40 ` Dmitry Kasatkin
2014-10-03 11:40 ` [PATCH v2 3/4] ima: ignore empty and with whitespaces policy lines Dmitry Kasatkin
2014-10-03 11:40 ` [PATCH v2 4/4] ima: use atomic bit operations to protect policy update interface Dmitry Kasatkin
3 siblings, 0 replies; 6+ messages in thread
From: Dmitry Kasatkin @ 2014-10-03 11:40 UTC (permalink / raw)
To: zohar, linux-ima-devel, linux-security-module
Cc: linux-kernel, dmitry.kasatkin, Dmitry Kasatkin
If rule is a comment, there is no need to allocate entry.
Move checking for comment before allocating entry.
Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
security/integrity/ima/ima_policy.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index cdc620b..bf232b9 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -694,6 +694,12 @@ ssize_t ima_parse_add_rule(char *rule)
return -EACCES;
}
+ p = strsep(&rule, "\n");
+ len = strlen(p) + 1;
+
+ if (*p == '#')
+ return len;
+
entry = kzalloc(sizeof(*entry), GFP_KERNEL);
if (!entry) {
integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL,
@@ -703,14 +709,6 @@ ssize_t ima_parse_add_rule(char *rule)
INIT_LIST_HEAD(&entry->list);
- p = strsep(&rule, "\n");
- len = strlen(p) + 1;
-
- if (*p == '#') {
- kfree(entry);
- return len;
- }
-
result = ima_parse_rule(p, entry);
if (result) {
kfree(entry);
--
1.9.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 3/4] ima: ignore empty and with whitespaces policy lines
2014-10-03 11:40 [PATCH v2 0/4] ima: few policy loading improvements Dmitry Kasatkin
2014-10-03 11:40 ` [PATCH v2 1/4] ima: report policy load status Dmitry Kasatkin
2014-10-03 11:40 ` [PATCH v2 2/4] ima: no need to allocate entry for comment Dmitry Kasatkin
@ 2014-10-03 11:40 ` Dmitry Kasatkin
2014-10-13 0:14 ` Mimi Zohar
2014-10-03 11:40 ` [PATCH v2 4/4] ima: use atomic bit operations to protect policy update interface Dmitry Kasatkin
3 siblings, 1 reply; 6+ messages in thread
From: Dmitry Kasatkin @ 2014-10-03 11:40 UTC (permalink / raw)
To: zohar, linux-ima-devel, linux-security-module
Cc: linux-kernel, dmitry.kasatkin, Dmitry Kasatkin
Empty policy lines cause parsing failures which is, especially
for new users, hard to spot. This patch prevents it.
It is now possible to 'cat policy > <securityfs>/ima/policy'.
Changes in v2:
* strip leading blanks and tabs in rules to prevent parsing failures
Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
security/integrity/ima/ima_policy.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index bf232b9..d2c47d4 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -696,8 +696,9 @@ ssize_t ima_parse_add_rule(char *rule)
p = strsep(&rule, "\n");
len = strlen(p) + 1;
+ p += strspn(p, " \t");
- if (*p == '#')
+ if (*p == '#' || *p == '\0')
return len;
entry = kzalloc(sizeof(*entry), GFP_KERNEL);
--
1.9.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 4/4] ima: use atomic bit operations to protect policy update interface
2014-10-03 11:40 [PATCH v2 0/4] ima: few policy loading improvements Dmitry Kasatkin
` (2 preceding siblings ...)
2014-10-03 11:40 ` [PATCH v2 3/4] ima: ignore empty and with whitespaces policy lines Dmitry Kasatkin
@ 2014-10-03 11:40 ` Dmitry Kasatkin
3 siblings, 0 replies; 6+ messages in thread
From: Dmitry Kasatkin @ 2014-10-03 11:40 UTC (permalink / raw)
To: zohar, linux-ima-devel, linux-security-module
Cc: linux-kernel, dmitry.kasatkin, Dmitry Kasatkin
Current implementation uses atomic counter to provide exclusive access
to the sysfs 'policy' entry to update the IMA policy. While practically
it is almost unlikely, usage of counter might potentially allow another
process to overflow the counter, open the interface and insert additional
rules into the policy being loaded.
This patch replace atomic counter with atomic bit operations which is more
reliable and widely used to provide exclusive access.
As bit operation keeps interface locked after successful update, it makes
it unnecessary to verify if default policy was set or not during parsing
and interface closing. This patch also removes it.
Changes in v3:
* move audit log message to ima_relead_policy() to report successful and
unsuccessful result
* unnecessary comment removed
Changes in v2:
* keep interface locked after successful policy load as in original design
* remove sysfs entry as in original design
Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
security/integrity/ima/ima_fs.c | 23 ++++++++++++++++-------
security/integrity/ima/ima_policy.c | 23 ++---------------------
2 files changed, 18 insertions(+), 28 deletions(-)
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 16d8527..973b568 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -288,7 +288,12 @@ static struct dentry *runtime_measurements_count;
static struct dentry *violations;
static struct dentry *ima_policy;
-static atomic_t policy_opencount = ATOMIC_INIT(1);
+enum ima_fs_flags {
+ IMA_FS_BUSY,
+};
+
+static unsigned long ima_fs_flags;
+
/*
* ima_open_policy: sequentialize access to the policy file
*/
@@ -297,9 +302,9 @@ static int ima_open_policy(struct inode *inode, struct file *filp)
/* No point in being allowed to open it if you aren't going to write */
if (!(filp->f_flags & O_WRONLY))
return -EACCES;
- if (atomic_dec_and_test(&policy_opencount))
- return 0;
- return -EBUSY;
+ if (test_and_set_bit(IMA_FS_BUSY, &ima_fs_flags))
+ return -EBUSY;
+ return 0;
}
/*
@@ -311,12 +316,16 @@ static int ima_open_policy(struct inode *inode, struct file *filp)
*/
static int ima_release_policy(struct inode *inode, struct file *file)
{
- pr_info("IMA: policy update %s\n",
- valid_policy ? "completed" : "failed");
+ const char *cause = valid_policy ? "completed" : "failed";
+
+ pr_info("IMA: policy update %s\n", cause);
+ integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL,
+ "policy_update", cause, !valid_policy, 0);
+
if (!valid_policy) {
ima_delete_rules();
valid_policy = 1;
- atomic_set(&policy_opencount, 1);
+ clear_bit(IMA_FS_BUSY, &ima_fs_flags);
return 0;
}
ima_update_policy();
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index d2c47d4..0d14d25 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -356,19 +356,8 @@ void __init ima_init_policy(void)
*/
void ima_update_policy(void)
{
- static const char op[] = "policy_update";
- const char *cause = "already-exists";
- int result = 1;
- int audit_info = 0;
-
- if (ima_rules == &ima_default_rules) {
- ima_rules = &ima_policy_rules;
- ima_update_policy_flag();
- cause = "complete";
- result = 0;
- }
- integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL,
- NULL, op, cause, result, audit_info);
+ ima_rules = &ima_policy_rules;
+ ima_update_policy_flag();
}
enum {
@@ -686,14 +675,6 @@ ssize_t ima_parse_add_rule(char *rule)
ssize_t result, len;
int audit_info = 0;
- /* Prevent installed policy from changing */
- if (ima_rules != &ima_default_rules) {
- integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL,
- NULL, op, "already-exists",
- -EACCES, audit_info);
- return -EACCES;
- }
-
p = strsep(&rule, "\n");
len = strlen(p) + 1;
p += strspn(p, " \t");
--
1.9.1
^ permalink raw reply [flat|nested] 6+ messages in thread