| Message ID | 20230421174259.2458-3-casey@schaufler-ca.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | LSM: Three basic syscalls | expand |
On Fri, Apr 21, 2023 at 10:42:50AM -0700, Casey Schaufler wrote: > As LSMs are registered add their lsm_id pointers to a table. > This will be used later for attribute reporting. > > Determine the number of possible security modules based on > their respective CONFIG options. This allows the number to be > known at build time. This allows data structures and tables > to use the constant. > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> Reviewed-by: Kees Cook <keescook@chromium.org> Nit below... > [...] > @@ -513,6 +531,15 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count, > { > int i; > > + if (lsm_active_cnt >= LSM_COUNT) > + panic("%s Too many LSMs registered.\n", __func__); > + /* > + * A security module may call security_add_hooks() more > + * than once. Landlock is one such case. > + */ > + if (lsm_active_cnt == 0 || lsm_idlist[lsm_active_cnt - 1] != lsmid) > + lsm_idlist[lsm_active_cnt++] = lsmid; > + I find this logic hard to parse. I think this might be better, since lsm_idlist will be entirely initialized to LSM_UNDEF, yes? /* * A security module may call security_add_hooks() more * than once during initialization, and LSM initialization * is serialized. Landlock is one such case. */ if (lsm_idlist[lsm_active_cnt] != lsmid) lsm_idlist[lsm_active_cnt++] = lsmid;
On 4/21/2023 12:20 PM, Kees Cook wrote: > On Fri, Apr 21, 2023 at 10:42:50AM -0700, Casey Schaufler wrote: >> As LSMs are registered add their lsm_id pointers to a table. >> This will be used later for attribute reporting. >> >> Determine the number of possible security modules based on >> their respective CONFIG options. This allows the number to be >> known at build time. This allows data structures and tables >> to use the constant. >> >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > Reviewed-by: Kees Cook <keescook@chromium.org> > > Nit below... > >> [...] >> @@ -513,6 +531,15 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count, >> { >> int i; >> >> + if (lsm_active_cnt >= LSM_COUNT) >> + panic("%s Too many LSMs registered.\n", __func__); >> + /* >> + * A security module may call security_add_hooks() more >> + * than once. Landlock is one such case. >> + */ >> + if (lsm_active_cnt == 0 || lsm_idlist[lsm_active_cnt - 1] != lsmid) >> + lsm_idlist[lsm_active_cnt++] = lsmid; >> + > I find this logic hard to parse. I think this might be better, since > lsm_idlist will be entirely initialized to LSM_UNDEF, yes? > > /* > * A security module may call security_add_hooks() more > * than once during initialization, and LSM initialization > * is serialized. Landlock is one such case. > */ > if (lsm_idlist[lsm_active_cnt] != lsmid) > lsm_idlist[lsm_active_cnt++] = lsmid; This code won't do the job. lsm_active_count indexes the first unset entry, not the last set entry.
diff --git a/include/linux/security.h b/include/linux/security.h index 5984d0d550b4..e70fc863b04a 100644 --- a/include/linux/security.h +++ b/include/linux/security.h @@ -138,6 +138,8 @@ enum lockdown_reason { }; extern const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1]; +extern u32 lsm_active_cnt; +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, diff --git a/security/security.c b/security/security.c index 58828a326024..3f98e5171176 100644 --- a/security/security.c +++ b/security/security.c @@ -28,12 +28,29 @@ #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 -/* How many LSMs were built into the kernel? */ -#define LSM_COUNT (__end_lsm_info - __start_lsm_info) +/* + * How many LSMs are built into the kernel as determined at + * build time. Used to determine fixed array sizes. + * The capability module is accounted for by CONFIG_SECURITY + */ +#define LSM_COUNT ( \ + (IS_ENABLED(CONFIG_SECURITY) ? 1 : 0) + \ + (IS_ENABLED(CONFIG_SECURITY_SELINUX) ? 1 : 0) + \ + (IS_ENABLED(CONFIG_SECURITY_SMACK) ? 1 : 0) + \ + (IS_ENABLED(CONFIG_SECURITY_TOMOYO) ? 1 : 0) + \ + (IS_ENABLED(CONFIG_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_LSM) ? 1 : 0) + \ + (IS_ENABLED(CONFIG_BPF_LSM) ? 1 : 0) + \ + (IS_ENABLED(CONFIG_SECURITY_LANDLOCK) ? 1 : 0)) /* * These are descriptions of the reasons that can be passed to the @@ -90,7 +107,7 @@ static __initdata const char *chosen_major_lsm; static __initconst const char * const builtin_lsm_order = CONFIG_LSM; /* Ordered list of LSMs to initialize. */ -static __initdata struct lsm_info **ordered_lsms; +static __initdata struct lsm_info *ordered_lsms[LSM_COUNT + 1]; static __initdata struct lsm_info *exclusive; static __initdata bool debug; @@ -341,13 +358,16 @@ static void __init report_lsm_order(void) pr_cont("\n"); } +/* + * Current index to use while initializing the lsm id list. + */ +u32 lsm_active_cnt __lsm_ro_after_init; +struct lsm_id *lsm_idlist[LSM_COUNT] __lsm_ro_after_init; + static void __init ordered_lsm_init(void) { struct lsm_info **lsm; - ordered_lsms = kcalloc(LSM_COUNT + 1, sizeof(*ordered_lsms), - GFP_KERNEL); - if (chosen_lsm_order) { if (chosen_major_lsm) { pr_warn("security=%s is ignored because it is superseded by lsm=%s\n", @@ -387,8 +407,6 @@ static void __init ordered_lsm_init(void) lsm_early_task(current); for (lsm = ordered_lsms; *lsm; lsm++) initialize_lsm(*lsm); - - kfree(ordered_lsms); } int __init early_security_init(void) @@ -513,6 +531,15 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count, { int i; + if (lsm_active_cnt >= LSM_COUNT) + panic("%s Too many LSMs registered.\n", __func__); + /* + * A security module may call security_add_hooks() more + * than once. Landlock is one such case. + */ + if (lsm_active_cnt == 0 || lsm_idlist[lsm_active_cnt - 1] != lsmid) + lsm_idlist[lsm_active_cnt++] = lsmid; + 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. Determine the number of possible security modules based on their respective CONFIG options. This allows the number to be known at build time. This allows data structures and tables to use the constant. Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> --- include/linux/security.h | 2 ++ security/security.c | 43 ++++++++++++++++++++++++++++++++-------- 2 files changed, 37 insertions(+), 8 deletions(-)