[RFC PATCH 0/9] Introducing the Loadpol LSM

[RFC PATCH 0/9] Introducing the Loadpol LSM

[Simon THOBY]

This RFC patch series aims at discussing the interest of a mechanism
that could restrict which modules are loaded at runtime.

Motivation
==========

In my experience, there is three main classes of kernel users.
On the one hand, some entities (companies or individuals) determine the exact set
of features they need, and recompile the kernel to match these needs (this is true
e.g. of cloud vendors, many security products, appliances, etc.). These users
can finely choose which kernel modules are available on their kernels, because
they know beforehand the exact use case, and they (mostly) control the software
and hardware stack.
On the other end of that spectrum, many entities use off-the-shelf distributions
with little to no further hardening (I believe this is the case of >90% of companies).
They don't really care about the presence of modules they may not need,
so all is well.
Finally, somewhere in the middle there are entitites that want to have some
control over what kernel modules are allowed on a system, but without the bandwith
of building and distributing internally their own kernel (because it would require
a dedicated team to keep ahead of the frequent updates, and because the added
value of that team is hard to justify). These entities will often use
off-the-shelf distributions, but they often miss the appropriate knobs to
be able to tweak the system in order to reach their desired security
level. In the particular case of kernel modules, Linux is very binary on
that point: either you allow the loading of any signed kernel module
(kernel.modules_disabled=0) or you completely disallow the loading of modules
(kernel.modules_disabled=1).
Note: there is also secure_insmod for SELinux, but it is a binary option too.

However these users do not want to trust all the kernel modules present
in the distribution packages (to reduce the attack surface, and/or for compliance
reasons).
They want to collate a list of kernel modules required to support their hardware
and software needs, and then to distribute a policy allowing these exact modules,
and blocking the rest.

One big example that comes to my mind is security software, because these finicky
beasts often come with a kernel module that can only be loaded once the
corresponding userland part has started, so very late in the boot process.
In addition, this type of module must be unloaded/reloaded when performing updates.
This is why loading all these modules at boot and then disabling the load of
further modules (via the kernel.modules_disabled sysctl) is not
sufficient.

Implementation
==============

This patch RFC builds a small LSM that can be configured at runtime (in
a practical setup, it would be loaded inside the - signed and verified -
initramfs), and locked to disallow further modifications of the policy.

This is loosely based on the policy mechanism of the IMA subsystem,
which was a great source of inspiration.
A new LSM hook is introduced to decide on a module load if the module
should be allowed or not). When called, the LSM iterates over its policy
and allow the module if a match associated with the ALLOW action is found.
A securityfs file is created to read the policy (using `seq_file` iterators) and update it.
The policy itself is very simple (two keywords to match modules, one
keyword to determine the action to take, and that's about it).
Finally, a sysctl entry exists to lock the policy in its current state.

Questions
=========

Do you believe that such a mechanism would be valuable?
And if not, do you have an idea of alternatives that users could combine to 
achieve the same results?

On an implementation detail, to determine if a module load request comes from
the kernel (instead of a user-initiated load), I currently do something
like `if (current->parent->flags & PF_WQ_WORKER))`. Is that a proper way
to determine that the current load is the consequence of a call to
`request_module`, and is that not overly broad (could this match
requests that are - in the end - user-initiated)?

Thank you for your consideration,
Have a nice day,
Simon

Simon THOBY (9):
  LSM: Introduce a new hook: security_kernel_module_load
  Introduce a new LSM: loadpol
  Loadpol LSM: filter kernel module request according to the policy
  Loadpol LSM: add a file in securityfs to read/modify the policy
  Loadpol LSM: add a sysctl to lock the policy
  Loadpol LSM: emit an audit log
  module: expose the list of blacklisted modules
  Loadpol LSM: include the blacklisted kernel modules in the policy
  Loadpol LSM: add a minimal documentation

 Documentation/admin-guide/LSM/Loadpol.rst |  81 ++++++
 Documentation/admin-guide/LSM/index.rst   |   1 +
 include/linux/lsm_count.h                 |   7 +
 include/linux/lsm_hook_defs.h             |   1 +
 include/linux/module.h                    |   4 +
 include/linux/security.h                  |   6 +
 include/uapi/linux/audit.h                |   1 +
 include/uapi/linux/lsm.h                  |   1 +
 kernel/module/main.c                      |   9 +
 security/Kconfig                          |   1 +
 security/Makefile                         |   1 +
 security/loadpol/Kconfig                  |  12 +
 security/loadpol/Makefile                 |   1 +
 security/loadpol/loadpol.c                |  76 ++++++
 security/loadpol/loadpol.h                |  43 +++
 security/loadpol/loadpol_fs.c             | 139 ++++++++++
 security/loadpol/loadpol_policy.c         | 311 ++++++++++++++++++++++
 security/security.c                       |  14 +
 18 files changed, 709 insertions(+)
 create mode 100644 Documentation/admin-guide/LSM/Loadpol.rst
 create mode 100644 security/loadpol/Kconfig
 create mode 100644 security/loadpol/Makefile
 create mode 100644 security/loadpol/loadpol.c
 create mode 100644 security/loadpol/loadpol.h
 create mode 100644 security/loadpol/loadpol_fs.c
 create mode 100644 security/loadpol/loadpol_policy.c

-- 
2.49.0

[RFC PATCH 1/9] LSM: Introduce a new hook: security_kernel_module_load

[Simon THOBY]

Introduce a new hook to allow LSMs to decide whether to block the load
of a kernel module.

Two hooks already exist:
- kernel_module_request is called when the kernel itself (not userspace)
 request the load of a module, e.g. because a device was detected.
 - security_kernel_load_data(LOADING_MODULE) is called when userspace calls
 init_module/finit_module, but lack information about the module because
 its  headers have not been loaded into kernel space, let alone parsed.
 This may not be sufficient for some LSMs.

This new hook is similar to security_kernel_load_data(LOADING_MODULE),
but called after the module signature and header are verified, and only
takes the module name for now.

Signed-off-by: Simon THOBY <git@nightmared.fr>
---
 include/linux/lsm_hook_defs.h |  1 +
 include/linux/module.h        |  1 +
 include/linux/security.h      |  6 ++++++
 kernel/module/main.c          |  4 ++++
 security/security.c           | 14 ++++++++++++++
 5 files changed, 26 insertions(+)

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index bf3bbac4e02a..51c5212d8bb6 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -223,6 +223,7 @@ LSM_HOOK(void, LSM_RET_VOID, cred_getlsmprop, const struct cred *c,
 LSM_HOOK(int, 0, kernel_act_as, struct cred *new, u32 secid)
 LSM_HOOK(int, 0, kernel_create_files_as, struct cred *new, struct inode *inode)
 LSM_HOOK(int, 0, kernel_module_request, char *kmod_name)
+LSM_HOOK(int, 0, kernel_module_load, const char *kmod_name)
 LSM_HOOK(int, 0, kernel_load_data, enum kernel_load_data_id id, bool contents)
 LSM_HOOK(int, 0, kernel_post_load_data, char *buf, loff_t size,
 	 enum kernel_load_data_id id, char *description)
diff --git a/include/linux/module.h b/include/linux/module.h
index 8050f77c3b64..b6b8d6f7f599 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -39,6 +39,7 @@ struct modversion_info {
 	char name[MODULE_NAME_LEN];
 };
 
+struct load_info;
 struct module;
 struct exception_table_entry;
 
diff --git a/include/linux/security.h b/include/linux/security.h
index cc9b54d95d22..e175b2cc8caf 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -498,6 +498,7 @@ void security_cred_getlsmprop(const struct cred *c, struct lsm_prop *prop);
 int security_kernel_act_as(struct cred *new, u32 secid);
 int security_kernel_create_files_as(struct cred *new, struct inode *inode);
 int security_kernel_module_request(char *kmod_name);
+int security_kernel_module_load(const char *kmod_name);
 int security_kernel_load_data(enum kernel_load_data_id id, bool contents);
 int security_kernel_post_load_data(char *buf, loff_t size,
 				   enum kernel_load_data_id id,
@@ -1255,6 +1256,11 @@ static inline int security_kernel_module_request(char *kmod_name)
 	return 0;
 }
 
+static inline int security_kernel_module_load(const char *kmod_name)
+{
+	return 0;
+}
+
 static inline int security_kernel_load_data(enum kernel_load_data_id id, bool contents)
 {
 	return 0;
diff --git a/kernel/module/main.c b/kernel/module/main.c
index a2859dc3eea6..12a1a5f4d823 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3228,6 +3228,10 @@ static int early_mod_check(struct load_info *info, int flags)
 		return -EPERM;
 	}
 
+	err = security_kernel_module_load(info->name);
+	if (err)
+		return err;
+
 	err = rewrite_section_headers(info, flags);
 	if (err)
 		return err;
diff --git a/security/security.c b/security/security.c
index fb57e8fddd91..b9430499c332 100644
--- a/security/security.c
+++ b/security/security.c
@@ -3336,6 +3336,20 @@ int security_kernel_module_request(char *kmod_name)
 	return call_int_hook(kernel_module_request, kmod_name);
 }
 
+/**
+ * security_kernel_module_load() - Check if loading a module is allowed
+ * @kmod_name: name of the kernel module being loaded
+ *
+ * This method is called when the userspace called init_module/finit_module
+ * with a valid module
+ *
+ * Return: Returns 0 if successful.
+ */
+int security_kernel_module_load(const char *kmod_name)
+{
+	return call_int_hook(kernel_module_load, kmod_name);
+}
+
 /**
  * security_kernel_read_file() - Read a file specified by userspace
  * @file: file
-- 
2.49.0

[RFC PATCH 2/9] Introduce a new LSM: loadpol

[Simon THOBY]

Create a new LSM to filter the load of kernel modules according
to a user-provided policy.

Signed-off-by: Simon THOBY <git@nightmared.fr>
---
 include/linux/lsm_count.h  |  7 +++++++
 include/uapi/linux/lsm.h   |  1 +
 security/Kconfig           |  1 +
 security/Makefile          |  1 +
 security/loadpol/Kconfig   | 12 ++++++++++++
 security/loadpol/Makefile  |  1 +
 security/loadpol/loadpol.c | 29 +++++++++++++++++++++++++++++
 security/loadpol/loadpol.h |  8 ++++++++
 8 files changed, 60 insertions(+)
 create mode 100644 security/loadpol/Kconfig
 create mode 100644 security/loadpol/Makefile
 create mode 100644 security/loadpol/loadpol.c
 create mode 100644 security/loadpol/loadpol.h

diff --git a/include/linux/lsm_count.h b/include/linux/lsm_count.h
index 16eb49761b25..9e0d96dfe9d2 100644
--- a/include/linux/lsm_count.h
+++ b/include/linux/lsm_count.h
@@ -84,6 +84,12 @@
 #define LANDLOCK_ENABLED
 #endif
 
+#if IS_ENABLED(CONFIG_SECURITY_LOADPOL)
+#define LOADPOL_ENABLED 1,
+#else
+#define LOADPOL_ENABLED
+#endif
+
 #if IS_ENABLED(CONFIG_IMA)
 #define IMA_ENABLED 1,
 #else
@@ -122,6 +128,7 @@
 		SAFESETID_ENABLED	\
 		BPF_LSM_ENABLED		\
 		LANDLOCK_ENABLED	\
+		LOADPOL_ENABLED		\
 		IMA_ENABLED		\
 		EVM_ENABLED		\
 		IPE_ENABLED)
diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h
index 938593dfd5da..ec8bdb415562 100644
--- a/include/uapi/linux/lsm.h
+++ b/include/uapi/linux/lsm.h
@@ -65,6 +65,7 @@ struct lsm_ctx {
 #define LSM_ID_IMA		111
 #define LSM_ID_EVM		112
 #define LSM_ID_IPE		113
+#define LSM_ID_LOADPOL		114
 
 /*
  * LSM_ATTR_XXX definitions identify different LSM attributes
diff --git a/security/Kconfig b/security/Kconfig
index 4816fc74f81e..e492c0d6768c 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -230,6 +230,7 @@ source "security/safesetid/Kconfig"
 source "security/lockdown/Kconfig"
 source "security/landlock/Kconfig"
 source "security/ipe/Kconfig"
+source "security/loadpol/Kconfig"
 
 source "security/integrity/Kconfig"
 
diff --git a/security/Makefile b/security/Makefile
index 22ff4c8bd8ce..562c572b7f23 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_CGROUPS)			+= device_cgroup.o
 obj-$(CONFIG_BPF_LSM)			+= bpf/
 obj-$(CONFIG_SECURITY_LANDLOCK)		+= landlock/
 obj-$(CONFIG_SECURITY_IPE)		+= ipe/
+obj-$(CONFIG_SECURITY_LOADPOL)		+= loadpol/
 
 # Object integrity file lists
 obj-$(CONFIG_INTEGRITY)			+= integrity/
diff --git a/security/loadpol/Kconfig b/security/loadpol/Kconfig
new file mode 100644
index 000000000000..8945e210ef69
--- /dev/null
+++ b/security/loadpol/Kconfig
@@ -0,0 +1,12 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+config SECURITY_LOADPOL
+	bool "LOADPOL support"
+	depends on SECURITY && MODULES
+	help
+	  Loadpol allows restricting the kernel modules that can be loaded
+	  dynamically according to a user-defined policy.
+
+	  If you are unsure how to answer this question, answer N. Otherwise,
+	  enable this and append "loadpol," to the CONFIG_LSM variable to
+	  enable Loadpol.
diff --git a/security/loadpol/Makefile b/security/loadpol/Makefile
new file mode 100644
index 000000000000..a794c8cfbfee
--- /dev/null
+++ b/security/loadpol/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_SECURITY_LOADPOL) := loadpol.o
diff --git a/security/loadpol/loadpol.c b/security/loadpol/loadpol.c
new file mode 100644
index 000000000000..3fc29263e2f8
--- /dev/null
+++ b/security/loadpol/loadpol.c
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include "linux/array_size.h"
+#include <linux/lsm_hooks.h>
+#include <uapi/linux/lsm.h>
+
+#include "loadpol.h"
+
+static int __init loadpol_init(void);
+
+static const struct lsm_id loadpol_lsmid = {
+	.name = LOADPOL_NAME,
+	.id = LSM_ID_LOADPOL,
+};
+
+static struct security_hook_list loadpol_hooks[] __ro_after_init = {
+};
+
+DEFINE_LSM(LOADPOL_NAME) = {
+	.name = LOADPOL_NAME,
+	.init = loadpol_init,
+};
+
+static int __init loadpol_init(void)
+{
+	security_add_hooks(loadpol_hooks, ARRAY_SIZE(loadpol_hooks), &loadpol_lsmid);
+	pr_info("Loadpol started.\n");
+	return 0;
+}
diff --git a/security/loadpol/loadpol.h b/security/loadpol/loadpol.h
new file mode 100644
index 000000000000..5e11474191f0
--- /dev/null
+++ b/security/loadpol/loadpol.h
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef _SECURITY_LOADPOL_LOADPOL_H
+#define _SECURITY_LOADPOL_LOADPOL_H
+
+#define LOADPOL_NAME "loadpol"
+
+#endif /* _SECURITY_LOADPOL_LOADPOL_H */
-- 
2.49.0

[RFC PATCH 3/9] Loadpol LSM: filter kernel module request according to the policy

[Simon THOBY]

When a kernel module is loaded, the LSM accepts or rejects the demand
according to its policy.

Signed-off-by: Simon THOBY <git@nightmared.fr>
---
 security/loadpol/Makefile         |  2 +-
 security/loadpol/loadpol.c        | 22 ++++++++++++
 security/loadpol/loadpol.h        | 27 ++++++++++++++
 security/loadpol/loadpol_policy.c | 59 +++++++++++++++++++++++++++++++
 4 files changed, 109 insertions(+), 1 deletion(-)
 create mode 100644 security/loadpol/loadpol_policy.c

diff --git a/security/loadpol/Makefile b/security/loadpol/Makefile
index a794c8cfbfee..062215e1f831 100644
--- a/security/loadpol/Makefile
+++ b/security/loadpol/Makefile
@@ -1 +1 @@
-obj-$(CONFIG_SECURITY_LOADPOL) := loadpol.o
+obj-$(CONFIG_SECURITY_LOADPOL) := loadpol.o loadpol_policy.o
diff --git a/security/loadpol/loadpol.c b/security/loadpol/loadpol.c
index 3fc29263e2f8..4d1a495a1462 100644
--- a/security/loadpol/loadpol.c
+++ b/security/loadpol/loadpol.c
@@ -6,6 +6,15 @@
 
 #include "loadpol.h"
 
+// default policy: allow all modules
+static struct loadpol_policy_entry default_policy_entries[] __ro_after_init = {
+	{
+		.origin = (ORIGIN_KERNEL | ORIGIN_USERSPACE),
+		.action = ACTION_ALLOW,
+		.module_name = NULL,
+	},
+};
+
 static int __init loadpol_init(void);
 
 static const struct lsm_id loadpol_lsmid = {
@@ -14,6 +23,7 @@ static const struct lsm_id loadpol_lsmid = {
 };
 
 static struct security_hook_list loadpol_hooks[] __ro_after_init = {
+	LSM_HOOK_INIT(kernel_module_load, loadpol_kernel_module_load),
 };
 
 DEFINE_LSM(LOADPOL_NAME) = {
@@ -23,6 +33,18 @@ DEFINE_LSM(LOADPOL_NAME) = {
 
 static int __init loadpol_init(void)
 {
+	for (int i = 0; i < ARRAY_SIZE(default_policy_entries); i++) {
+		struct loadpol_policy_entry *entry = kmemdup(
+			&default_policy_entries[i],
+			sizeof(struct loadpol_policy_entry),
+			GFP_KERNEL
+		);
+		if (!entry)
+			return -ENOMEM;
+
+		list_add_tail(&entry->list, loadpol_policy);
+	}
+
 	security_add_hooks(loadpol_hooks, ARRAY_SIZE(loadpol_hooks), &loadpol_lsmid);
 	pr_info("Loadpol started.\n");
 	return 0;
diff --git a/security/loadpol/loadpol.h b/security/loadpol/loadpol.h
index 5e11474191f0..a81d52f6d4da 100644
--- a/security/loadpol/loadpol.h
+++ b/security/loadpol/loadpol.h
@@ -3,6 +3,33 @@
 #ifndef _SECURITY_LOADPOL_LOADPOL_H
 #define _SECURITY_LOADPOL_LOADPOL_H
 
+#include "linux/list.h"
+
 #define LOADPOL_NAME "loadpol"
 
+enum policy_entry_origin {
+	ORIGIN_KERNEL = 1 << 0,
+	ORIGIN_USERSPACE = 1 << 1,
+};
+
+enum __packed policy_entry_action {
+	ACTION_UNDEFINED,
+	ACTION_ALLOW,
+	ACTION_DENY
+};
+
+struct loadpol_policy_entry {
+	struct list_head list;
+	// bitfield of policy_entry_origin
+	u8 origin;
+	enum policy_entry_action action;
+	// when NULL, the policy apply to every module
+	char *module_name;
+};
+
+extern struct list_head __rcu *loadpol_policy;
+
+// evaluate if a kernel module called 'kmod' is allowed to be loaded in the kernel
+int loadpol_kernel_module_load(const char *kmod);
+
 #endif /* _SECURITY_LOADPOL_LOADPOL_H */
diff --git a/security/loadpol/loadpol_policy.c b/security/loadpol/loadpol_policy.c
new file mode 100644
index 000000000000..6ba5ab600e3e
--- /dev/null
+++ b/security/loadpol/loadpol_policy.c
@@ -0,0 +1,59 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include "linux/rculist.h"
+#include <linux/sched.h>
+#include <linux/sysctl.h>
+#include <linux/parser.h>
+
+#include "loadpol.h"
+
+/*  use A/B policy entries: switch from one to the next every time the policy get overwritten */
+static LIST_HEAD(loadpol_policy_a);
+static LIST_HEAD(loadpol_policy_b);
+struct list_head __rcu *loadpol_policy = (struct list_head __rcu *)(&loadpol_policy_a);
+
+int loadpol_kernel_module_load(const char *kmod)
+{
+	struct task_struct *parent_task;
+	struct loadpol_policy_entry *entry;
+	struct list_head *policy_list_tmp;
+	enum policy_entry_origin orig = ORIGIN_USERSPACE;
+	bool allowed = false;
+
+	rcu_read_lock();
+	parent_task = rcu_dereference(current->parent);
+	/* the parent of the current task is a workqueue -> the request comes from the kernel */
+	if (parent_task && (parent_task->flags & PF_WQ_WORKER))
+		orig = ORIGIN_KERNEL;
+	rcu_read_unlock();
+
+	pr_debug("Loadpol: trying to load '%s' (asked by %s)",
+		 kmod,
+		 orig == ORIGIN_KERNEL ? "kernel" : "userspace");
+
+	rcu_read_lock();
+	policy_list_tmp = rcu_dereference(loadpol_policy);
+	list_for_each_entry_rcu(entry, policy_list_tmp, list) {
+		/* the requestor does not match */
+		if ((orig & entry->origin) == 0)
+			continue;
+
+		allowed = entry->action == ACTION_ALLOW;
+
+		if (!entry->module_name)
+			goto unlock_and_exit;
+
+		if (entry->module_name && match_wildcard(entry->module_name, kmod))
+			goto unlock_and_exit;
+	}
+
+	/* No match -> reject the demand */
+	allowed = false;
+
+unlock_and_exit:
+	rcu_read_unlock();
+
+	pr_debug("Loadpol: load of module '%s' %s", kmod, allowed ? "allowed" : "blocked");
+
+	return allowed ? 0 : -EPERM;
+}
-- 
2.49.0

[RFC PATCH 4/9] Loadpol LSM: add a file in securityfs to read/modify the policy

[Simon THOBY]

Privileged users (CAP_SYS_ADMIN in the root namespace) can read and
update the module policy.

Signed-off-by: Simon THOBY <git@nightmared.fr>
---
 security/loadpol/Makefile         |   2 +-
 security/loadpol/loadpol.h        |   8 +
 security/loadpol/loadpol_fs.c     | 108 +++++++++++++
 security/loadpol/loadpol_policy.c | 244 +++++++++++++++++++++++++++++-
 4 files changed, 360 insertions(+), 2 deletions(-)
 create mode 100644 security/loadpol/loadpol_fs.c

diff --git a/security/loadpol/Makefile b/security/loadpol/Makefile
index 062215e1f831..3351a4e90c1d 100644
--- a/security/loadpol/Makefile
+++ b/security/loadpol/Makefile
@@ -1 +1 @@
-obj-$(CONFIG_SECURITY_LOADPOL) := loadpol.o loadpol_policy.o
+obj-$(CONFIG_SECURITY_LOADPOL) := loadpol.o loadpol_policy.o loadpol_fs.o
diff --git a/security/loadpol/loadpol.h b/security/loadpol/loadpol.h
index a81d52f6d4da..e81aa322e178 100644
--- a/security/loadpol/loadpol.h
+++ b/security/loadpol/loadpol.h
@@ -4,6 +4,7 @@
 #define _SECURITY_LOADPOL_LOADPOL_H
 
 #include "linux/list.h"
+#include <linux/seq_file.h>
 
 #define LOADPOL_NAME "loadpol"
 
@@ -29,6 +30,13 @@ struct loadpol_policy_entry {
 
 extern struct list_head __rcu *loadpol_policy;
 
+void *loadpol_policy_start(struct seq_file *m, loff_t *pos);
+void *loadpol_policy_next(struct seq_file *m, void *v, loff_t *pos);
+void loadpol_policy_stop(struct seq_file *m, void *v);
+int loadpol_policy_show(struct seq_file *m, void *v);
+
+ssize_t loadpol_parse_ruleset(char *data);
+
 // evaluate if a kernel module called 'kmod' is allowed to be loaded in the kernel
 int loadpol_kernel_module_load(const char *kmod);
 
diff --git a/security/loadpol/loadpol_fs.c b/security/loadpol/loadpol_fs.c
new file mode 100644
index 000000000000..9134d11718a0
--- /dev/null
+++ b/security/loadpol/loadpol_fs.c
@@ -0,0 +1,108 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include "linux/array_size.h"
+#include <linux/security.h>
+
+#include "loadpol.h"
+
+static struct dentry *securityfs_dir;
+static struct dentry *securityfs_policy;
+
+static DEFINE_MUTEX(policy_write_mutex);
+
+static const struct seq_operations loadpol_policy_seqops = {
+	.start = loadpol_policy_start,
+	.next = loadpol_policy_next,
+	.stop = loadpol_policy_stop,
+	.show = loadpol_policy_show,
+};
+
+static int loadpol_open_policy(struct inode *inode, struct file *filp)
+{
+	// Check permissions when accessing with writable flags
+	if ((filp->f_flags & O_ACCMODE) != O_RDONLY) {
+		if (!capable(CAP_SYS_ADMIN))
+			return -EPERM;
+	}
+	return seq_open(filp, &loadpol_policy_seqops);
+}
+
+static ssize_t loadpol_write_policy(struct file *file, const char __user *buf,
+				    size_t count, loff_t *ppos)
+{
+	char *data;
+	ssize_t ret;
+
+	/*
+	 * arbitrary size limit (to prevent a DoS but still allow loading a policy with a few
+	 * thousands of entries)
+	 */
+	if (count >= 64 * PAGE_SIZE) {
+		ret = -ENOSPC;
+		goto out;
+	}
+
+	/* Partial writes are not permitted */
+	if (*ppos != 0) {
+		ret = -ESPIPE;
+		goto out;
+	}
+
+	data = memdup_user_nul(buf, count);
+	if (IS_ERR(data)) {
+		ret = PTR_ERR(data);
+		goto out;
+	}
+
+	ret = mutex_lock_interruptible(&policy_write_mutex);
+	if (ret < 0) {
+		ret = -EBUSY;
+		goto out_free;
+	}
+
+	ret = loadpol_parse_ruleset(data);
+	/* the policy was injested, return the write as having been completed */
+	if (!ret)
+		ret = count;
+
+	mutex_unlock(&policy_write_mutex);
+out_free:
+	kfree(data);
+out:
+	return ret;
+}
+
+static const struct file_operations loadpol_policy_ops = {
+	.open = loadpol_open_policy,
+	.write = loadpol_write_policy,
+	.read = seq_read,
+	.llseek = seq_lseek,
+};
+
+static int __init loadpol_init_fs(void)
+{
+	int ret;
+
+	securityfs_dir = securityfs_create_dir(LOADPOL_NAME, NULL);
+	if (IS_ERR(securityfs_dir)) {
+		ret = PTR_ERR(securityfs_dir);
+		goto err;
+	}
+
+	securityfs_policy = securityfs_create_file(
+		"policy", 0600, securityfs_dir, NULL, &loadpol_policy_ops
+	);
+	if (IS_ERR(securityfs_policy)) {
+		ret = PTR_ERR(securityfs_policy);
+		goto err;
+	}
+
+	return 0;
+err:
+	securityfs_remove(securityfs_policy);
+	securityfs_remove(securityfs_dir);
+	return ret;
+}
+
+/* only create debugfs entries once the filesystem is available */
+fs_initcall(loadpol_init_fs);
diff --git a/security/loadpol/loadpol_policy.c b/security/loadpol/loadpol_policy.c
index 6ba5ab600e3e..366046f00959 100644
--- a/security/loadpol/loadpol_policy.c
+++ b/security/loadpol/loadpol_policy.c
@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 
+#define pr_fmt(fmt) "loadpol: " fmt
+
 #include "linux/rculist.h"
 #include <linux/sched.h>
 #include <linux/sysctl.h>
@@ -12,6 +14,244 @@ static LIST_HEAD(loadpol_policy_a);
 static LIST_HEAD(loadpol_policy_b);
 struct list_head __rcu *loadpol_policy = (struct list_head __rcu *)(&loadpol_policy_a);
 
+enum loadpol_options {
+	Opt_action,
+	Opt_allowed,
+	Opt_denied,
+	Opt_kernel,
+	Opt_module,
+	Opt_origin,
+	Opt_userspace,
+	Opt_err,
+};
+
+static const match_table_t policy_options = {
+	{Opt_action, "action=%s"},
+	{Opt_allowed, "allow"},
+	{Opt_denied, "deny"},
+	{Opt_kernel, "kernel"},
+	{Opt_module, "module==%s"},
+	{Opt_origin, "origin==%s"},
+	{Opt_userspace, "user"},
+	{Opt_err, NULL},
+};
+
+#define opt(o) policy_options[o].pattern
+
+static void loadpol_free_entry(struct loadpol_policy_entry *entry)
+{
+	kfree(entry->module_name);
+	kfree(entry);
+}
+
+static void loadpol_free_ruleset(struct list_head *policy)
+{
+	struct loadpol_policy_entry *entry, *next_entry;
+
+	list_for_each_entry_safe(entry, next_entry, policy, list) {
+		list_del(&entry->list);
+		loadpol_free_entry(entry);
+	}
+}
+
+void *loadpol_policy_start(struct seq_file *m, loff_t *pos)
+{
+	struct list_head *entry_list;
+
+	rcu_read_lock();
+	entry_list = seq_list_start_rcu(rcu_dereference(loadpol_policy), *pos);
+	rcu_read_unlock();
+
+	if (!entry_list)
+		return NULL;
+
+	return container_of(entry_list, struct loadpol_policy_entry, list);
+}
+
+void *loadpol_policy_next(struct seq_file *m, void *v, loff_t *pos)
+{
+	struct list_head *entry_list;
+
+	rcu_read_lock();
+	entry_list = seq_list_next_rcu(v, rcu_dereference(loadpol_policy), pos);
+	rcu_read_unlock();
+
+	if (!entry_list)
+		return NULL;
+
+	return container_of(entry_list, struct loadpol_policy_entry, list);
+}
+
+void loadpol_policy_stop(struct seq_file *m, void *v)
+{
+}
+
+int loadpol_policy_show(struct seq_file *m, void *v)
+{
+	struct loadpol_policy_entry *entry = v;
+
+	seq_printf(m, opt(Opt_origin), "");
+	if (entry->origin & ORIGIN_KERNEL)
+		seq_puts(m, opt(Opt_kernel));
+	if (entry->origin & ORIGIN_KERNEL && entry->origin & ORIGIN_USERSPACE)
+		seq_puts(m, ",");
+	if (entry->origin & ORIGIN_USERSPACE)
+		seq_puts(m, opt(Opt_userspace));
+
+	seq_puts(m, " ");
+
+	if (entry->module_name) {
+		seq_printf(m, opt(Opt_module), entry->module_name);
+		seq_puts(m, " ");
+	}
+
+	seq_printf(m, opt(Opt_action),
+		   (entry->action == ACTION_ALLOW) ? opt(Opt_allowed) : opt(Opt_denied));
+
+	seq_puts(m, "\n");
+	return 0;
+}
+
+static struct loadpol_policy_entry *process_policy_rule(char *line)
+{
+	char *token, *subtoken;
+	struct loadpol_policy_entry *entry;
+	int ret = -EINVAL;
+
+	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+	if (!entry)
+		return ERR_PTR(-ENOMEM);
+
+	// not strictly necessary since we zero-initialize entry, but explicitness is good
+	entry->module_name = NULL;
+	entry->origin = ORIGIN_KERNEL | ORIGIN_USERSPACE;
+	entry->action = ACTION_UNDEFINED;
+
+	while ((token = strsep(&line, " \t"))) {
+		int token_id;
+		substring_t args[MAX_OPT_ARGS];
+
+		if (!strlen(token))
+			continue;
+
+		token_id = match_token(token, policy_options, args);
+		switch (token_id) {
+		case Opt_module:
+			if (entry->module_name) {
+				pr_warn("cannot define two names in the same entry: '%s'", line);
+				goto err;
+			}
+
+			if (!strlen(args[0].from)) {
+				pr_warn("empty module names are not supported: '%s'", line);
+				goto err;
+			}
+
+			entry->module_name = kstrdup(args[0].from, GFP_KERNEL);
+			if (!entry->module_name) {
+				ret = -ENOMEM;
+				goto err;
+			}
+
+			break;
+		case Opt_origin:
+			entry->origin = 0;
+
+			while ((subtoken = strsep(&args[0].from, ","))) {
+				if (!strcmp(subtoken, opt(Opt_kernel))) {
+					entry->origin |= ORIGIN_KERNEL;
+					continue;
+				}
+
+				if (!strcmp(subtoken, opt(Opt_userspace))) {
+					entry->origin |= ORIGIN_USERSPACE;
+					continue;
+				}
+
+				pr_warn("Unsupported origin '%s'", subtoken);
+				goto err;
+			}
+			break;
+		case Opt_action:
+			if (entry->action != ACTION_UNDEFINED) {
+				pr_warn("cannot define two action in the same entry: '%s'", line);
+				goto err;
+			}
+
+			if (!strcmp(args[0].from, opt(Opt_denied))) {
+				entry->action = ACTION_DENY;
+				continue;
+			}
+
+			if (!strcmp(args[0].from, opt(Opt_allowed))) {
+				entry->action = ACTION_ALLOW;
+				continue;
+			}
+
+			pr_warn("Loadpol: Unsupported action '%s'", args[0].from);
+			goto err;
+		case Opt_err:
+			pr_warn("Unsupported token %d: %s\n", token_id, token);
+			return ERR_PTR(-EINVAL);
+		}
+	}
+
+	return entry;
+err:
+	loadpol_free_entry(entry);
+
+	return ERR_PTR(ret);
+}
+
+ssize_t loadpol_parse_ruleset(char *data)
+{
+	struct list_head *new_policy_list;
+	struct loadpol_policy_entry *entry;
+	char *sep_ptr, *line;
+
+	rcu_read_lock();
+	new_policy_list = (rcu_dereference(loadpol_policy) == &loadpol_policy_a) ?
+			   &loadpol_policy_b : &loadpol_policy_a;
+	rcu_read_unlock();
+
+	/* wait for the RCU previous critical section to be over */
+	synchronize_rcu();
+
+	/*
+	 * At this point, we know that nobody else is iterating over new_policy_list: we are
+	 * inside a lock so we have no concurrent writer, and we called synchronize_rcu which ensure
+	 * that current readers are reading the other policy list
+	 * (policy_a if we operate on policy_b, or vice-versa).
+	 */
+
+	/* free the old policy entries */
+	loadpol_free_ruleset(new_policy_list);
+
+	sep_ptr = data;
+	while ((line = strsep(&sep_ptr, "\n"))) {
+		// ignore empty lines
+		if (!strlen(line))
+			continue;
+
+		entry = process_policy_rule(line);
+		if (IS_ERR(entry))
+			goto err;
+
+		list_add_tail(&entry->list, new_policy_list);
+	}
+
+	/* switch to policy */
+	rcu_assign_pointer(loadpol_policy, new_policy_list);
+
+	return 0;
+
+err:
+	/* free the newly created entries */
+	loadpol_free_ruleset(new_policy_list);
+
+	return -EINVAL;
+}
+
 int loadpol_kernel_module_load(const char *kmod)
 {
 	struct task_struct *parent_task;
@@ -53,7 +293,9 @@ int loadpol_kernel_module_load(const char *kmod)
 unlock_and_exit:
 	rcu_read_unlock();
 
-	pr_debug("Loadpol: load of module '%s' %s", kmod, allowed ? "allowed" : "blocked");
+	pr_debug("Loadpol: load of module '%s' %s",
+		 kmod,
+		 allowed ? "allowed" : "blocked");
 
 	return allowed ? 0 : -EPERM;
 }
-- 
2.49.0

[RFC PATCH 5/9] Loadpol LSM: add a sysctl to lock the policy

[Simon THOBY]

Once the policy is properly configurd, users may want to lock that
policy to ensure no future change can be applied to it.

Add a sysctl that can be toggled to lock the policy.

Signed-off-by: Simon THOBY <git@nightmared.fr>
---
 security/loadpol/loadpol_fs.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/security/loadpol/loadpol_fs.c b/security/loadpol/loadpol_fs.c
index 9134d11718a0..1fec94de9f40 100644
--- a/security/loadpol/loadpol_fs.c
+++ b/security/loadpol/loadpol_fs.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 
 #include "linux/array_size.h"
+#include <linux/sysctl.h>
 #include <linux/security.h>
 
 #include "loadpol.h"
@@ -8,8 +9,22 @@
 static struct dentry *securityfs_dir;
 static struct dentry *securityfs_policy;
 
+static bool policy_locked;
 static DEFINE_MUTEX(policy_write_mutex);
 
+static const struct ctl_table sysctls[] = {
+	{
+		.procname	= "locked",
+		.data		= &policy_locked,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		/* only allow a transition from 0 (not locked) to 1 (locked) */
+		.proc_handler	= proc_dointvec_minmax,
+		.extra1		= SYSCTL_ONE,
+		.extra2		= SYSCTL_ONE,
+	},
+};
+
 static const struct seq_operations loadpol_policy_seqops = {
 	.start = loadpol_policy_start,
 	.next = loadpol_policy_next,
@@ -33,6 +48,13 @@ static ssize_t loadpol_write_policy(struct file *file, const char __user *buf,
 	char *data;
 	ssize_t ret;
 
+	/* Once the policy is locked, modifications are blocked */
+	if (policy_locked) {
+		pr_warn("Loadpol is locked, the policy cannot be modified");
+		ret = -EPERM;
+		goto out;
+	}
+
 	/*
 	 * arbitrary size limit (to prevent a DoS but still allow loading a policy with a few
 	 * thousands of entries)
@@ -81,8 +103,15 @@ static const struct file_operations loadpol_policy_ops = {
 
 static int __init loadpol_init_fs(void)
 {
+	struct ctl_table_header *sysctl_hdr = NULL;
 	int ret;
 
+	sysctl_hdr = register_sysctl_sz("security/" LOADPOL_NAME, sysctls, ARRAY_SIZE(sysctls));
+	if (IS_ERR(sysctl_hdr)) {
+		ret = PTR_ERR(sysctl_hdr);
+		goto err;
+	}
+
 	securityfs_dir = securityfs_create_dir(LOADPOL_NAME, NULL);
 	if (IS_ERR(securityfs_dir)) {
 		ret = PTR_ERR(securityfs_dir);
@@ -99,6 +128,8 @@ static int __init loadpol_init_fs(void)
 
 	return 0;
 err:
+	if (!IS_ERR(sysctl_hdr))
+		unregister_sysctl_table(sysctl_hdr);
 	securityfs_remove(securityfs_policy);
 	securityfs_remove(securityfs_dir);
 	return ret;
-- 
2.49.0

[RFC PATCH 6/9] Loadpol LSM: emit an audit log

[Simon THOBY]

When a decision is reached, emit an audit log so that the userland may
know why the module load (eventually) failed.

Signed-off-by: Simon THOBY <git@nightmared.fr>
---
 include/uapi/linux/audit.h        |  1 +
 security/loadpol/loadpol_policy.c | 10 ++++++++++
 2 files changed, 11 insertions(+)

diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 9a4ecc9f6dc5..592070fd3c1a 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -148,6 +148,7 @@
 #define AUDIT_IPE_POLICY_LOAD	1422	/* IPE policy load */
 #define AUDIT_LANDLOCK_ACCESS	1423	/* Landlock denial */
 #define AUDIT_LANDLOCK_DOMAIN	1424	/* Landlock domain status */
+#define AUDIT_LOADPOL_ACTION	1430	/* Module load request filtered */
 
 #define AUDIT_FIRST_KERN_ANOM_MSG   1700
 #define AUDIT_LAST_KERN_ANOM_MSG    1799
diff --git a/security/loadpol/loadpol_policy.c b/security/loadpol/loadpol_policy.c
index 366046f00959..de2b116bc09d 100644
--- a/security/loadpol/loadpol_policy.c
+++ b/security/loadpol/loadpol_policy.c
@@ -6,6 +6,7 @@
 #include <linux/sched.h>
 #include <linux/sysctl.h>
 #include <linux/parser.h>
+#include <linux/audit.h>
 
 #include "loadpol.h"
 
@@ -257,6 +258,7 @@ int loadpol_kernel_module_load(const char *kmod)
 	struct task_struct *parent_task;
 	struct loadpol_policy_entry *entry;
 	struct list_head *policy_list_tmp;
+	struct audit_buffer *ab;
 	enum policy_entry_origin orig = ORIGIN_USERSPACE;
 	bool allowed = false;
 
@@ -293,6 +295,14 @@ int loadpol_kernel_module_load(const char *kmod)
 unlock_and_exit:
 	rcu_read_unlock();
 
+	ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_LOADPOL_ACTION);
+	if (!ab)
+		goto out;
+	audit_log_format(ab, "allowed=%d requested_module=%s", allowed, kmod);
+	audit_log_task_info(ab);
+	audit_log_end(ab);
+
+out:
 	pr_debug("Loadpol: load of module '%s' %s",
 		 kmod,
 		 allowed ? "allowed" : "blocked");
-- 
2.49.0

[RFC PATCH 7/9] module: expose the list of blacklisted modules

[Simon THOBY]

Other kernel components (e.g. the Loadpol LSM) may need to access the
modules blacklist.

Signed-off-by: Simon THOBY <git@nightmared.fr>
---
 include/linux/module.h | 3 +++
 kernel/module/main.c   | 5 +++++
 2 files changed, 8 insertions(+)

diff --git a/include/linux/module.h b/include/linux/module.h
index b6b8d6f7f599..2ef50d81dc8d 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -781,6 +781,9 @@ void set_module_sig_enforced(void);
 
 void module_for_each_mod(int(*func)(struct module *mod, void *data), void *data);
 
+/* return a comma-separated list of blacklisted kernel modules */
+const char *get_blacklisted_modules(void);
+
 #else /* !CONFIG_MODULES... */
 
 static inline struct module *__module_address(unsigned long addr)
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 12a1a5f4d823..8f0afbc57df7 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2765,6 +2765,11 @@ static bool blacklisted(const char *module_name)
 }
 core_param(module_blacklist, module_blacklist, charp, 0400);
 
+const char *get_blacklisted_modules(void)
+{
+	return module_blacklist;
+}
+
 static struct module *layout_and_allocate(struct load_info *info, int flags)
 {
 	struct module *mod;
-- 
2.49.0

[RFC PATCH 8/9] Loadpol LSM: include the blacklisted kernel modules in the policy

[Simon THOBY]

When kernel modules are blacklisted, list them explicitly in the loadpol
policy.

Signed-off-by: Simon THOBY <git@nightmared.fr>
---
 security/loadpol/loadpol.c | 35 ++++++++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/security/loadpol/loadpol.c b/security/loadpol/loadpol.c
index 4d1a495a1462..c3c1846a3398 100644
--- a/security/loadpol/loadpol.c
+++ b/security/loadpol/loadpol.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 
 #include "linux/array_size.h"
+#include <linux/module.h>
 #include <linux/lsm_hooks.h>
 #include <uapi/linux/lsm.h>
 
@@ -33,12 +34,36 @@ DEFINE_LSM(LOADPOL_NAME) = {
 
 static int __init loadpol_init(void)
 {
+	struct loadpol_policy_entry *entry;
+	char *module_name;
+
+	const char *module_blacklist = get_blacklisted_modules();
+
+	if (module_blacklist) {
+		size_t len;
+
+		for (const char *p = module_blacklist; *p; p += len) {
+			len = strcspn(p, ",");
+
+			module_name = kstrndup(p, len, GFP_KERNEL);
+			entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+			if (!module_name || !entry)
+				return -ENOMEM;
+
+			entry->origin = (ORIGIN_USERSPACE | ORIGIN_KERNEL);
+			entry->action = ACTION_DENY;
+			entry->module_name = module_name;
+			list_add_tail(&entry->list, loadpol_policy);
+
+			if (p[len] == ',')
+				len++;
+		}
+	}
+
 	for (int i = 0; i < ARRAY_SIZE(default_policy_entries); i++) {
-		struct loadpol_policy_entry *entry = kmemdup(
-			&default_policy_entries[i],
-			sizeof(struct loadpol_policy_entry),
-			GFP_KERNEL
-		);
+		entry = kmemdup(&default_policy_entries[i],
+				sizeof(struct loadpol_policy_entry),
+				GFP_KERNEL);
 		if (!entry)
 			return -ENOMEM;
 
-- 
2.49.0

[RFC PATCH 9/9] Loadpol LSM: add a minimal documentation

[Simon THOBY]

Introduce a minimal documentation for Loadpol, presenting the policy
format and the two user interfaces: the securityfs policy file and the
sysctl.

Signed-off-by: Simon THOBY <git@nightmared.fr>
---
 Documentation/admin-guide/LSM/Loadpol.rst | 81 +++++++++++++++++++++++
 Documentation/admin-guide/LSM/index.rst   |  1 +
 2 files changed, 82 insertions(+)
 create mode 100644 Documentation/admin-guide/LSM/Loadpol.rst

diff --git a/Documentation/admin-guide/LSM/Loadpol.rst b/Documentation/admin-guide/LSM/Loadpol.rst
new file mode 100644
index 000000000000..0aa24a8d393c
--- /dev/null
+++ b/Documentation/admin-guide/LSM/Loadpol.rst
@@ -0,0 +1,81 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=======
+Loadpol
+=======
+
+Loadpol is a Linux Security Module that enforces a user-provided policy
+when decided whether a dynamic module can be loaded or not.
+
+The policy can be read and rewritten at ``/sys/kernel/security/loadpol/policy``.
+
+A default policy is created that contains the current list of blacklisted modules,
+and a catch-all entry that allow loading any module.
+
+Policy format
+=============
+
+The policy is defined as a set of line-separated entries.
+Each entry define the conditions for a match (the origin of the load request and
+the name of the kernel module), and the action to take when the load request
+matches the entry.
+
+
+Entry syntax: ``[origin=(userspace|kernel|kernel,userspace)] [module=<module_name>] action=(allow|deny)``
+
+There are two matching conditions:
+
+``origin``:
+    Load Requests can come from two origins:
+
+    * ``userspace`` (ie. a program in userspace called modprobe/insmod)
+    * ``kernel`` (the kernel requested the module directly by calling
+      ``request_module(...)``, e.g. loading a filesystem when performing a
+      ``-o loop`` mount).
+
+    When unspecified, the condition defaults to ``kernel,userspace`` (which means
+    that both origins match).
+
+``module``:
+    Name of the kernel module being matched. The name can contain wilcards.
+    Beware, module aliases do not work!
+
+There are two possible actions:
+
+* ``allow``: permit the load of the kernel module.
+* ``deny``: reject the load of the kernel module and emit an audit log.
+
+The policy is not greedy: as soon as a match is found, the evaluation terminates
+with the result of that match. So be very careful with the order of your entries.
+
+The main use cases of the policy will probably be to define an allowlist
+(here, we allow ``module_a`` and any module starting with ``module_b`` loaded
+by the user)::
+
+	module==module_a action=allow
+	origin==user module==module_b* action=deny
+	action=deny
+
+But other mechanisms are possible, like a denylist
+(here we block ``module_a``, ``module_b`` if it is loaded by the kernel and
+any module starting with ``module_c`` loaded by the user)::
+
+	module==module_a action=deny
+	origin==kernel module==module_b action=deny
+	origin==user module==module_c* action=deny
+	action=allow
+
+Policy lock
+===========
+
+In order to protect the policy from tampering, a sysctl is provided to
+lock-in-place the currently-loaded policy.
+
+The ``security.loadpol.locked`` can take 2 values:
+
+0 - default:
+    the policy can be reloaded at runtime by any administrator.
+
+1 - locked:
+    the policy cannot be updated or modified, and loadpol cannot be disabled
+    without rebooting.
diff --git a/Documentation/admin-guide/LSM/index.rst b/Documentation/admin-guide/LSM/index.rst
index b44ef68f6e4d..01d36670d8ad 100644
--- a/Documentation/admin-guide/LSM/index.rst
+++ b/Documentation/admin-guide/LSM/index.rst
@@ -42,6 +42,7 @@ subdirectories.
 
    apparmor
    LoadPin
+   Loadpol
    SELinux
    Smack
    tomoyo
-- 
2.49.0

Re: [RFC PATCH 3/9] Loadpol LSM: filter kernel module request according to the policy

[Casey Schaufler]

On 5/21/2025 7:01 AM, Simon THOBY wrote:
> When a kernel module is loaded, the LSM accepts or rejects the demand
> according to its policy.
>
> Signed-off-by: Simon THOBY <git@nightmared.fr>
> ---
>  security/loadpol/Makefile         |  2 +-
>  security/loadpol/loadpol.c        | 22 ++++++++++++
>  security/loadpol/loadpol.h        | 27 ++++++++++++++
>  security/loadpol/loadpol_policy.c | 59 +++++++++++++++++++++++++++++++
>  4 files changed, 109 insertions(+), 1 deletion(-)
>  create mode 100644 security/loadpol/loadpol_policy.c
>
> diff --git a/security/loadpol/Makefile b/security/loadpol/Makefile
> index a794c8cfbfee..062215e1f831 100644
> --- a/security/loadpol/Makefile
> +++ b/security/loadpol/Makefile
> @@ -1 +1 @@
> -obj-$(CONFIG_SECURITY_LOADPOL) := loadpol.o
> +obj-$(CONFIG_SECURITY_LOADPOL) := loadpol.o loadpol_policy.o
> diff --git a/security/loadpol/loadpol.c b/security/loadpol/loadpol.c
> index 3fc29263e2f8..4d1a495a1462 100644
> --- a/security/loadpol/loadpol.c
> +++ b/security/loadpol/loadpol.c
> @@ -6,6 +6,15 @@
>  
>  #include "loadpol.h"
>  
> +// default policy: allow all modules
> +static struct loadpol_policy_entry default_policy_entries[] __ro_after_init = {
> +	{
> +		.origin = (ORIGIN_KERNEL | ORIGIN_USERSPACE),
> +		.action = ACTION_ALLOW,
> +		.module_name = NULL,
> +	},
> +};
> +
>  static int __init loadpol_init(void);
>  
>  static const struct lsm_id loadpol_lsmid = {
> @@ -14,6 +23,7 @@ static const struct lsm_id loadpol_lsmid = {
>  };
>  
>  static struct security_hook_list loadpol_hooks[] __ro_after_init = {
> +	LSM_HOOK_INIT(kernel_module_load, loadpol_kernel_module_load),
>  };
>  
>  DEFINE_LSM(LOADPOL_NAME) = {
> @@ -23,6 +33,18 @@ DEFINE_LSM(LOADPOL_NAME) = {
>  
>  static int __init loadpol_init(void)
>  {
> +	for (int i = 0; i < ARRAY_SIZE(default_policy_entries); i++) {
> +		struct loadpol_policy_entry *entry = kmemdup(
> +			&default_policy_entries[i],
> +			sizeof(struct loadpol_policy_entry),
> +			GFP_KERNEL
> +		);
> +		if (!entry)
> +			return -ENOMEM;
> +
> +		list_add_tail(&entry->list, loadpol_policy);
> +	}
> +
>  	security_add_hooks(loadpol_hooks, ARRAY_SIZE(loadpol_hooks), &loadpol_lsmid);
>  	pr_info("Loadpol started.\n");
>  	return 0;
> diff --git a/security/loadpol/loadpol.h b/security/loadpol/loadpol.h
> index 5e11474191f0..a81d52f6d4da 100644
> --- a/security/loadpol/loadpol.h
> +++ b/security/loadpol/loadpol.h
> @@ -3,6 +3,33 @@
>  #ifndef _SECURITY_LOADPOL_LOADPOL_H
>  #define _SECURITY_LOADPOL_LOADPOL_H
>  
> +#include "linux/list.h"
> +
>  #define LOADPOL_NAME "loadpol"
>  
> +enum policy_entry_origin {
> +	ORIGIN_KERNEL = 1 << 0,
> +	ORIGIN_USERSPACE = 1 << 1,
> +};
> +
> +enum __packed policy_entry_action {
> +	ACTION_UNDEFINED,
> +	ACTION_ALLOW,
> +	ACTION_DENY
> +};
> +
> +struct loadpol_policy_entry {
> +	struct list_head list;
> +	// bitfield of policy_entry_origin

The // comment style is not used in the kernel.

> +	u8 origin;
> +	enum policy_entry_action action;
> +	// when NULL, the policy apply to every module
> +	char *module_name;
> +};
> +
> +extern struct list_head __rcu *loadpol_policy;
> +
> +// evaluate if a kernel module called 'kmod' is allowed to be loaded in the kernel
> +int loadpol_kernel_module_load(const char *kmod);
> +
>  #endif /* _SECURITY_LOADPOL_LOADPOL_H */
> diff --git a/security/loadpol/loadpol_policy.c b/security/loadpol/loadpol_policy.c
> new file mode 100644
> index 000000000000..6ba5ab600e3e
> --- /dev/null
> +++ b/security/loadpol/loadpol_policy.c
> @@ -0,0 +1,59 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include "linux/rculist.h"
> +#include <linux/sched.h>
> +#include <linux/sysctl.h>
> +#include <linux/parser.h>
> +
> +#include "loadpol.h"
> +
> +/*  use A/B policy entries: switch from one to the next every time the policy get overwritten */
> +static LIST_HEAD(loadpol_policy_a);
> +static LIST_HEAD(loadpol_policy_b);
> +struct list_head __rcu *loadpol_policy = (struct list_head __rcu *)(&loadpol_policy_a);
> +
> +int loadpol_kernel_module_load(const char *kmod)
> +{
> +	struct task_struct *parent_task;
> +	struct loadpol_policy_entry *entry;
> +	struct list_head *policy_list_tmp;
> +	enum policy_entry_origin orig = ORIGIN_USERSPACE;
> +	bool allowed = false;
> +
> +	rcu_read_lock();
> +	parent_task = rcu_dereference(current->parent);
> +	/* the parent of the current task is a workqueue -> the request comes from the kernel */
> +	if (parent_task && (parent_task->flags & PF_WQ_WORKER))
> +		orig = ORIGIN_KERNEL;
> +	rcu_read_unlock();
> +
> +	pr_debug("Loadpol: trying to load '%s' (asked by %s)",
> +		 kmod,
> +		 orig == ORIGIN_KERNEL ? "kernel" : "userspace");
> +
> +	rcu_read_lock();
> +	policy_list_tmp = rcu_dereference(loadpol_policy);
> +	list_for_each_entry_rcu(entry, policy_list_tmp, list) {
> +		/* the requestor does not match */
> +		if ((orig & entry->origin) == 0)
> +			continue;
> +
> +		allowed = entry->action == ACTION_ALLOW;
> +
> +		if (!entry->module_name)
> +			goto unlock_and_exit;
> +
> +		if (entry->module_name && match_wildcard(entry->module_name, kmod))
> +			goto unlock_and_exit;
> +	}
> +
> +	/* No match -> reject the demand */
> +	allowed = false;
> +
> +unlock_and_exit:
> +	rcu_read_unlock();
> +
> +	pr_debug("Loadpol: load of module '%s' %s", kmod, allowed ? "allowed" : "blocked");
> +
> +	return allowed ? 0 : -EPERM;
> +}

Re: [RFC PATCH 3/9] Loadpol LSM: filter kernel module request according to the policy

[Randy Dunlap]

On 5/21/25 8:47 AM, Casey Schaufler wrote:
> On 5/21/2025 7:01 AM, Simon THOBY wrote:
>> When a kernel module is loaded, the LSM accepts or rejects the demand
>> according to its policy.
>>
>> Signed-off-by: Simon THOBY <git@nightmared.fr>
>> ---
>>  security/loadpol/Makefile         |  2 +-
>>  security/loadpol/loadpol.c        | 22 ++++++++++++
>>  security/loadpol/loadpol.h        | 27 ++++++++++++++
>>  security/loadpol/loadpol_policy.c | 59 +++++++++++++++++++++++++++++++
>>  4 files changed, 109 insertions(+), 1 deletion(-)
>>  create mode 100644 security/loadpol/loadpol_policy.c
>>

>> +
>> +struct loadpol_policy_entry {
>> +	struct list_head list;
>> +	// bitfield of policy_entry_origin
> 
> The // comment style is not used in the kernel.

Counter:

https://lore.kernel.org/lkml/CA+55aFyQYJerovMsSoSKS7PessZBr4vNp-3QUUwhqk4A4_jcbg@mail.gmail.com/


-- 
~Randy

Re: [RFC PATCH 9/9] Loadpol LSM: add a minimal documentation

[Randy Dunlap]

Hi--

On 5/21/25 7:01 AM, Simon THOBY wrote:
> Introduce a minimal documentation for Loadpol, presenting the policy
> format and the two user interfaces: the securityfs policy file and the
> sysctl.
> 
> Signed-off-by: Simon THOBY <git@nightmared.fr>
> ---
>  Documentation/admin-guide/LSM/Loadpol.rst | 81 +++++++++++++++++++++++
>  Documentation/admin-guide/LSM/index.rst   |  1 +
>  2 files changed, 82 insertions(+)
>  create mode 100644 Documentation/admin-guide/LSM/Loadpol.rst
> 
> diff --git a/Documentation/admin-guide/LSM/Loadpol.rst b/Documentation/admin-guide/LSM/Loadpol.rst
> new file mode 100644
> index 000000000000..0aa24a8d393c
> --- /dev/null
> +++ b/Documentation/admin-guide/LSM/Loadpol.rst
> @@ -0,0 +1,81 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +=======
> +Loadpol
> +=======
> +
> +Loadpol is a Linux Security Module that enforces a user-provided policy
> +when decided whether a dynamic module can be loaded or not.
> +
> +The policy can be read and rewritten at ``/sys/kernel/security/loadpol/policy``.
> +
> +A default policy is created that contains the current list of blacklisted modules,

Where does the current list of blacklisted modules come from?
Is it from the kernel command line parameter "module_blacklist=" or
somewhere else?

> +and a catch-all entry that allow loading any module.
> +
> +Policy format
> +=============
> +
> +The policy is defined as a set of line-separated entries.
> +Each entry define the conditions for a match (the origin of the load request and
> +the name of the kernel module), and the action to take when the load request
> +matches the entry.
> +
> +
> +Entry syntax: ``[origin=(userspace|kernel|kernel,userspace)] [module=<module_name>] action=(allow|deny)``
> +
> +There are two matching conditions:
> +
> +``origin``:
> +    Load Requests can come from two origins:
> +
> +    * ``userspace`` (ie. a program in userspace called modprobe/insmod)
                       (i.e.,

> +    * ``kernel`` (the kernel requested the module directly by calling
> +      ``request_module(...)``, e.g. loading a filesystem when performing a
> +      ``-o loop`` mount).
> +
> +    When unspecified, the condition defaults to ``kernel,userspace`` (which means
> +    that both origins match).
> +
> +``module``:
> +    Name of the kernel module being matched. The name can contain wilcards.

                                                                     wildcards.

> +    Beware, module aliases do not work!
> +


-- 
~Randy

Re: [RFC PATCH 3/9] Loadpol LSM: filter kernel module request according to the policy

[Simon Thoby]

On 5/21/25 17:47, Casey Schaufler wrote:
> On 5/21/2025 7:01 AM, Simon THOBY wrote:
>> When a kernel module is loaded, the LSM accepts or rejects the demand
>> according to its policy.
>>
>> Signed-off-by: Simon THOBY <git@nightmared.fr>
>> ---
>>  security/loadpol/Makefile         |  2 +-
>>  security/loadpol/loadpol.c        | 22 ++++++++++++
>>  security/loadpol/loadpol.h        | 27 ++++++++++++++
>>  security/loadpol/loadpol_policy.c | 59 +++++++++++++++++++++++++++++++
>>  4 files changed, 109 insertions(+), 1 deletion(-)
>>  create mode 100644 security/loadpol/loadpol_policy.c
>>
>> diff --git a/security/loadpol/Makefile b/security/loadpol/Makefile
>> index a794c8cfbfee..062215e1f831 100644
>> --- a/security/loadpol/Makefile
>> +++ b/security/loadpol/Makefile
>> @@ -1 +1 @@
>> -obj-$(CONFIG_SECURITY_LOADPOL) := loadpol.o
>> +obj-$(CONFIG_SECURITY_LOADPOL) := loadpol.o loadpol_policy.o
>> diff --git a/security/loadpol/loadpol.c b/security/loadpol/loadpol.c
>> index 3fc29263e2f8..4d1a495a1462 100644
>> --- a/security/loadpol/loadpol.c
>> +++ b/security/loadpol/loadpol.c
>> @@ -6,6 +6,15 @@
>>  
>>  #include "loadpol.h"
>>  
>> +// default policy: allow all modules
>> +static struct loadpol_policy_entry default_policy_entries[] __ro_after_init = {
>> +	{
>> +		.origin = (ORIGIN_KERNEL | ORIGIN_USERSPACE),
>> +		.action = ACTION_ALLOW,
>> +		.module_name = NULL,
>> +	},
>> +};
>> +
>>  static int __init loadpol_init(void);
>>  
>>  static const struct lsm_id loadpol_lsmid = {
>> @@ -14,6 +23,7 @@ static const struct lsm_id loadpol_lsmid = {
>>  };
>>  
>>  static struct security_hook_list loadpol_hooks[] __ro_after_init = {
>> +	LSM_HOOK_INIT(kernel_module_load, loadpol_kernel_module_load),
>>  };
>>  
>>  DEFINE_LSM(LOADPOL_NAME) = {
>> @@ -23,6 +33,18 @@ DEFINE_LSM(LOADPOL_NAME) = {
>>  
>>  static int __init loadpol_init(void)
>>  {
>> +	for (int i = 0; i < ARRAY_SIZE(default_policy_entries); i++) {
>> +		struct loadpol_policy_entry *entry = kmemdup(
>> +			&default_policy_entries[i],
>> +			sizeof(struct loadpol_policy_entry),
>> +			GFP_KERNEL
>> +		);
>> +		if (!entry)
>> +			return -ENOMEM;
>> +
>> +		list_add_tail(&entry->list, loadpol_policy);
>> +	}
>> +
>>  	security_add_hooks(loadpol_hooks, ARRAY_SIZE(loadpol_hooks), &loadpol_lsmid);
>>  	pr_info("Loadpol started.\n");
>>  	return 0;
>> diff --git a/security/loadpol/loadpol.h b/security/loadpol/loadpol.h
>> index 5e11474191f0..a81d52f6d4da 100644
>> --- a/security/loadpol/loadpol.h
>> +++ b/security/loadpol/loadpol.h
>> @@ -3,6 +3,33 @@
>>  #ifndef _SECURITY_LOADPOL_LOADPOL_H
>>  #define _SECURITY_LOADPOL_LOADPOL_H
>>  
>> +#include "linux/list.h"
>> +
>>  #define LOADPOL_NAME "loadpol"
>>  
>> +enum policy_entry_origin {
>> +	ORIGIN_KERNEL = 1 << 0,
>> +	ORIGIN_USERSPACE = 1 << 1,
>> +};
>> +
>> +enum __packed policy_entry_action {
>> +	ACTION_UNDEFINED,
>> +	ACTION_ALLOW,
>> +	ACTION_DENY
>> +};
>> +
>> +struct loadpol_policy_entry {
>> +	struct list_head list;
>> +	// bitfield of policy_entry_origin
> 
> The // comment style is not used in the kernel.
> 

Whoops, I had originally started with '//' comments before realizing the kernel comment
policy tends towards /* */ pairs, but looks like I haven't fixed all the '//' insertions I made.

Good catch!

<snip>

Re: [RFC PATCH 9/9] Loadpol LSM: add a minimal documentation

[Simon Thoby]

On 5/21/25 18:26, Randy Dunlap wrote:
> Hi--
> 
> On 5/21/25 7:01 AM, Simon THOBY wrote:
>> Introduce a minimal documentation for Loadpol, presenting the policy
>> format and the two user interfaces: the securityfs policy file and the
>> sysctl.
>>
>> Signed-off-by: Simon THOBY <git@nightmared.fr>
>> ---
>>  Documentation/admin-guide/LSM/Loadpol.rst | 81 +++++++++++++++++++++++
>>  Documentation/admin-guide/LSM/index.rst   |  1 +
>>  2 files changed, 82 insertions(+)
>>  create mode 100644 Documentation/admin-guide/LSM/Loadpol.rst
>>
>> diff --git a/Documentation/admin-guide/LSM/Loadpol.rst b/Documentation/admin-guide/LSM/Loadpol.rst
>> new file mode 100644
>> index 000000000000..0aa24a8d393c
>> --- /dev/null
>> +++ b/Documentation/admin-guide/LSM/Loadpol.rst
>> @@ -0,0 +1,81 @@
>> +.. SPDX-License-Identifier: GPL-2.0
>> +
>> +=======
>> +Loadpol
>> +=======
>> +
>> +Loadpol is a Linux Security Module that enforces a user-provided policy
>> +when decided whether a dynamic module can be loaded or not.
>> +
>> +The policy can be read and rewritten at ``/sys/kernel/security/loadpol/policy``.
>> +
>> +A default policy is created that contains the current list of blacklisted modules,
> 
> Where does the current list of blacklisted modules come from?
> Is it from the kernel command line parameter "module_blacklist=" or
> somewhere else?
> 

Correct, it comes from that command line argument.
It is actually not really necessary to include it in the policy (the kernel blacklist is applied
prior to the LSM hook, so it will always apply anyway), but I thought it nice to have a "bird view"
of all the policies that apply that can prevent the load of a kernel module.

>> +and a catch-all entry that allow loading any module.
>> +
>> +Policy format
>> +=============
>> +
>> +The policy is defined as a set of line-separated entries.
>> +Each entry define the conditions for a match (the origin of the load request and
>> +the name of the kernel module), and the action to take when the load request
>> +matches the entry.
>> +
>> +
>> +Entry syntax: ``[origin=(userspace|kernel|kernel,userspace)] [module=<module_name>] action=(allow|deny)``
>> +
>> +There are two matching conditions:
>> +
>> +``origin``:
>> +    Load Requests can come from two origins:
>> +
>> +    * ``userspace`` (ie. a program in userspace called modprobe/insmod)
>                        (i.e.,
> 
>> +    * ``kernel`` (the kernel requested the module directly by calling
>> +      ``request_module(...)``, e.g. loading a filesystem when performing a
>> +      ``-o loop`` mount).
>> +
>> +    When unspecified, the condition defaults to ``kernel,userspace`` (which means
>> +    that both origins match).
>> +
>> +``module``:
>> +    Name of the kernel module being matched. The name can contain wilcards.
> 
>                                                                      wildcards.
> 
>> +    Beware, module aliases do not work!
>> +
> 
> 

Thanks for the typos, will fix.

Re: [RFC PATCH 9/9] Loadpol LSM: add a minimal documentation

[Paul Moore]

On Wed, May 21, 2025 at 10:03 AM Simon THOBY <git@nightmared.fr> wrote:
>
> Introduce a minimal documentation for Loadpol, presenting the policy
> format and the two user interfaces: the securityfs policy file and the
> sysctl.
>
> Signed-off-by: Simon THOBY <git@nightmared.fr>
> ---
>  Documentation/admin-guide/LSM/Loadpol.rst | 81 +++++++++++++++++++++++
>  Documentation/admin-guide/LSM/index.rst   |  1 +
>  2 files changed, 82 insertions(+)
>  create mode 100644 Documentation/admin-guide/LSM/Loadpol.rst
>
> diff --git a/Documentation/admin-guide/LSM/Loadpol.rst b/Documentation/admin-guide/LSM/Loadpol.rst
> new file mode 100644
> index 000000000000..0aa24a8d393c
> --- /dev/null
> +++ b/Documentation/admin-guide/LSM/Loadpol.rst
> @@ -0,0 +1,81 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +=======
> +Loadpol
> +=======
> +
> +Loadpol is a Linux Security Module that enforces a user-provided policy
> +when decided whether a dynamic module can be loaded or not.

Considering the relatively small scope of Loadpol, I have to ask if
you've considered augmenting other LSMs to meet your needs?  While
LoadPin is different from what you are proposing here, it does
similarly limit its scope to kernel module load operations, and given
the current simplicity of LoadPin I imagine one could find a creative
way to extend it to support what you are trying to do.

> +The policy can be read and rewritten at ``/sys/kernel/security/loadpol/policy``.
> +
> +A default policy is created that contains the current list of blacklisted modules,
> +and a catch-all entry that allow loading any module.
> +
> +Policy format
> +=============
> +
> +The policy is defined as a set of line-separated entries.
> +Each entry define the conditions for a match (the origin of the load request and
> +the name of the kernel module), and the action to take when the load request
> +matches the entry.
> +
> +
> +Entry syntax: ``[origin=(userspace|kernel|kernel,userspace)] [module=<module_name>] action=(allow|deny)``
> +
> +There are two matching conditions:
> +
> +``origin``:
> +    Load Requests can come from two origins:
> +
> +    * ``userspace`` (ie. a program in userspace called modprobe/insmod)
> +    * ``kernel`` (the kernel requested the module directly by calling
> +      ``request_module(...)``, e.g. loading a filesystem when performing a
> +      ``-o loop`` mount).
> +
> +    When unspecified, the condition defaults to ``kernel,userspace`` (which means
> +    that both origins match).
> +
> +``module``:
> +    Name of the kernel module being matched. The name can contain wilcards.
> +    Beware, module aliases do not work!

It would be good to have a section in the documentation where you
discuss how the risks inherent to filtering on the module name, and
approaches that can be used to ensure that a malicious module is not
simply "borrowing" a known good module's name.

> +There are two possible actions:
> +
> +* ``allow``: permit the load of the kernel module.
> +* ``deny``: reject the load of the kernel module and emit an audit log.
> +
> +The policy is not greedy: as soon as a match is found, the evaluation terminates
> +with the result of that match. So be very careful with the order of your entries.
> +
> +The main use cases of the policy will probably be to define an allowlist
> +(here, we allow ``module_a`` and any module starting with ``module_b`` loaded
> +by the user)::
> +
> +       module==module_a action=allow
> +       origin==user module==module_b* action=deny
> +       action=deny
> +
> +But other mechanisms are possible, like a denylist
> +(here we block ``module_a``, ``module_b`` if it is loaded by the kernel and
> +any module starting with ``module_c`` loaded by the user)::
> +
> +       module==module_a action=deny
> +       origin==kernel module==module_b action=deny
> +       origin==user module==module_c* action=deny
> +       action=allow
> +
> +Policy lock
> +===========
> +
> +In order to protect the policy from tampering, a sysctl is provided to
> +lock-in-place the currently-loaded policy.
> +
> +The ``security.loadpol.locked`` can take 2 values:
> +
> +0 - default:
> +    the policy can be reloaded at runtime by any administrator.
> +
> +1 - locked:
> +    the policy cannot be updated or modified, and loadpol cannot be disabled
> +    without rebooting.
> diff --git a/Documentation/admin-guide/LSM/index.rst b/Documentation/admin-guide/LSM/index.rst
> index b44ef68f6e4d..01d36670d8ad 100644
> --- a/Documentation/admin-guide/LSM/index.rst
> +++ b/Documentation/admin-guide/LSM/index.rst
> @@ -42,6 +42,7 @@ subdirectories.
>
>     apparmor
>     LoadPin
> +   Loadpol
>     SELinux
>     Smack
>     tomoyo
> --
> 2.49.0

-- 
paul-moore.com

Re: [RFC PATCH 1/9] LSM: Introduce a new hook: security_kernel_module_load

[Serge E. Hallyn]

On Wed, May 21, 2025 at 04:01:05PM +0200, Simon THOBY wrote:
> Introduce a new hook to allow LSMs to decide whether to block the load
> of a kernel module.
> 
> Two hooks already exist:
> - kernel_module_request is called when the kernel itself (not userspace)
>  request the load of a module, e.g. because a device was detected.
>  - security_kernel_load_data(LOADING_MODULE) is called when userspace calls
>  init_module/finit_module, but lack information about the module because
>  its  headers have not been loaded into kernel space, let alone parsed.
>  This may not be sufficient for some LSMs.
> 
> This new hook is similar to security_kernel_load_data(LOADING_MODULE),
> but called after the module signature and header are verified, and only
> takes the module name for now.
> 
> Signed-off-by: Simon THOBY <git@nightmared.fr>
> ---
>  include/linux/lsm_hook_defs.h |  1 +
>  include/linux/module.h        |  1 +
>  include/linux/security.h      |  6 ++++++
>  kernel/module/main.c          |  4 ++++
>  security/security.c           | 14 ++++++++++++++
>  5 files changed, 26 insertions(+)
> 
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index bf3bbac4e02a..51c5212d8bb6 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -223,6 +223,7 @@ LSM_HOOK(void, LSM_RET_VOID, cred_getlsmprop, const struct cred *c,
>  LSM_HOOK(int, 0, kernel_act_as, struct cred *new, u32 secid)
>  LSM_HOOK(int, 0, kernel_create_files_as, struct cred *new, struct inode *inode)
>  LSM_HOOK(int, 0, kernel_module_request, char *kmod_name)
> +LSM_HOOK(int, 0, kernel_module_load, const char *kmod_name)
>  LSM_HOOK(int, 0, kernel_load_data, enum kernel_load_data_id id, bool contents)
>  LSM_HOOK(int, 0, kernel_post_load_data, char *buf, loff_t size,
>  	 enum kernel_load_data_id id, char *description)
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 8050f77c3b64..b6b8d6f7f599 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -39,6 +39,7 @@ struct modversion_info {
>  	char name[MODULE_NAME_LEN];
>  };
>  
> +struct load_info;
>  struct module;
>  struct exception_table_entry;
>  
> diff --git a/include/linux/security.h b/include/linux/security.h
> index cc9b54d95d22..e175b2cc8caf 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -498,6 +498,7 @@ void security_cred_getlsmprop(const struct cred *c, struct lsm_prop *prop);
>  int security_kernel_act_as(struct cred *new, u32 secid);
>  int security_kernel_create_files_as(struct cred *new, struct inode *inode);
>  int security_kernel_module_request(char *kmod_name);
> +int security_kernel_module_load(const char *kmod_name);
>  int security_kernel_load_data(enum kernel_load_data_id id, bool contents);
>  int security_kernel_post_load_data(char *buf, loff_t size,
>  				   enum kernel_load_data_id id,
> @@ -1255,6 +1256,11 @@ static inline int security_kernel_module_request(char *kmod_name)
>  	return 0;
>  }
>  
> +static inline int security_kernel_module_load(const char *kmod_name)
> +{
> +	return 0;
> +}
> +
>  static inline int security_kernel_load_data(enum kernel_load_data_id id, bool contents)
>  {
>  	return 0;
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index a2859dc3eea6..12a1a5f4d823 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -3228,6 +3228,10 @@ static int early_mod_check(struct load_info *info, int flags)
>  		return -EPERM;
>  	}
>  
> +	err = security_kernel_module_load(info->name);

Would it be more useful to pass in the whole info struct?

> +	if (err)
> +		return err;
> +
>  	err = rewrite_section_headers(info, flags);
>  	if (err)
>  		return err;
> diff --git a/security/security.c b/security/security.c
> index fb57e8fddd91..b9430499c332 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -3336,6 +3336,20 @@ int security_kernel_module_request(char *kmod_name)
>  	return call_int_hook(kernel_module_request, kmod_name);
>  }
>  
> +/**
> + * security_kernel_module_load() - Check if loading a module is allowed
> + * @kmod_name: name of the kernel module being loaded
> + *
> + * This method is called when the userspace called init_module/finit_module
> + * with a valid module
> + *
> + * Return: Returns 0 if successful.
> + */
> +int security_kernel_module_load(const char *kmod_name)
> +{
> +	return call_int_hook(kernel_module_load, kmod_name);
> +}
> +
>  /**
>   * security_kernel_read_file() - Read a file specified by userspace
>   * @file: file
> -- 
> 2.49.0
> 

Re: [RFC PATCH 1/9] LSM: Introduce a new hook: security_kernel_module_load

[Simon Thoby]

On 5/22/25 00:03, Serge E. Hallyn wrote:
> On Wed, May 21, 2025 at 04:01:05PM +0200, Simon THOBY wrote:
>> Introduce a new hook to allow LSMs to decide whether to block the load
>> of a kernel module.
>>
>> Two hooks already exist:
>> - kernel_module_request is called when the kernel itself (not userspace)
>>  request the load of a module, e.g. because a device was detected.
>>  - security_kernel_load_data(LOADING_MODULE) is called when userspace calls
>>  init_module/finit_module, but lack information about the module because
>>  its  headers have not been loaded into kernel space, let alone parsed.
>>  This may not be sufficient for some LSMs.
>>
>> This new hook is similar to security_kernel_load_data(LOADING_MODULE),
>> but called after the module signature and header are verified, and only
>> takes the module name for now.
>>
>> Signed-off-by: Simon THOBY <git@nightmared.fr>
>> ---
>>  include/linux/lsm_hook_defs.h |  1 +
>>  include/linux/module.h        |  1 +
>>  include/linux/security.h      |  6 ++++++
>>  kernel/module/main.c          |  4 ++++
>>  security/security.c           | 14 ++++++++++++++
>>  5 files changed, 26 insertions(+)
>>
>> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
>> index bf3bbac4e02a..51c5212d8bb6 100644
>> --- a/include/linux/lsm_hook_defs.h
>> +++ b/include/linux/lsm_hook_defs.h
>> @@ -223,6 +223,7 @@ LSM_HOOK(void, LSM_RET_VOID, cred_getlsmprop, const struct cred *c,
>>  LSM_HOOK(int, 0, kernel_act_as, struct cred *new, u32 secid)
>>  LSM_HOOK(int, 0, kernel_create_files_as, struct cred *new, struct inode *inode)
>>  LSM_HOOK(int, 0, kernel_module_request, char *kmod_name)
>> +LSM_HOOK(int, 0, kernel_module_load, const char *kmod_name)
>>  LSM_HOOK(int, 0, kernel_load_data, enum kernel_load_data_id id, bool contents)
>>  LSM_HOOK(int, 0, kernel_post_load_data, char *buf, loff_t size,
>>  	 enum kernel_load_data_id id, char *description)
>> diff --git a/include/linux/module.h b/include/linux/module.h
>> index 8050f77c3b64..b6b8d6f7f599 100644
>> --- a/include/linux/module.h
>> +++ b/include/linux/module.h
>> @@ -39,6 +39,7 @@ struct modversion_info {
>>  	char name[MODULE_NAME_LEN];
>>  };
>>  
>> +struct load_info;
>>  struct module;
>>  struct exception_table_entry;
>>  
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index cc9b54d95d22..e175b2cc8caf 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -498,6 +498,7 @@ void security_cred_getlsmprop(const struct cred *c, struct lsm_prop *prop);
>>  int security_kernel_act_as(struct cred *new, u32 secid);
>>  int security_kernel_create_files_as(struct cred *new, struct inode *inode);
>>  int security_kernel_module_request(char *kmod_name);
>> +int security_kernel_module_load(const char *kmod_name);
>>  int security_kernel_load_data(enum kernel_load_data_id id, bool contents);
>>  int security_kernel_post_load_data(char *buf, loff_t size,
>>  				   enum kernel_load_data_id id,
>> @@ -1255,6 +1256,11 @@ static inline int security_kernel_module_request(char *kmod_name)
>>  	return 0;
>>  }
>>  
>> +static inline int security_kernel_module_load(const char *kmod_name)
>> +{
>> +	return 0;
>> +}
>> +
>>  static inline int security_kernel_load_data(enum kernel_load_data_id id, bool contents)
>>  {
>>  	return 0;
>> diff --git a/kernel/module/main.c b/kernel/module/main.c
>> index a2859dc3eea6..12a1a5f4d823 100644
>> --- a/kernel/module/main.c
>> +++ b/kernel/module/main.c
>> @@ -3228,6 +3228,10 @@ static int early_mod_check(struct load_info *info, int flags)
>>  		return -EPERM;
>>  	}
>>  
>> +	err = security_kernel_module_load(info->name);
> 
> Would it be more useful to pass in the whole info struct?
> 

I thought about that, but was afraid the LSM hook is still called very early in
the boot process. I though the 'struct load_info' was only partially populated,
but upon further checking, you're right, and most fields of the structure were
already setup by the time the hook is called:
- len, hdr in the copy_module_from_user function
- sig_ok in module_sig_check
- sechdrs, secstrings, index, strtab and name in elf_validity_cache_copy

So I could definitely pass in the info struct instead.

On that note, I wonder if I should move 'struct load_info' out of kernel/module/internal.h,
because I'm fairly certain we don't want to have linux/security.h depending on an internal
header file from the module subsystem.

<snip>

Re: [RFC PATCH 9/9] Loadpol LSM: add a minimal documentation

[Simon Thoby]

On 5/21/25 23:31, Paul Moore wrote:
> On Wed, May 21, 2025 at 10:03 AM Simon THOBY <git@nightmared.fr> wrote:
>>
>> Introduce a minimal documentation for Loadpol, presenting the policy
>> format and the two user interfaces: the securityfs policy file and the
>> sysctl.
>>
>> Signed-off-by: Simon THOBY <git@nightmared.fr>
>> ---
>>  Documentation/admin-guide/LSM/Loadpol.rst | 81 +++++++++++++++++++++++
>>  Documentation/admin-guide/LSM/index.rst   |  1 +
>>  2 files changed, 82 insertions(+)
>>  create mode 100644 Documentation/admin-guide/LSM/Loadpol.rst
>>
>> diff --git a/Documentation/admin-guide/LSM/Loadpol.rst b/Documentation/admin-guide/LSM/Loadpol.rst
>> new file mode 100644
>> index 000000000000..0aa24a8d393c
>> --- /dev/null
>> +++ b/Documentation/admin-guide/LSM/Loadpol.rst
>> @@ -0,0 +1,81 @@
>> +.. SPDX-License-Identifier: GPL-2.0
>> +
>> +=======
>> +Loadpol
>> +=======
>> +
>> +Loadpol is a Linux Security Module that enforces a user-provided policy
>> +when decided whether a dynamic module can be loaded or not.
> 
> Considering the relatively small scope of Loadpol, I have to ask if
> you've considered augmenting other LSMs to meet your needs?  While
> LoadPin is different from what you are proposing here, it does
> similarly limit its scope to kernel module load operations, and given
> the current simplicity of LoadPin I imagine one could find a creative
> way to extend it to support what you are trying to do.
> 

I indeed felt a bit ridiculous introducing a new LSM for a limited feature
like that!
What's more, I don't see it being extended much in the future - we could
always imagine things like signed policy updates, but other than that it's
probably "feature-complete", as the feature itself is fairly small.
The difficulty with LoadPin is that it rely relies on the notion of
filesystem (which is coupled with the origin of the kernel modules) to ensure
that modules are valid. On a general-purpose distributions, the modules
would be stored on the same (non-integrity-verified) filesystem, so
segregating the modules by filesystem is not really possible there.
Extending LoadPin to provide the same features is probably possible, but I
fear this would add complexity to loading by trying to make it do two
slightly different jobs at once.

CC-ing Kees Cook as he is the maintainer of the LoadPin module, he may have
opinions on what features may or may not fit inside LoadPin.

As for other LSMs, I don't really see one that tries to provide similar
features, but I someone is open to extending an existing LSM for that, I
would be happy to help.

To stop the proliferation of LSMs (if this proves to be a problem),
I could see a "miscellaneous" one that groups the tiny LSMs (which each
feature set hidden behind a configuration option), like loadpin, yama,
safesetid, and future feature sets like loadpol.


>> +The policy can be read and rewritten at ``/sys/kernel/security/loadpol/policy``.
>> +
>> +A default policy is created that contains the current list of blacklisted modules,
>> +and a catch-all entry that allow loading any module.
>> +
>> +Policy format
>> +=============
>> +
>> +The policy is defined as a set of line-separated entries.
>> +Each entry define the conditions for a match (the origin of the load request and
>> +the name of the kernel module), and the action to take when the load request
>> +matches the entry.
>> +
>> +
>> +Entry syntax: ``[origin=(userspace|kernel|kernel,userspace)] [module=<module_name>] action=(allow|deny)``
>> +
>> +There are two matching conditions:
>> +
>> +``origin``:
>> +    Load Requests can come from two origins:
>> +
>> +    * ``userspace`` (ie. a program in userspace called modprobe/insmod)
>> +    * ``kernel`` (the kernel requested the module directly by calling
>> +      ``request_module(...)``, e.g. loading a filesystem when performing a
>> +      ``-o loop`` mount).
>> +
>> +    When unspecified, the condition defaults to ``kernel,userspace`` (which means
>> +    that both origins match).
>> +
>> +``module``:
>> +    Name of the kernel module being matched. The name can contain wilcards.
>> +    Beware, module aliases do not work!
> 
> It would be good to have a section in the documentation where you
> discuss how the risks inherent to filtering on the module name, and
> approaches that can be used to ensure that a malicious module is not
> simply "borrowing" a known good module's name.
> 
I understand the concern, but the idea of the LSM is not to substitute itself
to existing security mechanisms (as you know, it technically couldn't anyway, LSM
being all about stacking additional layers).
So we still rely on kernel module signing. To forge a malicious module with a valid
name, one would have to have access to the signing key. If that happens, I consider
the game to be over, as they can forge arbitrary kernel modules.
In most practical cases, I think people will trust two keys:
- the distributor key, built in the kernel
- User-supplied keys, distributed via MOK (or in the UEFI KEK maybe?)
Most modules would be signed with the distributor key, with security software
(or custom kernel drivers) being signed with the user key (hopefully off-system,
so that the end-users systems do not contain the signing key, otherwise this
defeats the whole point of signing).
All modules would still be signed, and we should be safe from a module lying about
its name (naming conflicts notwithstanding, but this shouldn't happen anyway).
But I could have an explanation in the documentation about that!

Re: [RFC PATCH 9/9] Loadpol LSM: add a minimal documentation

[Paul Moore]

On Thu, May 22, 2025 at 5:23 AM Simon Thoby <git@nightmared.fr> wrote:
> On 5/21/25 23:31, Paul Moore wrote:
> > On Wed, May 21, 2025 at 10:03 AM Simon THOBY <git@nightmared.fr> wrote:
> >>
> >> Introduce a minimal documentation for Loadpol, presenting the policy
> >> format and the two user interfaces: the securityfs policy file and the
> >> sysctl.
> >>
> >> Signed-off-by: Simon THOBY <git@nightmared.fr>
> >> ---
> >>  Documentation/admin-guide/LSM/Loadpol.rst | 81 +++++++++++++++++++++++
> >>  Documentation/admin-guide/LSM/index.rst   |  1 +
> >>  2 files changed, 82 insertions(+)
> >>  create mode 100644 Documentation/admin-guide/LSM/Loadpol.rst
> >>
> >> diff --git a/Documentation/admin-guide/LSM/Loadpol.rst b/Documentation/admin-guide/LSM/Loadpol.rst
> >> new file mode 100644
> >> index 000000000000..0aa24a8d393c
> >> --- /dev/null
> >> +++ b/Documentation/admin-guide/LSM/Loadpol.rst
> >> @@ -0,0 +1,81 @@
> >> +.. SPDX-License-Identifier: GPL-2.0
> >> +
> >> +=======
> >> +Loadpol
> >> +=======
> >> +
> >> +Loadpol is a Linux Security Module that enforces a user-provided policy
> >> +when decided whether a dynamic module can be loaded or not.
> >
> > Considering the relatively small scope of Loadpol, I have to ask if
> > you've considered augmenting other LSMs to meet your needs?  While
> > LoadPin is different from what you are proposing here, it does
> > similarly limit its scope to kernel module load operations, and given
> > the current simplicity of LoadPin I imagine one could find a creative
> > way to extend it to support what you are trying to do.
>
> I indeed felt a bit ridiculous introducing a new LSM for a limited feature
> like that!

Please don't feel bad about it, often simply doing "something" is what
makes things happen, even if that original "something" turns out not
to be the final "thing" :)

> What's more, I don't see it being extended much in the future - we could
> always imagine things like signed policy updates, but other than that it's
> probably "feature-complete", as the feature itself is fairly small.
> The difficulty with LoadPin is that it rely relies on the notion of
> filesystem (which is coupled with the origin of the kernel modules) to ensure
> that modules are valid. On a general-purpose distributions, the modules
> would be stored on the same (non-integrity-verified) filesystem, so
> segregating the modules by filesystem is not really possible there.
> Extending LoadPin to provide the same features is probably possible, but I
> fear this would add complexity to loading by trying to make it do two
> slightly different jobs at once.

My thinking around possible augmentation of LoadPin is that both
LoadPin and Loadpol share a similar, limited focus of controlling
access to kernel module loading and Loadpol has support for a basic
loadable policy, a policy that could likely be extended to support a
LoadPin-esque construct that limit module loading based on filesystem
pinning.  It probably makes more sense to think of adding LoadPin
support to Loadpol, rather than augmenting LoadPin to support the
Loadpol concepts, but for consistency with upstream we probably need
to speak in terms of the latter.

> CC-ing Kees Cook as he is the maintainer of the LoadPin module, he may have
> opinions on what features may or may not fit inside LoadPin.

Of course, the combination, or not, of LoadPin and Loadpol is really
up to the two of you.

> As for other LSMs, I don't really see one that tries to provide similar
> features, but I someone is open to extending an existing LSM for that, I
> would be happy to help.

I think LoadPin is the obvious choice.

-- 
paul-moore.com

Re: [RFC PATCH 9/9] Loadpol LSM: add a minimal documentation

[Simon Thoby]

On 5/30/25 01:49, Paul Moore wrote:
> On Thu, May 22, 2025 at 5:23 AM Simon Thoby <git@nightmared.fr> wrote:
>> On 5/21/25 23:31, Paul Moore wrote:
>>> On Wed, May 21, 2025 at 10:03 AM Simon THOBY <git@nightmared.fr> wrote:
>>>>
>>>> Introduce a minimal documentation for Loadpol, presenting the policy
>>>> format and the two user interfaces: the securityfs policy file and the
>>>> sysctl.
>>>>
>>>> Signed-off-by: Simon THOBY <git@nightmared.fr>
>>>> ---
>>>>  Documentation/admin-guide/LSM/Loadpol.rst | 81 +++++++++++++++++++++++
>>>>  Documentation/admin-guide/LSM/index.rst   |  1 +
>>>>  2 files changed, 82 insertions(+)
>>>>  create mode 100644 Documentation/admin-guide/LSM/Loadpol.rst
>>>>
>>>> diff --git a/Documentation/admin-guide/LSM/Loadpol.rst b/Documentation/admin-guide/LSM/Loadpol.rst
>>>> new file mode 100644
>>>> index 000000000000..0aa24a8d393c
>>>> --- /dev/null
>>>> +++ b/Documentation/admin-guide/LSM/Loadpol.rst
>>>> @@ -0,0 +1,81 @@
>>>> +.. SPDX-License-Identifier: GPL-2.0
>>>> +
>>>> +=======
>>>> +Loadpol
>>>> +=======
>>>> +
>>>> +Loadpol is a Linux Security Module that enforces a user-provided policy
>>>> +when decided whether a dynamic module can be loaded or not.
>>>
>>> Considering the relatively small scope of Loadpol, I have to ask if
>>> you've considered augmenting other LSMs to meet your needs?  While
>>> LoadPin is different from what you are proposing here, it does
>>> similarly limit its scope to kernel module load operations, and given
>>> the current simplicity of LoadPin I imagine one could find a creative
>>> way to extend it to support what you are trying to do.
>>
>> I indeed felt a bit ridiculous introducing a new LSM for a limited feature
>> like that!
> 
> Please don't feel bad about it, often simply doing "something" is what
> makes things happen, even if that original "something" turns out not
> to be the final "thing" :)
> 
>> What's more, I don't see it being extended much in the future - we could
>> always imagine things like signed policy updates, but other than that it's
>> probably "feature-complete", as the feature itself is fairly small.
>> The difficulty with LoadPin is that it rely relies on the notion of
>> filesystem (which is coupled with the origin of the kernel modules) to ensure
>> that modules are valid. On a general-purpose distributions, the modules
>> would be stored on the same (non-integrity-verified) filesystem, so
>> segregating the modules by filesystem is not really possible there.
>> Extending LoadPin to provide the same features is probably possible, but I
>> fear this would add complexity to loading by trying to make it do two
>> slightly different jobs at once.
> 
> My thinking around possible augmentation of LoadPin is that both
> LoadPin and Loadpol share a similar, limited focus of controlling
> access to kernel module loading and Loadpol has support for a basic
> loadable policy, a policy that could likely be extended to support a
> LoadPin-esque construct that limit module loading based on filesystem
> pinning.  It probably makes more sense to think of adding LoadPin
> support to Loadpol, rather than augmenting LoadPin to support the
> Loadpol concepts, but for consistency with upstream we probably need
> to speak in terms of the latter.
> 
Thanks for the reply, I now see what you meant. I will try to put something
together (hopefully next week), starting with looking at how we can express
the current LoadPin feature set as a loadable and user-extensible policy, and
then add non-filesystem-related policy entries (like module name restrictions)
to that policy.

Re: [RFC PATCH 9/9] Loadpol LSM: add a minimal documentation

[Paul Moore]

On Fri, May 30, 2025 at 3:03 AM Simon Thoby <git@nightmared.fr> wrote:
> On 5/30/25 01:49, Paul Moore wrote:
> >
> > My thinking around possible augmentation of LoadPin is that both
> > LoadPin and Loadpol share a similar, limited focus of controlling
> > access to kernel module loading and Loadpol has support for a basic
> > loadable policy, a policy that could likely be extended to support a
> > LoadPin-esque construct that limit module loading based on filesystem
> > pinning.  It probably makes more sense to think of adding LoadPin
> > support to Loadpol, rather than augmenting LoadPin to support the
> > Loadpol concepts, but for consistency with upstream we probably need
> > to speak in terms of the latter.
>
> Thanks for the reply, I now see what you meant. I will try to put something
> together (hopefully next week), starting with looking at how we can express
> the current LoadPin feature set as a loadable and user-extensible policy, and
> then add non-filesystem-related policy entries (like module name restrictions)
> to that policy.

You may want to see what Kees thinks of the idea before you spend too
much time on this as he is the LoadPin maintainer.  I'm guessing he
would be okay with the additions, but that is just a guess on my part.

-- 
paul-moore.com