[PATCH v2] ipe: add errno field to IPE policy load auditing

[Jasjiv Singh <jasjivsingh@linux.microsoft.com>]

Thanks for reviewing it. Here's the example generated from real logs:

AUDIT_IPE_POLICY_LOAD(1422):
audit:  AUDIT1422 policy_name="Test_Policy" policy_version=0.0.1 
policy_digest =sha256:84EFBA8FA71E62AE0A537FAB962F8A2BD1053964C4
299DCA92BFFF4DB82E86D3 auid=1000 ses=3 lsm=ipe res=1 errno=0

The above record shows a new policy has been successfully loaded into
the kernel with the policy name, version, and hash with the errno=0.

AUDIT_IPE_POLICY_LOAD(1422) with error:

audit: AUDIT1422 policy_name=? policy_version=? policy_digest=?
auid=1000 ses=3 lsm=ipe res=0 errno=-74

The above record shows a policy load failure due to an invalid policy.

I have updated the failure cases in new_policy() and update_policy(),
which covers each case as well.
 
Signed-off-by: Jasjiv Singh <jasjivsingh@linux.microsoft.com>
---
 Documentation/admin-guide/LSM/ipe.rst |  2 ++
 security/ipe/audit.c                  | 10 ++++------
 security/ipe/fs.c                     | 17 ++++++++++++-----
 security/ipe/policy.c                 |  4 +---
 security/ipe/policy_fs.c              | 24 +++++++++++++++++++-----
 5 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/Documentation/admin-guide/LSM/ipe.rst b/Documentation/admin-guide/LSM/ipe.rst
index 2143165f48c9..5dbf54471fab 100644
--- a/Documentation/admin-guide/LSM/ipe.rst
+++ b/Documentation/admin-guide/LSM/ipe.rst
@@ -453,6 +453,8 @@ Field descriptions:
 | errno          | integer    | No        | The result of the policy error as follows:        |
 |                |            |           |                                                   |
 |                |            |           | +  0: no error                                    |
+|                |            |           | +  -EPERM: Insufficient permission                |
+|                |            |           | +  -EEXIST: Same name policy already deployed     |
 |                |            |           | +  -EBADMSG: policy is invalid                    |
 |                |            |           | +  -ENOMEM: out of memory (OOM)                   |
 |                |            |           | +  -ERANGE: policy version number overflow        |
diff --git a/security/ipe/audit.c b/security/ipe/audit.c
index f810f7004498..8df307bb2bab 100644
--- a/security/ipe/audit.c
+++ b/security/ipe/audit.c
@@ -21,7 +21,7 @@
 
 #define AUDIT_POLICY_LOAD_FMT "policy_name=\"%s\" policy_version=%hu.%hu.%hu "\
 			      "policy_digest=" IPE_AUDIT_HASH_ALG ":"
-#define AUDIT_POLICY_LOAD_NULL_FMT "policy_name=? policy_version=? "\
+#define AUDIT_POLICY_LOAD_FAIL_FMT "policy_name=? policy_version=? "\
 				   "policy_digest=?"
 #define AUDIT_OLD_ACTIVE_POLICY_FMT "old_active_pol_name=\"%s\" "\
 				    "old_active_pol_version=%hu.%hu.%hu "\
@@ -255,9 +255,8 @@ void ipe_audit_policy_activation(const struct ipe_policy *const op,
  */
 void ipe_audit_policy_load(const struct ipe_policy *const p)
 {
-	int res = 0;
-	int err = 0;
 	struct audit_buffer *ab;
+	int err = 0;
 
 	ab = audit_log_start(audit_context(), GFP_KERNEL,
 			     AUDIT_IPE_POLICY_LOAD);
@@ -266,15 +265,14 @@ void ipe_audit_policy_load(const struct ipe_policy *const p)
 
 	if (!IS_ERR(p)) {
 		audit_policy(ab, AUDIT_POLICY_LOAD_FMT, p);
-		res = 1;
 	} else {
-		audit_log_format(ab, AUDIT_POLICY_LOAD_NULL_FMT);
+		audit_log_format(ab, AUDIT_POLICY_LOAD_FAIL_FMT);
 		err = PTR_ERR(p);
 	}
 
 	audit_log_format(ab, " auid=%u ses=%u lsm=ipe res=%d errno=%d",
 			 from_kuid(&init_user_ns, audit_get_loginuid(current)),
-			 audit_get_sessionid(current), res, err);
+			 audit_get_sessionid(current), !err, err);
 
 	audit_log_end(ab);
 }
diff --git a/security/ipe/fs.c b/security/ipe/fs.c
index 5b6d19fb844a..40805b13ee2c 100644
--- a/security/ipe/fs.c
+++ b/security/ipe/fs.c
@@ -141,12 +141,16 @@ static ssize_t new_policy(struct file *f, const char __user *data,
 	char *copy = NULL;
 	int rc = 0;
 
-	if (!file_ns_capable(f, &init_user_ns, CAP_MAC_ADMIN))
-		return -EPERM;
+	if (!file_ns_capable(f, &init_user_ns, CAP_MAC_ADMIN)) {
+		rc = -EPERM;
+		goto out;
+	}
 
 	copy = memdup_user_nul(data, len);
-	if (IS_ERR(copy))
-		return PTR_ERR(copy);
+	if (IS_ERR(copy)) {
+		rc = PTR_ERR(copy);
+		goto out;
+	}
 
 	p = ipe_new_policy(NULL, 0, copy, len);
 	if (IS_ERR(p)) {
@@ -161,8 +165,11 @@ static ssize_t new_policy(struct file *f, const char __user *data,
 	ipe_audit_policy_load(p);
 
 out:
-	if (rc < 0)
+	if (rc < 0) {
 		ipe_free_policy(p);
+		p = ERR_PTR(rc);
+		ipe_audit_policy_load(p);
+	}
 	kfree(copy);
 	return (rc < 0) ? rc : len;
 }
diff --git a/security/ipe/policy.c b/security/ipe/policy.c
index 0f616e9fbe61..b628f696e32b 100644
--- a/security/ipe/policy.c
+++ b/security/ipe/policy.c
@@ -202,9 +202,7 @@ struct ipe_policy *ipe_new_policy(const char *text, size_t textlen,
 	return new;
 err:
 	ipe_free_policy(new);
-	new = ERR_PTR(rc);
-	ipe_audit_policy_load(new);
-	return new;
+	return ERR_PTR(rc);
 }
 
 /**
diff --git a/security/ipe/policy_fs.c b/security/ipe/policy_fs.c
index 3bcd8cbd09df..74f4e7288331 100644
--- a/security/ipe/policy_fs.c
+++ b/security/ipe/policy_fs.c
@@ -12,6 +12,7 @@
 #include "policy.h"
 #include "eval.h"
 #include "fs.h"
+#include "audit.h"
 
 #define MAX_VERSION_SIZE ARRAY_SIZE("65535.65535.65535")
 
@@ -288,25 +289,38 @@ static ssize_t getactive(struct file *f, char __user *data,
 static ssize_t update_policy(struct file *f, const char __user *data,
 			     size_t len, loff_t *offset)
 {
+	const struct ipe_policy *p = NULL;
 	struct inode *root = NULL;
 	char *copy = NULL;
 	int rc = 0;
 
-	if (!file_ns_capable(f, &init_user_ns, CAP_MAC_ADMIN))
-		return -EPERM;
+	if (!file_ns_capable(f, &init_user_ns, CAP_MAC_ADMIN)) {
+		rc = -EPERM;
+		goto out;
+	}
 
 	copy = memdup_user(data, len);
-	if (IS_ERR(copy))
-		return PTR_ERR(copy);
+	if (IS_ERR(copy)) {
+		rc = PTR_ERR(copy);
+		goto out;
+	}
 
 	root = d_inode(f->f_path.dentry->d_parent);
 	inode_lock(root);
 	rc = ipe_update_policy(root, NULL, 0, copy, len);
+	if (rc < 0) {
+		inode_unlock(root);
+		goto out;
+	}
 	inode_unlock(root);
 
+out:
 	kfree(copy);
-	if (rc)
+	if (rc) {
+		p = ERR_PTR(rc);
+		ipe_audit_policy_load(p);
 		return rc;
+	}
 
 	return len;
 }
-- 
2.34.1