Re: [PATCH v1 2/8] LSM: Add an LSM identifier for external use

From: Casey Schaufler <casey@schaufler-ca.com>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: casey.schaufler@intel.com, paul@paul-moore.com,
	linux-security-module@vger.kernel.org, jmorris@namei.org,
	keescook@chromium.org, john.johansen@canonical.com,
	penguin-kernel@i-love.sakura.ne.jp,
	stephen.smalley.work@gmail.com, linux-kernel@vger.kernel.org,
	linux-api@vger.kernel.org, mic@digikod.net,
	casey@schaufler-ca.com
Subject: Re: [PATCH v1 2/8] LSM: Add an LSM identifier for external use
Date: Wed, 26 Oct 2022 12:36:34 -0700	[thread overview]
Message-ID: <5396ab1e-9b93-df33-ca49-58dc59459a76@schaufler-ca.com> (raw)
In-Reply-To: <Y1jMeWl0oV4/2zyE@kroah.com>

On 10/25/2022 10:58 PM, Greg KH wrote:
> On Tue, Oct 25, 2022 at 11:45:13AM -0700, Casey Schaufler wrote:
>> Add an integer member "id" to the struct lsm_id. This value is
>> a unique identifier associated with each security module. The
>> values are defined in a new UAPI header file. Each existing LSM
>> has been updated to include it's LSMID in the lsm_id.
>>
>> The LSM ID values are sequential, with the oldest module
>> LSM_ID_CAPABILITY being the lowest value and the existing
>> modules numbered in the order they were included in the
>> main line kernel. The first 32 values (0 - 31) are reserved
>> for some as yet unknown but important use.
>>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> ---
>>  include/linux/lsm_hooks.h    |  1 +
>>  include/uapi/linux/lsm.h     | 32 ++++++++++++++++++++++++++++++++
>>  security/apparmor/lsm.c      |  2 ++
>>  security/bpf/hooks.c         |  2 ++
>>  security/commoncap.c         |  2 ++
>>  security/landlock/setup.c    |  2 ++
>>  security/loadpin/loadpin.c   |  2 ++
>>  security/lockdown/lockdown.c |  2 ++
>>  security/safesetid/lsm.c     |  2 ++
>>  security/selinux/hooks.c     |  2 ++
>>  security/smack/smack_lsm.c   |  2 ++
>>  security/tomoyo/tomoyo.c     |  2 ++
>>  security/yama/yama_lsm.c     |  2 ++
>>  13 files changed, 55 insertions(+)
>>  create mode 100644 include/uapi/linux/lsm.h
>>
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index e383e468f742..dd4b4d95a172 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -1607,6 +1607,7 @@ struct security_hook_heads {
>>   */
>>  struct lsm_id {
>>  	const char	*lsm;		/* Name of the LSM */
>> +	int		id;		/* LSM ID */
> Again, kerneldoc.
>
> And if this crosses the user/kernel boundry, please make it __u64.  Or
> __s32?  Something explicit please.

Will do.

>>  };
>>  
>>  /*
>> diff --git a/include/uapi/linux/lsm.h b/include/uapi/linux/lsm.h
>> new file mode 100644
>> index 000000000000..d5bcbb9375df
>> --- /dev/null
>> +++ b/include/uapi/linux/lsm.h
>> @@ -0,0 +1,32 @@
>> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
>> +/*
>> + * Linus Security Modules (LSM) - User space API
> s/Linus/Linux/.

Whoops.

>
>
>> + *
>> + * Copyright (C) 2022 Casey Schaufler <casey@schaufler-ca.com>
>> + * Copyright (C) Intel Corporation
> No date for Intel?

The latest guidance I have received is that Intel does not want a date.

>> + */
>> +
>> +#ifndef _UAPI_LINUX_LSM_H
>> +#define _UAPI_LINUX_LSM_H
>> +
>> +/*
>> + * ID values to identify security modules.
>> + * A system may use more than one security module.
>> + *
>> + * LSM_ID_XXX values 0 - 31 are reserved for future use
> Reserved for what?  Why?

You're not the first person to ask. I'll remove the reserved values
for the next version. The invalid value has to change as the id field
is going to be unsigned.

>
> thanks,
>
> greg k-h