linux-kernel - Re: [PATCH v12 02/11] LSM: Maintain a table of LSM attribute data

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHC9VhRY1_tdLVnFGK4ZxRDEs+JKJWD3VR+iHrcrm9Psmbowtg@mail.gmail.com>
Date:   Fri, 21 Jul 2023 17:40:23 -0400
From:   Paul Moore <paul@...l-moore.com>
To:     Casey Schaufler <casey@...aufler-ca.com>
Cc:     Mickaël Salaün <mic@...ikod.net>,
        linux-security-module@...r.kernel.org, jmorris@...ei.org,
        serge@...lyn.com, keescook@...omium.org,
        john.johansen@...onical.com, penguin-kernel@...ove.sakura.ne.jp,
        stephen.smalley.work@...il.com, linux-kernel@...r.kernel.org,
        linux-api@...r.kernel.org
Subject: Re: [PATCH v12 02/11] LSM: Maintain a table of LSM attribute data

On Fri, Jul 14, 2023 at 3:42 PM Casey Schaufler <casey@...aufler-ca.com> wrote:
> On 7/11/2023 8:35 AM, Mickaël Salaün wrote:
> > On 29/06/2023 21:55, 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@...aufler-ca.com>
> >> Reviewed-by: Kees Cook <keescook@...omium.org>
> >> Reviewed-by: Serge Hallyn <serge@...lyn.com>
> >> ---
> >>   include/linux/security.h |  2 ++
> >>   security/security.c      | 37 +++++++++++++++++++++++++++++++++++++
> >>   2 files changed, 39 insertions(+)

...

> >> diff --git a/security/security.c b/security/security.c
> >> index e56714ef045a..5a699e47478b 100644
> >> --- a/security/security.c
> >> +++ b/security/security.c
> >> @@ -521,6 +546,18 @@ 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 during initialization, and LSM initialization
> >> +     * is serialized. Landlock is one such case.
> >> +     * Look at the previous entry, if there is one, for duplication.
> >> +     */
> >> +    if (lsm_active_cnt == 0 || lsm_idlist[lsm_active_cnt - 1] !=
> >> lsmid) {
> >
> > Isn't it possible to have interleaved security_add_hooks() calls?
>
> The initialization is serial and interleaving isn't possible.
>
> >> +        if (lsm_active_cnt >= LSM_CONFIG_COUNT)
> >> +            panic("%s Too many LSMs registered.\n", __func__);
> >
> > I'm not sure we should panic, but from a security point of view it is
> > critical enough…
>
> It's possible this should be a BUG() instance, but the panic() more
> closely resembles what's nearby in the code.

I think the panic() call is okay.  If something is so horribly broken
that we hit this case we have little option but to panic the system as
booting with the LSM controls busted in such a way is very not good.

There are probably those that would object to the above statement, but
those people aren't likely to be building a kernel with any LSMs in
the first place.

-- 
paul-moore.com