On Fri, Jul 14, 2023 at 11:26 PM Fan Wu [off-list ref] wrote:

On Sat, Jul 08, 2023 at 12:23:04AM -0400, Paul Moore wrote:

quoted

On Jun 28, 2023 Fan Wu [off-list ref] wrote:

quoted

As is typical with LSMs, IPE uses securityfs as its interface with
userspace. for a complete list of the interfaces and the respective
inputs/outputs, please see the documentation under
admin-guide/LSM/ipe.rst

Signed-off-by: Deven Bowers <redacted>
Signed-off-by: Fan Wu <redacted>
---
 security/ipe/Makefile    |   2 +
 security/ipe/fs.c        | 101 ++++++++
 security/ipe/fs.h        |  16 ++
 security/ipe/ipe.c       |   3 +
 security/ipe/ipe.h       |   2 +
 security/ipe/policy.c    | 111 +++++++++
 security/ipe/policy.h    |   9 +
 security/ipe/policy_fs.c | 481 +++++++++++++++++++++++++++++++++++++++
 8 files changed, 725 insertions(+)
 create mode 100644 security/ipe/fs.c
 create mode 100644 security/ipe/fs.h
 create mode 100644 security/ipe/policy_fs.c

...

quoted

quoted

@@ -39,6 +67,65 @@ static int set_pkcs7_data(void *ctx, const void *data, size_t len,
    return 0;
 }

+/**
+ * ipe_update_policy - parse a new policy and replace @old with it.

What does "@old" refer to?  I'm guessing you want to drop the "@".

Yes it shouldn't be here, sorry confusion.

quoted

quoted

+ * @root: Supplies a pointer to the securityfs inode saved the policy.
+ * @text: Supplies a pointer to the plain text policy.
+ * @textlen: Supplies the length of @text.
+ * @pkcs7: Supplies a pointer to a buffer containing a pkcs7 message.
+ * @pkcs7len: Supplies the length of @pkcs7len.
+ *
+ * @text/@textlen is mutually exclusive with @pkcs7/@pkcs7len - see
+ * ipe_new_policy.
+ *
+ * Return:
+ * * !IS_ERR       - The old policy

"The old policy" is what?

Let me try to pharse it in another way, how about the existing policy
saved in the inode before update?

That sounds better, thanks.

quoted

quoted

diff --git a/security/ipe/policy_fs.c b/security/ipe/policy_fs.c
new file mode 100644
index 000000000000..52a120118cda
--- /dev/null
+++ b/security/ipe/policy_fs.c

@@ -0,0 +1,481 @@

...

quoted

+/**
+ * getactive - Read handler for "ipe/policies/$name/active".
+ * @f: Supplies a file structure representing the securityfs node.
+ * @data: Suppleis a buffer passed to the write syscall.
+ * @len: Supplies the length of @data.
+ * @offset: unused.
+ *
+ * @data will be populated with the 1 or 0 depending on if the
+ * corresponding policy is active.
+ *
+ * Return:
+ * * >0    - Success, Length of buffer written
+ * * <0    - Error
+ */
+static ssize_t getactive(struct file *f, char __user *data,
+                    size_t len, loff_t *offset)
+{
+   int rc = 0;
+   const char *str;
+   struct inode *root = NULL;
+   const struct ipe_policy *p = NULL;
+
+   root = d_inode(f->f_path.dentry->d_parent);
+
+   inode_lock_shared(root);
+   p = (struct ipe_policy *)root->i_private;
+   if (!p) {
+           inode_unlock_shared(root);
+           return -ENOENT;
+   }
+   inode_unlock_shared(root);
+
+   str = (p == rcu_access_pointer(ipe_active_policy)) ? "1" : "0";

The line above should be wrapped with a RCU lock.

This call only checks the value inside the pointer but doesn't dereference it.
Also from https://lwn.net/Articles/652156/ I found it says "The call to
rcu_access_pointer() need not be protected. In contrast, rcu_dereference() must
either be within an RCU read-side critical section", so I didn't add the lock
here, is this article outdated?

No, I believe you are correct.  There is always something new to learn
with RCU, thanks ;)

-- 
paul-moore.com