On 10/23/2023 8:52 PM, Paul Moore wrote:
On Oct 4, 2023 Fan Wu <wufan@xxxxxxxxxxxxxxxxxxx> wrote:
IPE is designed to provide system level trust guarantees, this usually
implies that trust starts from bootup with a hardware root of trust,
which validates the bootloader. After this, the bootloader verifies the
kernel and the initramfs.
As there's no currently supported integrity method for initramfs, and
it's typically already verified by the bootloader, introduce a property
that causes the first superblock to have an execution to be "pinned",
which is typically initramfs.
When the "pinned" device is unmounted, it will be "unpinned" and
`boot_verified` property will always evaluate to false afterward.
We use a pointer with a spin_lock to "pin" the device instead of rcu
because rcu synchronization may sleep, which is not allowed when
unmounting a device.
Signed-off-by: Deven Bowers <deven.desai@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Fan Wu <wufan@xxxxxxxxxxxxxxxxxxx>
...
---
security/ipe/eval.c | 72 +++++++++++++++++++++++++++++++++++-
security/ipe/eval.h | 2 +
security/ipe/hooks.c | 12 ++++++
security/ipe/hooks.h | 2 +
security/ipe/ipe.c | 1 +
security/ipe/policy.h | 2 +
security/ipe/policy_parser.c | 35 +++++++++++++++++-
7 files changed, 124 insertions(+), 2 deletions(-)
diff --git a/security/ipe/eval.c b/security/ipe/eval.c
index 8a8bcc5c7d7f..bdac4abc0ddb 100644
--- a/security/ipe/eval.c
+++ b/security/ipe/eval.c
@@ -9,6 +9,7 @@
#include <linux/file.h>
#include <linux/sched.h>
#include <linux/rcupdate.h>
+#include <linux/spinlock.h>
#include "ipe.h"
#include "eval.h"
@@ -16,6 +17,44 @@
struct ipe_policy __rcu *ipe_active_policy;
+static const struct super_block *pinned_sb;
+static DEFINE_SPINLOCK(pin_lock);
+#define FILE_SUPERBLOCK(f) ((f)->f_path.mnt->mnt_sb)
+
+/**
+ * pin_sb - Pin the underlying superblock of @f, marking it as trusted.
+ * @sb: Supplies a super_block structure to be pinned.
+ */
+static void pin_sb(const struct super_block *sb)
+{
+ if (!sb)
+ return;
+ spin_lock(&pin_lock);
+ if (!pinned_sb)
+ pinned_sb = sb;
+ spin_unlock(&pin_lock);
+}
+
+/**
+ * from_pinned - Determine whether @sb is the pinned super_block.
+ * @sb: Supplies a super_block to check against the pinned super_block.
+ *
+ * Return:
+ * * true - @sb is the pinned super_block
+ * * false - @sb is not the pinned super_block
+ */
+static bool from_pinned(const struct super_block *sb)
+{
+ bool rv;
+
+ if (!sb)
+ return false;
+ spin_lock(&pin_lock);
+ rv = !IS_ERR_OR_NULL(pinned_sb) && pinned_sb == sb;
+ spin_unlock(&pin_lock);
It's okay for an initial version, but I still think you need to get
away from this spinlock in from_pinned() as quickly as possible.
Maybe I'm wrong, but this looks like a major source of lock contention.
I understand the issue around RCU and the potential for matching on
a reused buffer/address, but if you modified IPE to have its own LSM
security blob in super_block::security you could mark the superblock
when it was mounted and do a lockless lookup here in from_pinned().
Thank you for the suggestion. After some testing, I discovered that
switching to RCU to pin the super block and using a security blob to
mark a pinned super block works. This approach do avoid many spinlock
operations. I'll incorporate these changes in the next version of the patch.
-Fan
+ return rv;
+}
--
paul-moore.com
