Re: [PATCH v38 01/39] LSM: Identify modules by more than name

[Casey Schaufler]

On 3/3/2023 7:17 AM, Georgia Garcia wrote:
> Hi!
>
> On Tue, 2022-09-27 at 12:53 -0700, Casey Schaufler wrote:
>> Create a struct lsm_id to contain identifying information
>> about Linux Security Modules (LSMs). At inception this contains
>> a single member, which is the name of the module. Change the
>> security_add_hooks() interface to use this structure. Change
>> the individual modules to maintain their own struct lsm_id and
>> pass it to security_add_hooks().
>>
>> Signed-off-by: Casey Schaufler <casey@xxxxxxxxxxxxxxxx>
>> ---
>>  include/linux/lsm_hooks.h    | 11 +++++++++--
>>  security/apparmor/lsm.c      |  6 +++++-
>>  security/bpf/hooks.c         | 11 ++++++++++-
>>  security/commoncap.c         |  6 +++++-
>>  security/landlock/cred.c     |  2 +-
>>  security/landlock/fs.c       |  2 +-
>>  security/landlock/ptrace.c   |  2 +-
>>  security/landlock/setup.c    |  4 ++++
>>  security/landlock/setup.h    |  1 +
>>  security/loadpin/loadpin.c   |  7 ++++++-
>>  security/lockdown/lockdown.c |  6 +++++-
>>  security/safesetid/lsm.c     |  7 ++++++-
>>  security/security.c          | 12 ++++++------
>>  security/selinux/hooks.c     |  7 ++++++-
>>  security/smack/smack_lsm.c   |  6 +++++-
>>  security/tomoyo/tomoyo.c     |  7 ++++++-
>>  security/yama/yama_lsm.c     |  6 +++++-
>>  17 files changed, 82 insertions(+), 21 deletions(-)
>>
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index 3aa6030302f5..23054881eb08 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -1598,6 +1598,13 @@ struct security_hook_heads {
>>  	#undef LSM_HOOK
>>  } __randomize_layout;
>>  
>> +/*
>> + * Information that identifies a security module.
>> + */
>> +struct lsm_id {
>> +	const char	*lsm;		/* Name of the LSM */
>> +};
>> +
>>  /*
>>   * Security module hook list structure.
>>   * For use with generic list macros for common operations.
>> @@ -1606,7 +1613,7 @@ struct security_hook_list {
>>  	struct hlist_node		list;
>>  	struct hlist_head		*head;
>>  	union security_list_options	hook;
>> -	const char			*lsm;
>> +	struct lsm_id			*lsmid;
>>  } __randomize_layout;
>>  
>>  /*
>> @@ -1641,7 +1648,7 @@ extern struct security_hook_heads security_hook_heads;
>>  extern char *lsm_names;
>>  
>>  extern void security_add_hooks(struct security_hook_list *hooks, int count,
>> -				const char *lsm);
>> +			       struct lsm_id *lsmid);
>>  
>>  #define LSM_FLAG_LEGACY_MAJOR	BIT(0)
>>  #define LSM_FLAG_EXCLUSIVE	BIT(1)
>> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
>> index e29cade7b662..b71f7d4159d7 100644
>> --- a/security/apparmor/lsm.c
>> +++ b/security/apparmor/lsm.c
>> @@ -1202,6 +1202,10 @@ struct lsm_blob_sizes apparmor_blob_sizes __lsm_ro_after_init = {
>>  	.lbs_task = sizeof(struct aa_task_ctx),
>>  };
>>  
>> +static struct lsm_id apparmor_lsmid __lsm_ro_after_init = {
>> +	.lsm      = "apparmor",
>> +};
>> +
>>  static struct security_hook_list apparmor_hooks[] __lsm_ro_after_init = {
>>  	LSM_HOOK_INIT(ptrace_access_check, apparmor_ptrace_access_check),
>>  	LSM_HOOK_INIT(ptrace_traceme, apparmor_ptrace_traceme),
>> @@ -1897,7 +1901,7 @@ static int __init apparmor_init(void)
>>  		goto buffers_out;
>>  	}
>>  	security_add_hooks(apparmor_hooks, ARRAY_SIZE(apparmor_hooks),
>> -				"apparmor");
>> +				&apparmor_lsmid);
>>  
>>  	/* Report that AppArmor successfully initialized */
>>  	apparmor_initialized = 1;
>> diff --git a/security/bpf/hooks.c b/security/bpf/hooks.c
>> index e5971fa74fd7..e50de3abfde2 100644
>> --- a/security/bpf/hooks.c
>> +++ b/security/bpf/hooks.c
>> @@ -15,9 +15,18 @@ static struct security_hook_list bpf_lsm_hooks[] __lsm_ro_after_init = {
>>  	LSM_HOOK_INIT(task_free, bpf_task_storage_free),
>>  };
>>  
>> +/*
>> + * slot has to be LSMBLOB_NEEDED because some of the hooks
>> + * supplied by this module require a slot.
>> + */
>> +struct lsm_id bpf_lsmid __lsm_ro_after_init = {
>> +	.lsm      = "bpf",
>> +};
> Can bpf_lsmid be static too?

Yes. Thank you for the review.

>
>> +
>>  static int __init bpf_lsm_init(void)
>>  {
>> -	security_add_hooks(bpf_lsm_hooks, ARRAY_SIZE(bpf_lsm_hooks), "bpf");
>> +	security_add_hooks(bpf_lsm_hooks, ARRAY_SIZE(bpf_lsm_hooks),
>> +			   &bpf_lsmid);
>>  	pr_info("LSM support for eBPF active\n");
>>  	return 0;
>>  }
> Thanks