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

[Simon Thoby <git@nightmared.fr>]

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>