| Message ID | 20221025184519.13231-5-casey@schaufler-ca.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | LSM: Two basic syscalls | expand |
On Tue, Oct 25, 2022 at 11:45:15AM -0700, Casey Schaufler wrote: > As LSMs are registered add their lsm_id pointers to a table. > This will be used later for attribute reporting. > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > --- > include/linux/security.h | 17 +++++++++++++++++ > security/security.c | 18 ++++++++++++++++++ > 2 files changed, 35 insertions(+) > > diff --git a/include/linux/security.h b/include/linux/security.h > index ca1b7109c0db..e1678594d983 100644 > --- a/include/linux/security.h > +++ b/include/linux/security.h > @@ -138,6 +138,23 @@ enum lockdown_reason { > > extern const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1]; > > +#define LSMID_ENTRIES ( \ > + 1 + /* capabilities */ \ No #define for capabilities? > + (IS_ENABLED(CONFIG_SECURITY_SELINUX) ? 1 : 0) + \ > + (IS_ENABLED(CONFIG_SECURITY_SMACK) ? 1 : 0) + \ > + (IS_ENABLED(CONFIG_SECURITY_TOMOYO) ? 1 : 0) + \ > + (IS_ENABLED(CONFIG_SECURITY_IMA) ? 1 : 0) + \ > + (IS_ENABLED(CONFIG_SECURITY_APPARMOR) ? 1 : 0) + \ > + (IS_ENABLED(CONFIG_SECURITY_YAMA) ? 1 : 0) + \ > + (IS_ENABLED(CONFIG_SECURITY_LOADPIN) ? 1 : 0) + \ > + (IS_ENABLED(CONFIG_SECURITY_SAFESETID) ? 1 : 0) + \ > + (IS_ENABLED(CONFIG_SECURITY_LOCKDOWN) ? 1 : 0) + \ > + (IS_ENABLED(CONFIG_BPF_LSM) ? 1 : 0) + \ > + (IS_ENABLED(CONFIG_SECURITY_LANDLOCK) ? 1 : 0)) > + > +extern int lsm_id; u64? thanks, greg k-h
On 10/25/2022 11:00 PM, Greg KH wrote: > On Tue, Oct 25, 2022 at 11:45:15AM -0700, Casey Schaufler wrote: >> As LSMs are registered add their lsm_id pointers to a table. >> This will be used later for attribute reporting. >> >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> >> --- >> include/linux/security.h | 17 +++++++++++++++++ >> security/security.c | 18 ++++++++++++++++++ >> 2 files changed, 35 insertions(+) >> >> diff --git a/include/linux/security.h b/include/linux/security.h >> index ca1b7109c0db..e1678594d983 100644 >> --- a/include/linux/security.h >> +++ b/include/linux/security.h >> @@ -138,6 +138,23 @@ enum lockdown_reason { >> >> extern const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1]; >> >> +#define LSMID_ENTRIES ( \ >> + 1 + /* capabilities */ \ > No #define for capabilities? Nope. There isn't one. CONFIG_SECURITY takes care of it. >> + (IS_ENABLED(CONFIG_SECURITY_SELINUX) ? 1 : 0) + \ >> + (IS_ENABLED(CONFIG_SECURITY_SMACK) ? 1 : 0) + \ >> + (IS_ENABLED(CONFIG_SECURITY_TOMOYO) ? 1 : 0) + \ >> + (IS_ENABLED(CONFIG_SECURITY_IMA) ? 1 : 0) + \ >> + (IS_ENABLED(CONFIG_SECURITY_APPARMOR) ? 1 : 0) + \ >> + (IS_ENABLED(CONFIG_SECURITY_YAMA) ? 1 : 0) + \ >> + (IS_ENABLED(CONFIG_SECURITY_LOADPIN) ? 1 : 0) + \ >> + (IS_ENABLED(CONFIG_SECURITY_SAFESETID) ? 1 : 0) + \ >> + (IS_ENABLED(CONFIG_SECURITY_LOCKDOWN) ? 1 : 0) + \ >> + (IS_ENABLED(CONFIG_BPF_LSM) ? 1 : 0) + \ >> + (IS_ENABLED(CONFIG_SECURITY_LANDLOCK) ? 1 : 0)) >> + >> +extern int lsm_id; > u64? u32. I doubt we'll get more than 32K security modules. > > thanks, > > greg k-h
On Wed, Oct 26, 2022 at 05:38:21PM -0700, Casey Schaufler wrote: > On 10/25/2022 11:00 PM, Greg KH wrote: > > On Tue, Oct 25, 2022 at 11:45:15AM -0700, Casey Schaufler wrote: > >> As LSMs are registered add their lsm_id pointers to a table. > >> This will be used later for attribute reporting. > >> > >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > >> --- > >> include/linux/security.h | 17 +++++++++++++++++ > >> security/security.c | 18 ++++++++++++++++++ > >> 2 files changed, 35 insertions(+) > >> > >> diff --git a/include/linux/security.h b/include/linux/security.h > >> index ca1b7109c0db..e1678594d983 100644 > >> --- a/include/linux/security.h > >> +++ b/include/linux/security.h > >> @@ -138,6 +138,23 @@ enum lockdown_reason { > >> > >> extern const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1]; > >> > >> +#define LSMID_ENTRIES ( \ > >> + 1 + /* capabilities */ \ > > No #define for capabilities? > > Nope. There isn't one. CONFIG_SECURITY takes care of it. > > >> + (IS_ENABLED(CONFIG_SECURITY_SELINUX) ? 1 : 0) + \ > >> + (IS_ENABLED(CONFIG_SECURITY_SMACK) ? 1 : 0) + \ > >> + (IS_ENABLED(CONFIG_SECURITY_TOMOYO) ? 1 : 0) + \ > >> + (IS_ENABLED(CONFIG_SECURITY_IMA) ? 1 : 0) + \ > >> + (IS_ENABLED(CONFIG_SECURITY_APPARMOR) ? 1 : 0) + \ > >> + (IS_ENABLED(CONFIG_SECURITY_YAMA) ? 1 : 0) + \ > >> + (IS_ENABLED(CONFIG_SECURITY_LOADPIN) ? 1 : 0) + \ > >> + (IS_ENABLED(CONFIG_SECURITY_SAFESETID) ? 1 : 0) + \ > >> + (IS_ENABLED(CONFIG_SECURITY_LOCKDOWN) ? 1 : 0) + \ > >> + (IS_ENABLED(CONFIG_BPF_LSM) ? 1 : 0) + \ > >> + (IS_ENABLED(CONFIG_SECURITY_LANDLOCK) ? 1 : 0)) > >> + > >> +extern int lsm_id; > > u64? > > u32. I doubt we'll get more than 32K security modules. These should be bits, not values, right? Wait, this magic entry value is going to change depeneding on what is, or is not, enabled. How is that a stable user/kernel api at all? confused. greg k-h
On 10/26/2022 11:29 PM, Greg KH wrote: > On Wed, Oct 26, 2022 at 05:38:21PM -0700, Casey Schaufler wrote: >> On 10/25/2022 11:00 PM, Greg KH wrote: >>> On Tue, Oct 25, 2022 at 11:45:15AM -0700, Casey Schaufler wrote: >>>> As LSMs are registered add their lsm_id pointers to a table. >>>> This will be used later for attribute reporting. >>>> >>>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> >>>> --- >>>> include/linux/security.h | 17 +++++++++++++++++ >>>> security/security.c | 18 ++++++++++++++++++ >>>> 2 files changed, 35 insertions(+) >>>> >>>> diff --git a/include/linux/security.h b/include/linux/security.h >>>> index ca1b7109c0db..e1678594d983 100644 >>>> --- a/include/linux/security.h >>>> +++ b/include/linux/security.h >>>> @@ -138,6 +138,23 @@ enum lockdown_reason { >>>> >>>> extern const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1]; >>>> >>>> +#define LSMID_ENTRIES ( \ >>>> + 1 + /* capabilities */ \ >>> No #define for capabilities? >> Nope. There isn't one. CONFIG_SECURITY takes care of it. >> >>>> + (IS_ENABLED(CONFIG_SECURITY_SELINUX) ? 1 : 0) + \ >>>> + (IS_ENABLED(CONFIG_SECURITY_SMACK) ? 1 : 0) + \ >>>> + (IS_ENABLED(CONFIG_SECURITY_TOMOYO) ? 1 : 0) + \ >>>> + (IS_ENABLED(CONFIG_SECURITY_IMA) ? 1 : 0) + \ >>>> + (IS_ENABLED(CONFIG_SECURITY_APPARMOR) ? 1 : 0) + \ >>>> + (IS_ENABLED(CONFIG_SECURITY_YAMA) ? 1 : 0) + \ >>>> + (IS_ENABLED(CONFIG_SECURITY_LOADPIN) ? 1 : 0) + \ >>>> + (IS_ENABLED(CONFIG_SECURITY_SAFESETID) ? 1 : 0) + \ >>>> + (IS_ENABLED(CONFIG_SECURITY_LOCKDOWN) ? 1 : 0) + \ >>>> + (IS_ENABLED(CONFIG_BPF_LSM) ? 1 : 0) + \ >>>> + (IS_ENABLED(CONFIG_SECURITY_LANDLOCK) ? 1 : 0)) >>>> + >>>> +extern int lsm_id; >>> u64? >> u32. I doubt we'll get more than 32K security modules. > These should be bits, not values, right? lsm_id is the count of security modules that are registered. It seemed like a good name for the value at the time, but as it's causing confusion I should probably change it. > Wait, this magic entry value is going to change depeneding on what is, > or is not, enabled. How is that a stable user/kernel api at all? > > confused. I'll clarify. This patch isn't implementing an API, but is required by subsequent patches that do. Does linux-api want to see patches that are in support of APIs, or just those with actual API implementation? Thank you. > greg k-h
On Thu, Oct 27, 2022 at 10:08:23AM -0700, Casey Schaufler wrote: > On 10/26/2022 11:29 PM, Greg KH wrote: > > On Wed, Oct 26, 2022 at 05:38:21PM -0700, Casey Schaufler wrote: > >> On 10/25/2022 11:00 PM, Greg KH wrote: > >>> On Tue, Oct 25, 2022 at 11:45:15AM -0700, Casey Schaufler wrote: > >>>> As LSMs are registered add their lsm_id pointers to a table. > >>>> This will be used later for attribute reporting. > >>>> > >>>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > >>>> --- > >>>> include/linux/security.h | 17 +++++++++++++++++ > >>>> security/security.c | 18 ++++++++++++++++++ > >>>> 2 files changed, 35 insertions(+) > >>>> > >>>> diff --git a/include/linux/security.h b/include/linux/security.h > >>>> index ca1b7109c0db..e1678594d983 100644 > >>>> --- a/include/linux/security.h > >>>> +++ b/include/linux/security.h > >>>> @@ -138,6 +138,23 @@ enum lockdown_reason { > >>>> > >>>> extern const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1]; > >>>> > >>>> +#define LSMID_ENTRIES ( \ > >>>> + 1 + /* capabilities */ \ > >>> No #define for capabilities? > >> Nope. There isn't one. CONFIG_SECURITY takes care of it. > >> > >>>> + (IS_ENABLED(CONFIG_SECURITY_SELINUX) ? 1 : 0) + \ > >>>> + (IS_ENABLED(CONFIG_SECURITY_SMACK) ? 1 : 0) + \ > >>>> + (IS_ENABLED(CONFIG_SECURITY_TOMOYO) ? 1 : 0) + \ > >>>> + (IS_ENABLED(CONFIG_SECURITY_IMA) ? 1 : 0) + \ > >>>> + (IS_ENABLED(CONFIG_SECURITY_APPARMOR) ? 1 : 0) + \ > >>>> + (IS_ENABLED(CONFIG_SECURITY_YAMA) ? 1 : 0) + \ > >>>> + (IS_ENABLED(CONFIG_SECURITY_LOADPIN) ? 1 : 0) + \ > >>>> + (IS_ENABLED(CONFIG_SECURITY_SAFESETID) ? 1 : 0) + \ > >>>> + (IS_ENABLED(CONFIG_SECURITY_LOCKDOWN) ? 1 : 0) + \ > >>>> + (IS_ENABLED(CONFIG_BPF_LSM) ? 1 : 0) + \ > >>>> + (IS_ENABLED(CONFIG_SECURITY_LANDLOCK) ? 1 : 0)) > >>>> + > >>>> +extern int lsm_id; > >>> u64? > >> u32. I doubt we'll get more than 32K security modules. > > These should be bits, not values, right? > > lsm_id is the count of security modules that are registered. > It seemed like a good name for the value at the time, but as > it's causing confusion I should probably change it. Yeah, that's confusing. "lsm_num_availble" might be better. > > Wait, this magic entry value is going to change depeneding on what is, > > or is not, enabled. How is that a stable user/kernel api at all? > > > > confused. > > I'll clarify. > > This patch isn't implementing an API, but is required by subsequent > patches that do. Does linux-api want to see patches that are in support > of APIs, or just those with actual API implementation? There's nothing wrong with seeing this patch, I was just confused as it seemed to be a user facing api. It wasn't obvious to me, sorry. greg k-h
On Thu, Oct 27, 2022 at 1:14 PM Greg KH <gregkh@linuxfoundation.org> wrote: > On Thu, Oct 27, 2022 at 10:08:23AM -0700, Casey Schaufler wrote: > > On 10/26/2022 11:29 PM, Greg KH wrote: > > > On Wed, Oct 26, 2022 at 05:38:21PM -0700, Casey Schaufler wrote: > > >> On 10/25/2022 11:00 PM, Greg KH wrote: > > >>> On Tue, Oct 25, 2022 at 11:45:15AM -0700, Casey Schaufler wrote: > > >>>> As LSMs are registered add their lsm_id pointers to a table. > > >>>> This will be used later for attribute reporting. > > >>>> > > >>>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > > >>>> --- > > >>>> include/linux/security.h | 17 +++++++++++++++++ > > >>>> security/security.c | 18 ++++++++++++++++++ > > >>>> 2 files changed, 35 insertions(+) > > >>>> > > >>>> diff --git a/include/linux/security.h b/include/linux/security.h > > >>>> index ca1b7109c0db..e1678594d983 100644 > > >>>> --- a/include/linux/security.h > > >>>> +++ b/include/linux/security.h > > >>>> @@ -138,6 +138,23 @@ enum lockdown_reason { > > >>>> > > >>>> extern const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1]; > > >>>> > > >>>> +#define LSMID_ENTRIES ( \ > > >>>> + 1 + /* capabilities */ \ > > >>> No #define for capabilities? > > >> Nope. There isn't one. CONFIG_SECURITY takes care of it. > > >> > > >>>> + (IS_ENABLED(CONFIG_SECURITY_SELINUX) ? 1 : 0) + \ > > >>>> + (IS_ENABLED(CONFIG_SECURITY_SMACK) ? 1 : 0) + \ > > >>>> + (IS_ENABLED(CONFIG_SECURITY_TOMOYO) ? 1 : 0) + \ > > >>>> + (IS_ENABLED(CONFIG_SECURITY_IMA) ? 1 : 0) + \ > > >>>> + (IS_ENABLED(CONFIG_SECURITY_APPARMOR) ? 1 : 0) + \ > > >>>> + (IS_ENABLED(CONFIG_SECURITY_YAMA) ? 1 : 0) + \ > > >>>> + (IS_ENABLED(CONFIG_SECURITY_LOADPIN) ? 1 : 0) + \ > > >>>> + (IS_ENABLED(CONFIG_SECURITY_SAFESETID) ? 1 : 0) + \ > > >>>> + (IS_ENABLED(CONFIG_SECURITY_LOCKDOWN) ? 1 : 0) + \ > > >>>> + (IS_ENABLED(CONFIG_BPF_LSM) ? 1 : 0) + \ > > >>>> + (IS_ENABLED(CONFIG_SECURITY_LANDLOCK) ? 1 : 0)) > > >>>> + > > >>>> +extern int lsm_id; > > >>> u64? > > >> u32. I doubt we'll get more than 32K security modules. > > > These should be bits, not values, right? > > > > lsm_id is the count of security modules that are registered. > > It seemed like a good name for the value at the time, but as > > it's causing confusion I should probably change it. > > Yeah, that's confusing. "lsm_num_availble" might be better. Yes, this really should be named something else. I'm partial to "lsm_count" as it is shorter than the other suggestion, but this is hardly something to worry too much about. -- paul-moore.com
diff --git a/include/linux/security.h b/include/linux/security.h index ca1b7109c0db..e1678594d983 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -138,6 +138,23 @@ enum lockdown_reason { extern const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1]; +#define LSMID_ENTRIES ( \ + 1 + /* capabilities */ \ + (IS_ENABLED(CONFIG_SECURITY_SELINUX) ? 1 : 0) + \ + (IS_ENABLED(CONFIG_SECURITY_SMACK) ? 1 : 0) + \ + (IS_ENABLED(CONFIG_SECURITY_TOMOYO) ? 1 : 0) + \ + (IS_ENABLED(CONFIG_SECURITY_IMA) ? 1 : 0) + \ + (IS_ENABLED(CONFIG_SECURITY_APPARMOR) ? 1 : 0) + \ + (IS_ENABLED(CONFIG_SECURITY_YAMA) ? 1 : 0) + \ + (IS_ENABLED(CONFIG_SECURITY_LOADPIN) ? 1 : 0) + \ + (IS_ENABLED(CONFIG_SECURITY_SAFESETID) ? 1 : 0) + \ + (IS_ENABLED(CONFIG_SECURITY_LOCKDOWN) ? 1 : 0) + \ + (IS_ENABLED(CONFIG_BPF_LSM) ? 1 : 0) + \ + (IS_ENABLED(CONFIG_SECURITY_LANDLOCK) ? 1 : 0)) + +extern int lsm_id; +extern struct lsm_id *lsm_idlist[]; + /* These functions are in security/commoncap.c */ extern int cap_capable(const struct cred *cred, struct user_namespace *ns, int cap, unsigned int opts); diff --git a/security/security.c b/security/security.c index b2eb0ccd954b..bf206996a2af 100644 --- a/security/security.c +++ b/security/security.c @@ -28,6 +28,7 @@ #include <linux/backing-dev.h> #include <linux/string.h> #include <linux/msg.h> +#include <uapi/linux/lsm.h> #include <net/flow.h> #define MAX_LSM_EVM_XATTR 2 @@ -320,6 +321,12 @@ static void __init lsm_early_task(struct task_struct *task); static int lsm_append(const char *new, char **result); +/* + * Current index to use while initializing the lsm id list. + */ +int lsm_id __lsm_ro_after_init; +struct lsm_id *lsm_idlist[LSMID_ENTRIES] __lsm_ro_after_init; + static void __init ordered_lsm_init(void) { struct lsm_info **lsm; @@ -364,6 +371,7 @@ static void __init ordered_lsm_init(void) for (lsm = ordered_lsms; *lsm; lsm++) initialize_lsm(*lsm); + init_debug("lsm count = %d\n", lsm_id); kfree(ordered_lsms); } @@ -485,6 +493,16 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count, { int i; + /* + * A security module may call security_add_hooks() more + * than once. Landlock is one such case. + */ + if (lsm_id == 0 || lsm_idlist[lsm_id - 1] != lsmid) + lsm_idlist[lsm_id++] = lsmid; + + if (lsm_id > LSMID_ENTRIES) + panic("%s Too many LSMs registered.\n", __func__); + for (i = 0; i < count; i++) { hooks[i].lsmid = lsmid; hlist_add_tail_rcu(&hooks[i].list, hooks[i].head);
As LSMs are registered add their lsm_id pointers to a table. This will be used later for attribute reporting. Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> --- include/linux/security.h | 17 +++++++++++++++++ security/security.c | 18 ++++++++++++++++++ 2 files changed, 35 insertions(+)